diff options
36 files changed, 317 insertions, 185 deletions
diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs index 8d5512a6a..817c7aec4 100644 --- a/crates/nu-cli/src/completions/custom_completions.rs +++ b/crates/nu-cli/src/completions/custom_completions.rs @@ -7,7 +7,7 @@ use nu_protocol::{ ast::{Argument, Call, Expr, Expression}, debugger::WithoutDebug, engine::{Stack, StateWorkingSet}, - CompletionSort, PipelineData, Span, Type, Value, + CompletionSort, DeclId, PipelineData, Span, Type, Value, }; use nu_utils::IgnoreCaseExt; use std::collections::HashMap; @@ -16,12 +16,12 @@ use super::completion_common::sort_suggestions; pub struct CustomCompletion { stack: Stack, - decl_id: usize, + decl_id: DeclId, line: String, } impl CustomCompletion { - pub fn new(stack: Stack, decl_id: usize, line: String) -> Self { + pub fn new(stack: Stack, decl_id: DeclId, line: String) -> Self { Self { stack, decl_id, diff --git a/crates/nu-cli/src/menus/menu_completions.rs b/crates/nu-cli/src/menus/menu_completions.rs index 4c4c9f59e..38a48afba 100644 --- a/crates/nu-cli/src/menus/menu_completions.rs +++ b/crates/nu-cli/src/menus/menu_completions.rs @@ -2,7 +2,7 @@ use nu_engine::eval_block; use nu_protocol::{ debugger::WithoutDebug, engine::{EngineState, Stack}, - IntoPipelineData, Span, Value, + BlockId, IntoPipelineData, Span, Value, }; use reedline::{menu_functions::parse_selection_char, Completer, Suggestion}; use std::sync::Arc; @@ -10,7 +10,7 @@ use std::sync::Arc; const SELECTION_CHAR: char = '!'; pub struct NuMenuCompleter { - block_id: usize, + block_id: BlockId, span: Span, stack: Stack, engine_state: Arc<EngineState>, @@ -19,7 +19,7 @@ pub struct NuMenuCompleter { impl NuMenuCompleter { pub fn new( - block_id: usize, + block_id: BlockId, span: Span, stack: Stack, engine_state: Arc<EngineState>, diff --git a/crates/nu-cmd-lang/src/core_commands/overlay/use_.rs b/crates/nu-cmd-lang/src/core_commands/overlay/use_.rs index 04cdb7a3d..1263fcaa2 100644 --- a/crates/nu-cmd-lang/src/core_commands/overlay/use_.rs +++ b/crates/nu-cmd-lang/src/core_commands/overlay/use_.rs @@ -2,7 +2,7 @@ use nu_engine::{ command_prelude::*, find_in_dirs_env, get_dirs_var_from_call, get_eval_block, redirect_env, }; use nu_parser::trim_quotes_str; -use nu_protocol::{ast::Expr, engine::CommandType}; +use nu_protocol::{ast::Expr, engine::CommandType, ModuleId}; use std::path::Path; @@ -64,7 +64,7 @@ impl Command for OverlayUse { let mut name_arg: Spanned<String> = call.req(engine_state, caller_stack, 0)?; name_arg.item = trim_quotes_str(&name_arg.item).to_string(); - let maybe_origin_module_id = + let maybe_origin_module_id: Option<ModuleId> = if let Some(overlay_expr) = call.get_parser_info(caller_stack, "overlay_expr") { if let Expr::Overlay(module_id) = &overlay_expr.expr { *module_id diff --git a/crates/nu-command/src/charting/hashable_value.rs b/crates/nu-command/src/charting/hashable_value.rs index e72f0fee5..b74b5f776 100644 --- a/crates/nu-command/src/charting/hashable_value.rs +++ b/crates/nu-command/src/charting/hashable_value.rs @@ -183,6 +183,7 @@ mod test { use nu_protocol::{ ast::{CellPath, PathMember}, engine::Closure, + BlockId, }; use std::collections::HashSet; @@ -244,7 +245,7 @@ mod test { Value::list(vec![Value::bool(true, span)], span), Value::closure( Closure { - block_id: 0, + block_id: BlockId::new(0), captures: Vec::new(), }, span, diff --git a/crates/nu-command/src/debug/explain.rs b/crates/nu-command/src/debug/explain.rs index dc68b283e..7d01727ce 100644 --- a/crates/nu-command/src/debug/explain.rs +++ b/crates/nu-command/src/debug/explain.rs @@ -271,7 +271,7 @@ pub fn debug_string_without_formatting(value: &Value) -> String { .join(" ") ), //TODO: It would be good to drill deeper into closures. - Value::Closure { val, .. } => format!("<Closure {}>", val.block_id), + Value::Closure { val, .. } => format!("<Closure {}>", val.block_id.get()), Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), diff --git a/crates/nu-command/src/debug/view_ir.rs b/crates/nu-command/src/debug/view_ir.rs index 676285844..1a7309de4 100644 --- a/crates/nu-command/src/debug/view_ir.rs +++ b/crates/nu-command/src/debug/view_ir.rs @@ -1,4 +1,5 @@ use nu_engine::command_prelude::*; +use nu_protocol::{BlockId, DeclId}; #[derive(Clone)] pub struct ViewIr; @@ -86,7 +87,8 @@ the declaration may not be in scope. let decl_id = val .try_into() .ok() - .filter(|id| *id < engine_state.num_decls()) + .map(DeclId::new) + .filter(|id| id.get() < engine_state.num_decls()) .ok_or_else(|| ShellError::IncorrectValue { msg: "not a valid decl id".into(), val_span: target.span(), @@ -102,11 +104,15 @@ the declaration may not be in scope. })? } // Block by ID - often shows up in IR - Value::Int { val, .. } => val.try_into().map_err(|_| ShellError::IncorrectValue { - msg: "not a valid block id".into(), - val_span: target.span(), - call_span: call.head, - })?, + Value::Int { val, .. } => { + val.try_into() + .map(BlockId::new) + .map_err(|_| ShellError::IncorrectValue { + msg: "not a valid block id".into(), + val_span: target.span(), + call_span: call.head, + })? + } // Pass through errors Value::Error { error, .. } => return Err(*error), _ => { @@ -119,7 +125,7 @@ the declaration may not be in scope. let Some(block) = engine_state.try_get_block(block_id) else { return Err(ShellError::GenericError { - error: format!("Unknown block ID: {block_id}"), + error: format!("Unknown block ID: {}", block_id.get()), msg: "ensure the block ID is correct and try again".into(), span: Some(target.span()), help: None, diff --git a/crates/nu-command/src/env/source_env.rs b/crates/nu-command/src/env/source_env.rs index 9768c4619..348418c24 100644 --- a/crates/nu-command/src/env/source_env.rs +++ b/crates/nu-command/src/env/source_env.rs @@ -2,7 +2,7 @@ use nu_engine::{ command_prelude::*, find_in_dirs_env, get_dirs_var_from_call, get_eval_block_with_early_return, redirect_env, }; -use nu_protocol::engine::CommandType; +use nu_protocol::{engine::CommandType, BlockId}; use std::path::PathBuf; /// Source a file for environment variables. @@ -50,6 +50,7 @@ impl Command for SourceEnv { // Note: this hidden positional is the block_id that corresponded to the 0th position // it is put here by the parser let block_id: i64 = call.req_parser_info(engine_state, caller_stack, "block_id")?; + let block_id = BlockId::new(block_id as usize); // Set the currently evaluated directory (file-relative PWD) let file_path = if let Some(path) = find_in_dirs_env( @@ -78,7 +79,7 @@ impl Command for SourceEnv { ); // Evaluate the block - let block = engine_state.get_block(block_id as usize).clone(); + let block = engine_state.get_block(block_id).clone(); let mut callee_stack = caller_stack .gather_captures(engine_state, &block.captures) .reset_pipes(); diff --git a/crates/nu-command/src/formats/to/text.rs b/crates/nu-command/src/formats/to/text.rs index 21b543d0e..b1c1fdb1b 100644 --- a/crates/nu-command/src/formats/to/text.rs +++ b/crates/nu-command/src/formats/to/text.rs @@ -118,7 +118,7 @@ fn local_into_string(value: Value, separator: &str, config: &Config) -> String { .map(|(x, y)| format!("{}: {}", x, local_into_string(y, ", ", config))) .collect::<Vec<_>>() .join(separator), - Value::Closure { val, .. } => format!("<Closure {}>", val.block_id), + Value::Closure { val, .. } => format!("<Closure {}>", val.block_id.get()), Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), diff --git a/crates/nu-command/src/misc/source.rs b/crates/nu-command/src/misc/source.rs index df002c998..01f971a10 100644 --- a/crates/nu-command/src/misc/source.rs +++ b/crates/nu-command/src/misc/source.rs @@ -1,5 +1,5 @@ use nu_engine::{command_prelude::*, get_eval_block_with_early_return}; -use nu_protocol::engine::CommandType; +use nu_protocol::{engine::CommandType, BlockId}; /// Source a file for environment variables. #[derive(Clone)] @@ -44,7 +44,8 @@ impl Command for Source { // Note: this hidden positional is the block_id that corresponded to the 0th position // it is put here by the parser let block_id: i64 = call.req_parser_info(engine_state, stack, "block_id")?; - let block = engine_state.get_block(block_id as usize).clone(); + let block_id = BlockId::new(block_id as usize); + let block = engine_state.get_block(block_id).clone(); let eval_block_with_early_return = get_eval_block_with_early_return(engine_state); diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index f0effb9a3..c365d360c 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -10,8 +10,8 @@ use nu_protocol::{ debugger::DebugContext, engine::{Closure, EngineState, Redirection, Stack, StateWorkingSet}, eval_base::Eval, - ByteStreamSource, Config, DataSource, FromValue, IntoPipelineData, OutDest, PipelineData, - PipelineMetadata, ShellError, Span, Spanned, Type, Value, VarId, ENV_VARIABLE_ID, + BlockId, ByteStreamSource, Config, DataSource, FromValue, IntoPipelineData, OutDest, + PipelineData, PipelineMetadata, ShellError, Span, Spanned, Type, Value, VarId, ENV_VARIABLE_ID, }; use nu_utils::IgnoreCaseExt; use std::{fs::OpenOptions, path::PathBuf, sync::Arc}; @@ -752,7 +752,7 @@ impl Eval for EvalRuntime { fn eval_subexpression<D: DebugContext>( engine_state: &EngineState, stack: &mut Stack, - block_id: usize, + block_id: BlockId, span: Span, ) -> Result<Value, ShellError> { let block = engine_state.get_block(block_id); @@ -899,7 +899,7 @@ impl Eval for EvalRuntime { fn eval_row_condition_or_closure( engine_state: &EngineState, stack: &mut Stack, - block_id: usize, + block_id: BlockId, span: Span, ) -> Result<Value, ShellError> { let captures = engine_state diff --git a/crates/nu-engine/src/scope.rs b/crates/nu-engine/src/scope.rs index d9650d2c2..bbc44f518 100644 --- a/crates/nu-engine/src/scope.rs +++ b/crates/nu-engine/src/scope.rs @@ -1,16 +1,16 @@ use nu_protocol::{ ast::Expr, engine::{Command, EngineState, Stack, Visibility}, - record, ModuleId, Signature, Span, SyntaxShape, Type, Value, + record, DeclId, ModuleId, Signature, Span, SyntaxShape, Type, Value, VarId, }; use std::{cmp::Ordering, collections::HashMap}; pub struct ScopeData<'e, 's> { engine_state: &'e EngineState, stack: &'s Stack, - vars_map: HashMap<&'e Vec<u8>, &'e usize>, - decls_map: HashMap<&'e Vec<u8>, &'e usize>, - modules_map: HashMap<&'e Vec<u8>, &'e usize>, + vars_map: HashMap<&'e Vec<u8>, &'e VarId>, + decls_map: HashMap<&'e Vec<u8>, &'e DeclId>, + modules_map: HashMap<&'e Vec<u8>, &'e ModuleId>, visibility: Visibility, } @@ -62,7 +62,7 @@ impl<'e, 's> ScopeData<'e, 's> { Value::nothing(span) }; - let var_id_val = Value::int(**var_id as i64, span); + let var_id_val = Value::int(var_id.get() as i64, span); vars.push(Value::record( record! { @@ -116,7 +116,7 @@ impl<'e, 's> ScopeData<'e, 's> { "creates_scope" => Value::bool(signature.creates_scope, span), "extra_description" => Value::string(decl.extra_description(), span), "search_terms" => Value::string(decl.search_terms().join(", "), span), - "decl_id" => Value::int(**decl_id as i64, span), + "decl_id" => Value::int(decl_id.get() as i64, span), }; commands.push(Value::record(record, span)) @@ -333,7 +333,7 @@ impl<'e, 's> ScopeData<'e, 's> { let record = record! { "name" => Value::string(String::from_utf8_lossy(command_name), span), "description" => Value::string(decl.description(), span), - "decl_id" => Value::int(**decl_id as i64, span), + "decl_id" => Value::int(decl_id.get() as i64, span), }; externals.push(Value::record(record, span)) @@ -353,7 +353,7 @@ impl<'e, 's> ScopeData<'e, 's> { if let Some(alias) = decl.as_alias() { let aliased_decl_id = if let Expr::Call(wrapped_call) = &alias.wrapped_call.expr { - Value::int(wrapped_call.decl_id as i64, span) + Value::int(wrapped_call.decl_id.get() as i64, span) } else { Value::nothing(span) }; @@ -367,7 +367,7 @@ impl<'e, 's> ScopeData<'e, 's> { "name" => Value::string(String::from_utf8_lossy(&decl_name), span), "expansion" => Value::string(expansion, span), "description" => Value::string(alias.description(), span), - "decl_id" => Value::int(decl_id as i64, span), + "decl_id" => Value::int(decl_id.get() as i64, span), "aliased_decl_id" => aliased_decl_id, }, span, @@ -395,7 +395,7 @@ impl<'e, 's> ScopeData<'e, 's> { Some(Value::record( record! { "name" => Value::string(String::from_utf8_lossy(name_bytes), span), - "decl_id" => Value::int(*decl_id as i64, span), + "decl_id" => Value::int(decl_id.get() as i64, span), }, span, )) @@ -414,7 +414,7 @@ impl<'e, 's> ScopeData<'e, 's> { Some(Value::record( record! { "name" => Value::string(String::from_utf8_lossy(name_bytes), span), - "decl_id" => Value::int(*decl_id as i64, span), + "decl_id" => Value::int(decl_id.get() as i64, span), }, span, )) @@ -433,7 +433,7 @@ impl<'e, 's> ScopeData<'e, 's> { Some(Value::record( record! { "name" => Value::string(String::from_utf8_lossy(name_bytes), span), - "decl_id" => Value::int(*decl_id as i64, span), + "decl_id" => Value::int(decl_id.get() as i64, span), }, span, )) @@ -457,7 +457,7 @@ impl<'e, 's> ScopeData<'e, 's> { record! { "name" => Value::string(String::from_utf8_lossy(name_bytes), span), "type" => Value::string(self.engine_state.get_var(*var_id).ty.to_string(), span), - "var_id" => Value::int(*var_id as i64, span), + "var_id" => Value::int(var_id.get() as i64, span), }, span, ) @@ -486,7 +486,7 @@ impl<'e, 's> ScopeData<'e, 's> { "has_env_block" => Value::bool(module.env_block.is_some(), span), "description" => Value::string(module_desc, span), "extra_description" => Value::string(module_extra_desc, span), - "module_id" => Value::int(*module_id as i64, span), + "module_id" => Value::int(module_id.get() as i64, span), }, span, ) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index b75a57a28..8ca5249bd 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -3528,7 +3528,7 @@ pub fn parse_source(working_set: &mut StateWorkingSet, lite_command: &LiteComman "block_id".to_string(), Expression::new( working_set, - Expr::Int(block_id as i64), + Expr::Int(block_id.get() as i64), spans[1], Type::Any, ), diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 909fd75d6..4ac161bb8 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -11,7 +11,7 @@ use itertools::Itertools; use log::trace; use nu_engine::DIR_VAR_PARSER_INFO; use nu_protocol::{ - ast::*, engine::StateWorkingSet, eval_const::eval_constant, BlockId, DidYouMean, Flag, + ast::*, engine::StateWorkingSet, eval_const::eval_constant, BlockId, DeclId, DidYouMean, Flag, ParseError, PositionalArg, Signature, Span, Spanned, SyntaxShape, Type, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID, }; @@ -921,9 +921,9 @@ pub fn parse_internal_call( working_set: &mut StateWorkingSet, command_span: Span, spans: &[Span], - decl_id: usize, + decl_id: DeclId, ) -> ParsedInternalCall { - trace!("parsing: internal call (decl id: {})", decl_id); + trace!("parsing: internal call (decl id: {})", decl_id.get()); let mut call = Call::new(command_span); call.decl_id = decl_id; @@ -6583,6 +6583,7 @@ pub fn parse( let mut errors = vec![]; for (block_idx, block) in working_set.delta.blocks.iter().enumerate() { let block_id = block_idx + working_set.permanent_state.num_blocks(); + let block_id = BlockId::new(block_id); if !seen_blocks.contains_key(&block_id) { let mut captures = vec![]; @@ -6625,7 +6626,7 @@ pub fn parse( // already saved in permanent state if !captures.is_empty() && block_captures_empty - && block_id >= working_set.permanent_state.num_blocks() + && block_id.get() >= working_set.permanent_state.num_blocks() { let block = working_set.get_block_mut(block_id); block.captures = captures.into_iter().map(|(var_id, _)| var_id).collect(); diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 86511bdca..b26ae20db 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -2,7 +2,7 @@ use nu_parser::*; use nu_protocol::{ ast::{Argument, Expr, Expression, ExternalArgument, PathMember, Range}, engine::{Call, Command, EngineState, Stack, StateWorkingSet}, - Category, ParseError, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, + Category, DeclId, ParseError, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, }; use rstest::rstest; @@ -607,7 +607,7 @@ pub fn parse_call() { assert!(element.redirection.is_none()); if let Expr::Call(call) = &element.expr.expr { - assert_eq!(call.decl_id, 0); + assert_eq!(call.decl_id, DeclId::new(0)); } } @@ -661,7 +661,7 @@ pub fn parse_call_short_flag_batch_arg_allowed() { assert!(element.redirection.is_none()); if let Expr::Call(call) = &element.expr.expr { - assert_eq!(call.decl_id, 0); + assert_eq!(call.decl_id, DeclId::new(0)); assert_eq!(call.arguments.len(), 2); matches!(call.arguments[0], Argument::Named((_, None, None))); matches!(call.arguments[1], Argument::Named((_, None, Some(_)))); diff --git a/crates/nu-plugin-core/src/util/with_custom_values_in.rs b/crates/nu-plugin-core/src/util/with_custom_values_in.rs index 8fcd843ac..d5f710256 100644 --- a/crates/nu-plugin-core/src/util/with_custom_values_in.rs +++ b/crates/nu-plugin-core/src/util/with_custom_values_in.rs @@ -1,5 +1,8 @@ use nu_protocol::{CustomValue, IntoSpanned, ShellError, Spanned, Value}; +#[allow(unused_imports)] // both are definitely used +use nu_protocol::{BlockId, VarId}; + /// Do something with all [`CustomValue`]s recursively within a `Value`. This is not limited to /// plugin custom values. pub fn with_custom_values_in<E>( @@ -36,8 +39,8 @@ fn find_custom_values() { ]), "closure" => Value::test_closure( Closure { - block_id: 0, - captures: vec![(0, cv.clone()), (1, Value::test_string("foo"))] + block_id: BlockId::new(0), + captures: vec![(VarId::new(0), cv.clone()), (VarId::new(1), Value::test_string("foo"))] } ), }); diff --git a/crates/nu-plugin-engine/src/context.rs b/crates/nu-plugin-engine/src/context.rs index 6b6b1a15f..4a6af0b87 100644 --- a/crates/nu-plugin-engine/src/context.rs +++ b/crates/nu-plugin-engine/src/context.rs @@ -177,7 +177,7 @@ impl<'a> PluginExecutionContext for PluginExecutionCommandContext<'a> { error: "Plugin misbehaving".into(), msg: format!( "Tried to evaluate unknown block id: {}", - closure.item.block_id + closure.item.block_id.get() ), span: Some(closure.span), help: None, @@ -226,10 +226,10 @@ impl<'a> PluginExecutionContext for PluginExecutionCommandContext<'a> { redirect_stdout: bool, redirect_stderr: bool, ) -> Result<PipelineData, ShellError> { - if decl_id >= self.engine_state.num_decls() { + if decl_id.get() >= self.engine_state.num_decls() { return Err(ShellError::GenericError { error: "Plugin misbehaving".into(), - msg: format!("Tried to call unknown decl id: {}", decl_id), + msg: format!("Tried to call unknown decl id: {}", decl_id.get()), span: Some(call.head), help: None, inner: vec![], diff --git a/crates/nu-plugin-engine/src/interface/tests.rs b/crates/nu-plugin-engine/src/interface/tests.rs index 31828973d..5058a4569 100644 --- a/crates/nu-plugin-engine/src/interface/tests.rs +++ b/crates/nu-plugin-engine/src/interface/tests.rs @@ -17,7 +17,7 @@ use nu_plugin_protocol::{ use nu_protocol::{ ast::{Math, Operator}, engine::Closure, - ByteStreamType, CustomValue, DataSource, IntoInterruptiblePipelineData, IntoSpanned, + BlockId, ByteStreamType, CustomValue, DataSource, IntoInterruptiblePipelineData, IntoSpanned, PipelineData, PipelineMetadata, PluginMetadata, PluginSignature, ShellError, Signals, Span, Spanned, Value, }; @@ -431,7 +431,7 @@ fn manager_consume_engine_call_forwards_to_subscriber_with_pipeline_data() -> Re call: EngineCall::EvalClosure { closure: Spanned { item: Closure { - block_id: 0, + block_id: BlockId::new(0), captures: vec![], }, span: Span::test_data(), diff --git a/crates/nu-plugin-engine/src/plugin_custom_value_with_source/tests.rs b/crates/nu-plugin-engine/src/plugin_custom_value_with_source/tests.rs index fd8a81a56..0c6e33c19 100644 --- a/crates/nu-plugin-engine/src/plugin_custom_value_with_source/tests.rs +++ b/crates/nu-plugin-engine/src/plugin_custom_value_with_source/tests.rs @@ -1,7 +1,9 @@ use std::sync::Arc; use nu_plugin_protocol::test_util::{test_plugin_custom_value, TestCustomValue}; -use nu_protocol::{engine::Closure, record, CustomValue, IntoSpanned, ShellError, Span, Value}; +use nu_protocol::{ + engine::Closure, record, BlockId, CustomValue, IntoSpanned, ShellError, Span, Value, VarId, +}; use crate::{ test_util::test_plugin_custom_value_with_source, PluginCustomValueWithSource, PluginSource, @@ -132,8 +134,11 @@ fn check_closure_custom_values( fn add_source_in_nested_closure() -> Result<(), ShellError> { let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value())); let mut val = Value::test_closure(Closure { - block_id: 0, - captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())], + block_id: BlockId::new(0), + captures: vec![ + (VarId::new(0), orig_custom_val.clone()), + (VarId::new(1), orig_custom_val.clone()), + ], }); let source = Arc::new(PluginSource::new_fake("foo")); PluginCustomValueWithSource::add_source_in(&mut val, &source)?; diff --git a/crates/nu-plugin-protocol/src/lib.rs b/crates/nu-plugin-protocol/src/lib.rs index 0473b2c49..ef574456a 100644 --- a/crates/nu-plugin-protocol/src/lib.rs +++ b/crates/nu-plugin-protocol/src/lib.rs @@ -627,7 +627,7 @@ pub enum EngineCallResponse<D> { PipelineData(D), Config(SharedCow<Config>), ValueMap(HashMap<String, Value>), - Identifier(usize), + Identifier(DeclId), } impl<D> EngineCallResponse<D> { diff --git a/crates/nu-plugin-protocol/src/plugin_custom_value/tests.rs b/crates/nu-plugin-protocol/src/plugin_custom_value/tests.rs index 5969451d8..a93e75872 100644 --- a/crates/nu-plugin-protocol/src/plugin_custom_value/tests.rs +++ b/crates/nu-plugin-protocol/src/plugin_custom_value/tests.rs @@ -1,7 +1,7 @@ use crate::test_util::{expected_test_custom_value, test_plugin_custom_value, TestCustomValue}; use super::PluginCustomValue; -use nu_protocol::{engine::Closure, record, CustomValue, ShellError, Span, Value}; +use nu_protocol::{engine::Closure, record, BlockId, CustomValue, ShellError, Span, Value, VarId}; fn check_record_custom_values( val: &Value, @@ -156,8 +156,11 @@ fn serialize_in_list() -> Result<(), ShellError> { fn serialize_in_closure() -> Result<(), ShellError> { let orig_custom_val = Value::test_custom_value(Box::new(TestCustomValue(24))); let mut val = Value::test_closure(Closure { - block_id: 0, - captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())], + block_id: BlockId::new(0), + captures: vec![ + (VarId::new(0), orig_custom_val.clone()), + (VarId::new(1), orig_custom_val.clone()), + ], }); PluginCustomValue::serialize_custom_values_in(&mut val)?; @@ -239,8 +242,11 @@ fn deserialize_in_list() -> Result<(), ShellError> { fn deserialize_in_closure() -> Result<(), ShellError> { let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value())); let mut val = Value::test_closure(Closure { - block_id: 0, - captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())], + block_id: BlockId::new(0), + captures: vec![ + (VarId::new(0), orig_custom_val.clone()), + (VarId::new(1), orig_custom_val.clone()), + ], }); PluginCustomValue::deserialize_custom_values_in(&mut val)?; diff --git a/crates/nu-plugin/src/plugin/interface/tests.rs b/crates/nu-plugin/src/plugin/interface/tests.rs index f8002b2c6..5ff9f6f6c 100644 --- a/crates/nu-plugin/src/plugin/interface/tests.rs +++ b/crates/nu-plugin/src/plugin/interface/tests.rs @@ -9,8 +9,8 @@ use nu_plugin_protocol::{ PluginCustomValue, PluginInput, PluginOutput, Protocol, ProtocolInfo, StreamData, }; use nu_protocol::{ - engine::Closure, ByteStreamType, Config, CustomValue, IntoInterruptiblePipelineData, - LabeledError, PipelineData, PluginSignature, ShellError, Signals, Span, Spanned, Value, + engine::Closure, BlockId, ByteStreamType, Config, CustomValue, IntoInterruptiblePipelineData, + LabeledError, PipelineData, PluginSignature, ShellError, Signals, Span, Spanned, Value, VarId, }; use std::{ collections::HashMap, @@ -1040,8 +1040,8 @@ fn interface_eval_closure_with_stream() -> Result<(), ShellError> { .eval_closure_with_stream( &Spanned { item: Closure { - block_id: 42, - captures: vec![(0, Value::test_int(5))], + block_id: BlockId::new(42), + captures: vec![(VarId::new(0), Value::test_int(5))], }, span: Span::test_data(), }, @@ -1064,10 +1064,14 @@ fn interface_eval_closure_with_stream() -> Result<(), ShellError> { redirect_stdout, redirect_stderr, } => { - assert_eq!(42, closure.item.block_id, "closure.item.block_id"); + assert_eq!( + BlockId::new(42), + closure.item.block_id, + "closure.item.block_id" + ); assert_eq!(1, closure.item.captures.len(), "closure.item.captures.len"); assert_eq!( - (0, Value::test_int(5)), + (VarId::new(0), Value::test_int(5)), closure.item.captures[0], "closure.item.captures[0]" ); diff --git a/crates/nu-protocol/src/ast/call.rs b/crates/nu-protocol/src/ast/call.rs index 0f7b25e21..0c42583c5 100644 --- a/crates/nu-protocol/src/ast/call.rs +++ b/crates/nu-protocol/src/ast/call.rs @@ -96,7 +96,7 @@ pub struct Call { impl Call { pub fn new(head: Span) -> Call { Self { - decl_id: 0, + decl_id: DeclId::new(0), head, arguments: vec![], parser_info: HashMap::new(), diff --git a/crates/nu-protocol/src/ast/expr.rs b/crates/nu-protocol/src/ast/expr.rs index d6386f92d..62a2f3ad3 100644 --- a/crates/nu-protocol/src/ast/expr.rs +++ b/crates/nu-protocol/src/ast/expr.rs @@ -6,7 +6,7 @@ use super::{ Range, Table, ValueWithUnit, }; use crate::{ - ast::ImportPattern, engine::StateWorkingSet, BlockId, OutDest, Signature, Span, VarId, + ast::ImportPattern, engine::StateWorkingSet, BlockId, ModuleId, OutDest, Signature, Span, VarId, }; /// An [`Expression`] AST node @@ -47,7 +47,7 @@ pub enum Expr { CellPath(CellPath), FullCellPath(Box<FullCellPath>), ImportPattern(Box<ImportPattern>), - Overlay(Option<BlockId>), // block ID of the overlay's origin module + Overlay(Option<ModuleId>), Signature(Box<Signature>), StringInterpolation(Vec<Expression>), /// The boolean is `true` if the string is quoted. diff --git a/crates/nu-protocol/src/engine/call.rs b/crates/nu-protocol/src/engine/call.rs index 741e2bd87..eebf4860c 100644 --- a/crates/nu-protocol/src/engine/call.rs +++ b/crates/nu-protocol/src/engine/call.rs @@ -31,7 +31,7 @@ impl Call<'_> { // anyway. Call { head: span, - decl_id: 0, + decl_id: DeclId::new(0), inner: CallImpl::AstBox(Box::new(ast::Call::new(span))), } } diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 6d7aff120..28463f752 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -93,7 +93,7 @@ pub struct EngineState { pub config: Arc<Config>, pub pipeline_externals_state: Arc<(AtomicU32, AtomicU32)>, pub repl_state: Arc<Mutex<ReplState>>, - pub table_decl_id: Option<usize>, + pub table_decl_id: Option<DeclId>, #[cfg(feature = "plugin")] pub plugin_path: Option<PathBuf>, #[cfg(feature = "plugin")] @@ -114,9 +114,9 @@ pub struct EngineState { // The max number of compiled regexes to keep around in a LRU cache, arbitrarily chosen const REGEX_CACHE_SIZE: usize = 100; // must be nonzero, otherwise will panic -pub const NU_VARIABLE_ID: usize = 0; -pub const IN_VARIABLE_ID: usize = 1; -pub const ENV_VARIABLE_ID: usize = 2; +pub const NU_VARIABLE_ID: VarId = VarId::new(0); +pub const IN_VARIABLE_ID: VarId = VarId::new(1); +pub const ENV_VARIABLE_ID: VarId = VarId::new(2); // NOTE: If you add more to this list, make sure to update the > checks based on the last in the list // The first span is unknown span @@ -144,7 +144,7 @@ impl EngineState { // make sure we have some default overlay: scope: ScopeFrame::with_empty_overlay( DEFAULT_OVERLAY_NAME.as_bytes().to_vec(), - 0, + ModuleId::new(0), false, ), signal_handlers: None, @@ -380,7 +380,7 @@ impl EngineState { let other_names = other.active_overlays.iter().map(|other_id| { &other .overlays - .get(*other_id) + .get(other_id.get()) .expect("internal error: missing overlay") .0 }); @@ -410,7 +410,7 @@ impl EngineState { &self .scope .overlays - .get(overlay_id) + .get(overlay_id.get()) .expect("internal error: missing overlay") .0 } @@ -419,7 +419,7 @@ impl EngineState { &self .scope .overlays - .get(overlay_id) + .get(overlay_id.get()) .expect("internal error: missing overlay") .1 } @@ -763,7 +763,7 @@ impl EngineState { pub fn get_var(&self, var_id: VarId) -> &Variable { self.vars - .get(var_id) + .get(var_id.get()) .expect("internal error: missing variable") } @@ -773,12 +773,12 @@ impl EngineState { } pub fn generate_nu_constant(&mut self) { - self.vars[NU_VARIABLE_ID].const_val = Some(create_nu_constant(self, Span::unknown())); + self.vars[NU_VARIABLE_ID.get()].const_val = Some(create_nu_constant(self, Span::unknown())); } pub fn get_decl(&self, decl_id: DeclId) -> &dyn Command { self.decls - .get(decl_id) + .get(decl_id.get()) .expect("internal error: missing declaration") .as_ref() } @@ -810,7 +810,7 @@ impl EngineState { pub fn get_signature(&self, decl: &dyn Command) -> Signature { if let Some(block_id) = decl.block_id() { - *self.blocks[block_id].signature.clone() + *self.blocks[block_id.get()].signature.clone() } else { decl.signature() } @@ -830,7 +830,7 @@ impl EngineState { pub fn get_block(&self, block_id: BlockId) -> &Arc<Block> { self.blocks - .get(block_id) + .get(block_id.get()) .expect("internal error: missing block") } @@ -840,18 +840,18 @@ impl EngineState { /// are normally a compiler error. This only exists to stop plugins from crashing the engine if /// they send us something invalid. pub fn try_get_block(&self, block_id: BlockId) -> Option<&Arc<Block>> { - self.blocks.get(block_id) + self.blocks.get(block_id.get()) } pub fn get_module(&self, module_id: ModuleId) -> &Module { self.modules - .get(module_id) + .get(module_id.get()) .expect("internal error: missing module") } pub fn get_virtual_path(&self, virtual_path_id: VirtualPathId) -> &(String, VirtualPath) { self.virtual_paths - .get(virtual_path_id) + .get(virtual_path_id.get()) .expect("internal error: missing virtual path") } @@ -879,7 +879,7 @@ impl EngineState { covered_span, }); - self.num_files() - 1 + FileId::new(self.num_files() - 1) } pub fn set_config_path(&mut self, key: &str, val: PathBuf) { @@ -1065,7 +1065,7 @@ mod engine_state_tests { let mut engine_state = StateWorkingSet::new(&engine_state); let id = engine_state.add_file("test.nu".into(), &[]); - assert_eq!(id, 0); + assert_eq!(id, FileId::new(0)); } #[test] @@ -1076,8 +1076,8 @@ mod engine_state_tests { let mut working_set = StateWorkingSet::new(&engine_state); let working_set_id = working_set.add_file("child.nu".into(), &[]); - assert_eq!(parent_id, 0); - assert_eq!(working_set_id, 1); + assert_eq!(parent_id, FileId::new(0)); + assert_eq!(working_set_id, FileId::new(1)); } #[test] diff --git a/crates/nu-protocol/src/engine/overlay.rs b/crates/nu-protocol/src/engine/overlay.rs index e396e30c2..9e52cffce 100644 --- a/crates/nu-protocol/src/engine/overlay.rs +++ b/crates/nu-protocol/src/engine/overlay.rs @@ -76,7 +76,7 @@ impl ScopeFrame { pub fn with_empty_overlay(name: Vec<u8>, origin: ModuleId, prefixed: bool) -> Self { Self { overlays: vec![(name, OverlayFrame::from_origin(origin, prefixed))], - active_overlays: vec![0], + active_overlays: vec![OverlayId::new(0)], removed_overlays: vec![], predecls: HashMap::new(), } @@ -86,7 +86,7 @@ impl ScopeFrame { for overlay_id in self.active_overlays.iter().rev() { if let Some(var_id) = self .overlays - .get(*overlay_id) + .get(overlay_id.get()) .expect("internal error: missing overlay") .1 .vars @@ -139,7 +139,7 @@ impl ScopeFrame { pub fn get_overlay_name(&self, overlay_id: OverlayId) -> &[u8] { &self .overlays - .get(overlay_id) + .get(overlay_id.get()) .expect("internal error: missing overlay") .0 } @@ -147,7 +147,7 @@ impl ScopeFrame { pub fn get_overlay(&self, overlay_id: OverlayId) -> &OverlayFrame { &self .overlays - .get(overlay_id) + .get(overlay_id.get()) .expect("internal error: missing overlay") .1 } @@ -155,19 +155,23 @@ impl ScopeFrame { pub fn get_overlay_mut(&mut self, overlay_id: OverlayId) -> &mut OverlayFrame { &mut self .overlays - .get_mut(overlay_id) + .get_mut(overlay_id.get()) .expect("internal error: missing overlay") .1 } pub fn find_overlay(&self, name: &[u8]) -> Option<OverlayId> { - self.overlays.iter().position(|(n, _)| n == name) + self.overlays + .iter() + .position(|(n, _)| n == name) + .map(OverlayId::new) } pub fn find_active_overlay(&self, name: &[u8]) -> Option<OverlayId> { self.overlays .iter() .position(|(n, _)| n == name) + .map(OverlayId::new) .filter(|id| self.active_overlays.contains(id)) } } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 843315871..31c1175e7 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -734,7 +734,7 @@ impl Stack { mod test { use std::sync::Arc; - use crate::{engine::EngineState, Span, Value}; + use crate::{engine::EngineState, Span, Value, VarId}; use super::Stack; @@ -749,22 +749,25 @@ mod test { #[test] fn test_children_see_inner_values() { let mut original = Stack::new(); - original.add_var(0, string_value("hello")); + original.add_var(VarId::new(0), string_value("hello")); let cloned = Stack::with_parent(Arc::new(original)); - assert_eq!(cloned.get_var(0, ZERO_SPAN), Ok(string_value("hello"))); + assert_eq!( + cloned.get_var(VarId::new(0), ZERO_SPAN), + Ok(string_value("hello")) + ); } #[test] fn test_children_dont_see_deleted_values() { let mut original = Stack::new(); - original.add_var(0, string_value("hello")); + original.add_var(VarId::new(0), string_value("hello")); let mut cloned = Stack::with_parent(Arc::new(original)); - cloned.remove_var(0); + cloned.remove_var(VarId::new(0)); assert_eq!( - cloned.get_var(0, ZERO_SPAN), + cloned.get_var(VarId::new(0), ZERO_SPAN), Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) ); } @@ -772,60 +775,69 @@ mod test { #[test] fn test_children_changes_override_parent() { let mut original = Stack::new(); - original.add_var(0, string_value("hello")); + original.add_var(VarId::new(0), string_value("hello")); let mut cloned = Stack::with_parent(Arc::new(original)); - cloned.add_var(0, string_value("there")); - assert_eq!(cloned.get_var(0, ZERO_SPAN), Ok(string_value("there"))); + cloned.add_var(VarId::new(0), string_value("there")); + assert_eq!( + cloned.get_var(VarId::new(0), ZERO_SPAN), + Ok(string_value("there")) + ); - cloned.remove_var(0); + cloned.remove_var(VarId::new(0)); // the underlying value shouldn't magically re-appear assert_eq!( - cloned.get_var(0, ZERO_SPAN), + cloned.get_var(VarId::new(0), ZERO_SPAN), Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) ); } #[test] fn test_children_changes_persist_in_offspring() { let mut original = Stack::new(); - original.add_var(0, string_value("hello")); + original.add_var(VarId::new(0), string_value("hello")); let mut cloned = Stack::with_parent(Arc::new(original)); - cloned.add_var(1, string_value("there")); + cloned.add_var(VarId::new(1), string_value("there")); - cloned.remove_var(0); + cloned.remove_var(VarId::new(0)); let cloned = Stack::with_parent(Arc::new(cloned)); assert_eq!( - cloned.get_var(0, ZERO_SPAN), + cloned.get_var(VarId::new(0), ZERO_SPAN), Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) ); - assert_eq!(cloned.get_var(1, ZERO_SPAN), Ok(string_value("there"))); + assert_eq!( + cloned.get_var(VarId::new(1), ZERO_SPAN), + Ok(string_value("there")) + ); } #[test] fn test_merging_children_back_to_parent() { let mut original = Stack::new(); let engine_state = EngineState::new(); - original.add_var(0, string_value("hello")); + original.add_var(VarId::new(0), string_value("hello")); let original_arc = Arc::new(original); let mut cloned = Stack::with_parent(original_arc.clone()); - cloned.add_var(1, string_value("there")); + cloned.add_var(VarId::new(1), string_value("there")); - cloned.remove_var(0); + cloned.remove_var(VarId::new(0)); cloned.add_env_var("ADDED_IN_CHILD".to_string(), string_value("New Env Var")); let original = Stack::with_changes_from_child(original_arc, cloned); assert_eq!( - original.get_var(0, ZERO_SPAN), + original.get_var(VarId::new(0), ZERO_SPAN), Err(crate::ShellError::VariableNotFoundAtRuntime { span: ZERO_SPAN }) ); - assert_eq!(original.get_var(1, ZERO_SPAN), Ok(string_value("there"))); + assert_eq!( + original.get_var(VarId::new(1), ZERO_SPAN), + Ok(string_value("there")) + ); assert_eq!( original.get_env_var(&engine_state, "ADDED_IN_CHILD"), diff --git a/crates/nu-protocol/src/engine/state_delta.rs b/crates/nu-protocol/src/engine/state_delta.rs index 033867e4c..0204dbe7f 100644 --- a/crates/nu-protocol/src/engine/state_delta.rs +++ b/crates/nu-protocol/src/engine/state_delta.rs @@ -98,7 +98,7 @@ impl StateDelta { Some( &mut last_scope .overlays - .get_mut(*last_overlay_id) + .get_mut(last_overlay_id.get()) .expect("internal error: missing required overlay") .1, ) @@ -117,7 +117,7 @@ impl StateDelta { Some( &last_scope .overlays - .get(*last_overlay_id) + .get(last_overlay_id.get()) .expect("internal error: missing required overlay") .1, ) diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs index eb60f9df8..d35318c84 100644 --- a/crates/nu-protocol/src/engine/state_working_set.rs +++ b/crates/nu-protocol/src/engine/state_working_set.rs @@ -4,8 +4,8 @@ use crate::{ description::build_desc, CachedFile, Command, CommandType, EngineState, OverlayFrame, StateDelta, Variable, VirtualPath, Visibility, }, - BlockId, Category, CompileError, Config, DeclId, FileId, GetSpan, Module, ModuleId, ParseError, - ParseWarning, Signature, Span, SpanId, Type, Value, VarId, VirtualPathId, + BlockId, Category, CompileError, Config, DeclId, FileId, GetSpan, Module, ModuleId, OverlayId, + ParseError, ParseWarning, Signature, Span, SpanId, Type, Value, VarId, VirtualPathId, }; use core::panic; use std::{ @@ -92,7 +92,7 @@ impl<'a> StateWorkingSet<'a> { for overlay_id in scope_frame.active_overlays.iter().rev() { let (overlay_name, _) = scope_frame .overlays - .get(*overlay_id) + .get(overlay_id.get()) .expect("internal error: missing overlay"); names.insert(overlay_name); @@ -112,6 +112,7 @@ impl<'a> StateWorkingSet<'a> { self.delta.decls.push(decl); let decl_id = self.num_decls() - 1; + let decl_id = DeclId::new(decl_id); self.last_overlay_mut().insert_decl(name, decl_id); @@ -152,6 +153,7 @@ impl<'a> StateWorkingSet<'a> { self.delta.decls.push(decl); let decl_id = self.num_decls() - 1; + let decl_id = DeclId::new(decl_id); self.delta .last_scope_frame_mut() @@ -268,7 +270,7 @@ impl<'a> StateWorkingSet<'a> { self.delta.blocks.push(block); - self.num_blocks() - 1 + BlockId::new(self.num_blocks() - 1) } pub fn add_module(&mut self, name: &str, module: Module, comments: Vec<Span>) -> ModuleId { @@ -276,6 +278,7 @@ impl<'a> StateWorkingSet<'a> { self.delta.modules.push(Arc::new(module)); let module_id = self.num_modules() - 1; + let module_id = ModuleId::new(module_id); if !comments.is_empty() { self.delta @@ -314,7 +317,7 @@ impl<'a> StateWorkingSet<'a> { } pub fn get_contents_of_file(&self, file_id: FileId) -> Option<&[u8]> { - if let Some(cached_file) = self.permanent_state.get_file_contents().get(file_id) { + if let Some(cached_file) = self.permanent_state.get_file_contents().get(file_id.get()) { return Some(&cached_file.content); } // The index subtraction will not underflow, if we hit the permanent state first. @@ -322,7 +325,7 @@ impl<'a> StateWorkingSet<'a> { if let Some(cached_file) = self .delta .get_file_contents() - .get(file_id - self.permanent_state.num_files()) + .get(file_id.get() - self.permanent_state.num_files()) { return Some(&cached_file.content); } @@ -335,7 +338,7 @@ impl<'a> StateWorkingSet<'a> { // First, look for the file to see if we already have it for (idx, cached_file) in self.files().enumerate() { if *cached_file.name == filename && &*cached_file.content == contents { - return idx; + return FileId::new(idx); } } @@ -350,18 +353,19 @@ impl<'a> StateWorkingSet<'a> { covered_span, }); - self.num_files() - 1 + FileId::new(self.num_files() - 1) } #[must_use] pub fn add_virtual_path(&mut self, name: String, virtual_path: VirtualPath) -> VirtualPathId { self.delta.virtual_paths.push((name, virtual_path)); - self.num_virtual_paths() - 1 + VirtualPathId::new(self.num_virtual_paths() - 1) } pub fn get_span_for_filename(&self, filename: &str) -> Option<Span> { let file_id = self.files().position(|file| &*file.name == filename)?; + let file_id = FileId::new(file_id); Some(self.get_span_for_file(file_id)) } @@ -373,7 +377,7 @@ impl<'a> StateWorkingSet<'a> { pub fn get_span_for_file(&self, file_id: FileId) -> Span { let result = self .files() - .nth(file_id) + .nth(file_id.get()) .expect("internal error: could not find source for previously parsed file"); result.covered_span @@ -526,7 +530,7 @@ impl<'a> StateWorkingSet<'a> { pub fn next_var_id(&self) -> VarId { let num_permanent_vars = self.permanent_state.num_vars(); - num_permanent_vars + self.delta.vars.len() + VarId::new(num_permanent_vars + self.delta.vars.len()) } pub fn list_variables(&self) -> Vec<&[u8]> { @@ -635,40 +639,40 @@ impl<'a> StateWorkingSet<'a> { pub fn set_variable_type(&mut self, var_id: VarId, ty: Type) { let num_permanent_vars = self.permanent_state.num_vars(); - if var_id < num_permanent_vars { + if var_id.get() < num_permanent_vars { panic!("Internal error: attempted to set into permanent state from working set") } else { - self.delta.vars[var_id - num_permanent_vars].ty = ty; + self.delta.vars[var_id.get() - num_permanent_vars].ty = ty; } } pub fn set_variable_const_val(&mut self, var_id: VarId, val: Value) { let num_permanent_vars = self.permanent_state.num_vars(); - if var_id < num_permanent_vars { + if var_id.get() < num_permanent_vars { panic!("Internal error: attempted to set into permanent state from working set") } else { - self.delta.vars[var_id - num_permanent_vars].const_val = Some(val); + self.delta.vars[var_id.get() - num_permanent_vars].const_val = Some(val); } } pub fn get_variable(&self, var_id: VarId) -> &Variable { let num_permanent_vars = self.permanent_state.num_vars(); - if var_id < num_permanent_vars { + if var_id.get() < num_permanent_vars { self.permanent_state.get_var(var_id) } else { self.delta .vars - .get(var_id - num_permanent_vars) + .get(var_id.get() - num_permanent_vars) .expect("internal error: missing variable") } } pub fn get_variable_if_possible(&self, var_id: VarId) -> Option<&Variable> { let num_permanent_vars = self.permanent_state.num_vars(); - if var_id < num_permanent_vars { + if var_id.get() < num_permanent_vars { Some(self.permanent_state.get_var(var_id)) } else { - self.delta.vars.get(var_id - num_permanent_vars) + self.delta.vars.get(var_id.get() - num_permanent_vars) } } @@ -687,12 +691,12 @@ impl<'a> StateWorkingSet<'a> { pub fn get_decl(&self, decl_id: DeclId) -> &dyn Command { let num_permanent_decls = self.permanent_state.num_decls(); - if decl_id < num_permanent_decls { + if decl_id.get() < num_permanent_decls { self.permanent_state.get_decl(decl_id) } else { self.delta .decls - .get(decl_id - num_permanent_decls) + .get(decl_id.get() - num_permanent_decls) .expect("internal error: missing declaration") .as_ref() } @@ -700,12 +704,12 @@ impl<'a> StateWorkingSet<'a> { pub fn get_decl_mut(&mut self, decl_id: DeclId) -> &mut Box<dyn Command> { let num_permanent_decls = self.permanent_state.num_decls(); - if decl_id < num_permanent_decls { + if decl_id.get() < num_permanent_decls { panic!("internal error: can only mutate declarations in working set") } else { self.delta .decls - .get_mut(decl_id - num_permanent_decls) + .get_mut(decl_id.get() - num_permanent_decls) .expect("internal error: missing declaration") } } @@ -756,36 +760,36 @@ impl<'a> StateWorkingSet<'a> { pub fn get_block(&self, block_id: BlockId) -> &Arc<Block> { let num_permanent_blocks = self.permanent_state.num_blocks(); - if block_id < num_permanent_blocks { + if block_id.get() < num_permanent_blocks { self.permanent_state.get_block(block_id) } else { self.delta .blocks - .get(block_id - num_permanent_blocks) + .get(block_id.get() - num_permanent_blocks) .expect("internal error: missing block") } } pub fn get_module(&self, module_id: ModuleId) -> &Module { let num_permanent_modules = self.permanent_state.num_modules(); - if module_id < num_permanent_modules { + if module_id.get() < num_permanent_modules { self.permanent_state.get_module(module_id) } else { self.delta .modules - .get(module_id - num_permanent_modules) + .get(module_id.get() - num_permanent_modules) .expect("internal error: missing module") } } pub fn get_block_mut(&mut self, block_id: BlockId) -> &mut Block { let num_permanent_blocks = self.permanent_state.num_blocks(); - if block_id < num_permanent_blocks { + if block_id.get() < num_permanent_blocks { panic!("Attempt to mutate a block that is in the permanent (immutable) state") } else { self.delta .blocks - .get_mut(block_id - num_permanent_blocks) + .get_mut(block_id.get() - num_permanent_blocks) .map(Arc::make_mut) .expect("internal error: missing block") } @@ -912,7 +916,7 @@ impl<'a> StateWorkingSet<'a> { last_scope_frame .overlays .push((name, OverlayFrame::from_origin(origin, prefixed))); - last_scope_frame.overlays.len() - 1 + OverlayId::new(last_scope_frame.overlays.len() - 1) }; last_scope_frame @@ -989,13 +993,13 @@ impl<'a> StateWorkingSet<'a> { pub fn find_module_by_span(&self, span: Span) -> Option<ModuleId> { for (id, module) in self.delta.modules.iter().enumerate() { if Some(span) == module.span { - return Some(self.permanent_state.num_modules() + id); + return Some(ModuleId::new(self.permanent_state.num_modules() + id)); } } for (module_id, module) in self.permanent_state.modules.iter().enumerate() { if Some(span) == module.span { - return Some(module_id); + return Some(ModuleId::new(module_id)); } } @@ -1020,12 +1024,12 @@ impl<'a> StateWorkingSet<'a> { pub fn get_virtual_path(&self, virtual_path_id: VirtualPathId) -> &(String, VirtualPath) { let num_permanent_virtual_paths = self.permanent_state.num_virtual_paths(); - if virtual_path_id < num_permanent_virtual_paths { + if virtual_path_id.get() < num_permanent_virtual_paths { self.permanent_state.get_virtual_path(virtual_path_id) } else { self.delta .virtual_paths - .get(virtual_path_id - num_permanent_virtual_paths) + .get(virtual_path_id.get() - num_permanent_virtual_paths) .expect("internal error: missing virtual path") } } diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index 933342f57..12070f9ca 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -5,7 +5,7 @@ use crate::{ ExternalArgument, ListItem, Math, Operator, RecordItem, }, debugger::DebugContext, - Config, GetSpan, Range, Record, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, + BlockId, Config, GetSpan, Range, Record, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, }; use std::{collections::HashMap, sync::Arc}; @@ -369,7 +369,7 @@ pub trait Eval { fn eval_subexpression<D: DebugContext>( state: Self::State<'_>, mut_state: &mut Self::MutState, - block_id: usize, + block_id: BlockId, span: Span, ) -> Result<Value, ShellError>; @@ -396,7 +396,7 @@ pub trait Eval { fn eval_row_condition_or_closure( state: Self::State<'_>, mut_state: &mut Self::MutState, - block_id: usize, + block_id: BlockId, span: Span, ) -> Result<Value, ShellError>; diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 72f165d19..26750b867 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -7,7 +7,8 @@ use crate::{ debugger::{DebugContext, WithoutDebug}, engine::{EngineState, StateWorkingSet}, eval_base::Eval, - record, Config, HistoryFileFormat, PipelineData, Record, ShellError, Span, Value, VarId, + record, BlockId, Config, HistoryFileFormat, PipelineData, Record, ShellError, Span, Value, + VarId, }; use nu_system::os_info::{get_kernel_version, get_os_arch, get_os_family, get_os_name}; use std::{ @@ -481,7 +482,7 @@ impl Eval for EvalConst { fn eval_subexpression<D: DebugContext>( working_set: &StateWorkingSet, _: &mut (), - block_id: usize, + block_id: BlockId, span: Span, ) -> Result<Value, ShellError> { // TODO: Allow debugging const eval @@ -516,7 +517,7 @@ impl Eval for EvalConst { fn eval_row_condition_or_closure( _: &StateWorkingSet, _: &mut (), - _: usize, + _: BlockId, span: Span, ) -> Result<Value, ShellError> { Err(ShellError::NotAConstant { span }) diff --git a/crates/nu-protocol/src/id.rs b/crates/nu-protocol/src/id.rs index 829ee8f36..84600e3c0 100644 --- a/crates/nu-protocol/src/id.rs +++ b/crates/nu-protocol/src/id.rs @@ -1,12 +1,90 @@ +use std::any; +use std::fmt::{Debug, Error, Formatter}; +use std::marker::PhantomData; + use serde::{Deserialize, Serialize}; -pub type VarId = usize; -pub type DeclId = usize; -pub type BlockId = usize; -pub type ModuleId = usize; -pub type OverlayId = usize; -pub type FileId = usize; -pub type VirtualPathId = usize; +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct Id<T> { + inner: usize, + _phantom: PhantomData<T>, +} + +impl<T> Id<T> { + /// Creates a new `Id`. + /// + /// Using a distinct type like `Id` instead of `usize` helps us avoid mixing plain integers + /// with identifiers. + #[inline] + pub const fn new(inner: usize) -> Self { + Self { + inner, + _phantom: PhantomData, + } + } + + /// Returns the inner `usize` value. + /// + /// This requires an explicit call, ensuring we only use the raw value when intended. + #[inline] + pub const fn get(self) -> usize { + self.inner + } +} + +impl<T> Debug for Id<T> { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { + let marker = any::type_name::<T>().split("::").last().expect("not empty"); + write!(f, "{marker}Id({})", self.inner) + } +} + +impl<T> Serialize for Id<T> { + fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> + where + S: serde::Serializer, + { + self.inner.serialize(serializer) + } +} + +impl<'de, T> Deserialize<'de> for Id<T> { + fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> + where + D: serde::Deserializer<'de>, + { + let inner = usize::deserialize(deserializer)?; + Ok(Self { + inner, + _phantom: PhantomData, + }) + } +} + +pub mod marker { + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct Var; + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct Decl; + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct Block; + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct Module; + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct Overlay; + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct File; + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct VirtualPath; +} + +pub type VarId = Id<marker::Var>; +pub type DeclId = Id<marker::Decl>; +pub type BlockId = Id<marker::Block>; +pub type ModuleId = Id<marker::Module>; +pub type OverlayId = Id<marker::Overlay>; +pub type FileId = Id<marker::File>; +pub type VirtualPathId = Id<marker::VirtualPath>; #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub struct SpanId(pub usize); // more robust ID style used in the new parser diff --git a/crates/nu-protocol/src/ir/display.rs b/crates/nu-protocol/src/ir/display.rs index 0168306f8..68ef9ebd7 100644 --- a/crates/nu-protocol/src/ir/display.rs +++ b/crates/nu-protocol/src/ir/display.rs @@ -276,11 +276,11 @@ impl<'a> FmtDecl<'a> { impl fmt::Display for FmtDecl<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "decl {} {:?}", self.0, self.1) + write!(f, "decl {} {:?}", self.0.get(), self.1) } } -struct FmtVar<'a>(DeclId, Option<&'a str>); +struct FmtVar<'a>(VarId, Option<&'a str>); impl<'a> FmtVar<'a> { fn new(engine_state: &'a EngineState, var_id: VarId) -> Self { @@ -297,9 +297,9 @@ impl<'a> FmtVar<'a> { impl fmt::Display for FmtVar<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(name) = self.1 { - write!(f, "var {} {:?}", self.0, name) + write!(f, "var {} {:?}", self.0.get(), name) } else { - write!(f, "var {}", self.0) + write!(f, "var {}", self.0.get()) } } } @@ -346,9 +346,9 @@ impl<'a> fmt::Display for FmtLiteral<'a> { Literal::Filesize(q) => write!(f, "filesize({q}b)"), Literal::Duration(q) => write!(f, "duration({q}ns)"), Literal::Binary(b) => write!(f, "binary({})", FmtData(self.data, *b)), - Literal::Block(id) => write!(f, "block({id})"), - Literal::Closure(id) => write!(f, "closure({id})"), - Literal::RowCondition(id) => write!(f, "row_condition({id})"), + Literal::Block(id) => write!(f, "block({})", id.get()), + Literal::Closure(id) => write!(f, "closure({})", id.get()), + Literal::RowCondition(id) => write!(f, "row_condition({})", id.get()), Literal::Range { start, step, diff --git a/crates/nu-protocol/src/parser_path.rs b/crates/nu-protocol/src/parser_path.rs index 4935d9fbe..09e90475f 100644 --- a/crates/nu-protocol/src/parser_path.rs +++ b/crates/nu-protocol/src/parser_path.rs @@ -1,4 +1,7 @@ -use crate::engine::{StateWorkingSet, VirtualPath}; +use crate::{ + engine::{StateWorkingSet, VirtualPath}, + FileId, +}; use std::{ ffi::OsStr, path::{Path, PathBuf}, @@ -112,7 +115,7 @@ impl ParserPath { std::fs::File::open(p).map(|f| Box::new(f) as Box<dyn std::io::Read>) } ParserPath::VirtualFile(_, file_id) => working_set - .get_contents_of_file(*file_id) + .get_contents_of_file(FileId::new(*file_id)) .map(|bytes| Box::new(bytes) as Box<dyn std::io::Read>) .ok_or(std::io::ErrorKind::NotFound.into()), @@ -136,7 +139,9 @@ impl ParserPath { virtual_path: &VirtualPath, ) -> Self { match virtual_path { - VirtualPath::File(file_id) => ParserPath::VirtualFile(PathBuf::from(name), *file_id), + VirtualPath::File(file_id) => { + ParserPath::VirtualFile(PathBuf::from(name), file_id.get()) + } VirtualPath::Dir(entries) => ParserPath::VirtualDir( PathBuf::from(name), entries diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 80f96707a..5a6461ab1 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -23,7 +23,7 @@ use crate::{ ast::{Bits, Boolean, CellPath, Comparison, Math, Operator, PathMember}, did_you_mean, engine::{Closure, EngineState}, - Config, ShellError, Signals, Span, Type, + BlockId, Config, ShellError, Signals, Span, Type, }; use chrono::{DateTime, Datelike, FixedOffset, Locale, TimeZone}; use chrono_humanize::HumanTime; @@ -863,7 +863,7 @@ impl Value { .collect::<Vec<_>>() .join(separator) ), - Value::Closure { val, .. } => format!("<Closure {}>", val.block_id), + Value::Closure { val, .. } => format!("<Closure {}>", val.block_id.get()), Value::Nothing { .. } => String::new(), Value::Error { error, .. } => format!("{error:?}"), Value::Binary { val, .. } => format!("{val:?}"), @@ -2029,7 +2029,7 @@ impl Value { Value::test_record(Record::new()), Value::test_list(Vec::new()), Value::test_closure(Closure { - block_id: 0, + block_id: BlockId::new(0), captures: Vec::new(), }), Value::test_nothing(), diff --git a/crates/nuon/src/lib.rs b/crates/nuon/src/lib.rs index 891ccae62..304c8f977 100644 --- a/crates/nuon/src/lib.rs +++ b/crates/nuon/src/lib.rs @@ -12,7 +12,7 @@ mod tests { use nu_protocol::{ ast::{CellPath, PathMember, RangeInclusion}, engine::Closure, - record, IntRange, Range, Span, Value, + record, BlockId, IntRange, Range, Span, Value, }; use crate::{from_nuon, to_nuon, ToStyle}; @@ -175,7 +175,7 @@ mod tests { fn to_nuon_errs_on_closure() { assert!(to_nuon( &Value::test_closure(Closure { - block_id: 0, + block_id: BlockId::new(0), captures: vec![] }), ToStyle::Raw, |