diff --git a/crates/nu-cmd-lang/src/core_commands/const_.rs b/crates/nu-cmd-lang/src/core_commands/const_.rs index 35dea1fda8..ccabab5320 100644 --- a/crates/nu-cmd-lang/src/core_commands/const_.rs +++ b/crates/nu-cmd-lang/src/core_commands/const_.rs @@ -47,15 +47,17 @@ impl Command for Const { call: &Call, _input: PipelineData, ) -> Result { - let var_id = call - .positional_nth(0) - .expect("checked through parser") - .as_var() - .expect("internal error: missing variable"); + let var_id = if let Some(id) = call.positional_nth(0).and_then(|pos| pos.as_var()) { + id + } else { + return Err(ShellError::NushellFailedSpanned { + msg: "Could not get variable".to_string(), + label: "variable not added by the parser".to_string(), + span: call.head, + }); + }; - if let Some(constval) = engine_state.find_constant(var_id, &[]) { - // Instead of creating a second copy of the value in the stack, we could change - // stack.get_var() to check engine_state.find_constant(). + if let Some(constval) = &engine_state.get_var(var_id).const_val { stack.add_var(var_id, constval.clone()); Ok(PipelineData::empty()) diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index 3d69cca731..3a6dbf8292 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -294,9 +294,7 @@ pub fn find_in_dirs_env( .flatten() }; - let lib_dirs = dirs_var - .and_then(|dirs_var| engine_state.find_constant(dirs_var, &[])) - .cloned(); + let lib_dirs = dirs_var.and_then(|var_id| engine_state.get_var(var_id).const_val.clone()); // TODO: remove (see #8310) let lib_dirs_fallback = stack.get_env_var(engine_state, "NU_LIB_DIRS"); diff --git a/crates/nu-parser/src/eval.rs b/crates/nu-parser/src/eval.rs index 7f5b2e845a..59aaa7c32a 100644 --- a/crates/nu-parser/src/eval.rs +++ b/crates/nu-parser/src/eval.rs @@ -23,7 +23,7 @@ pub fn eval_constant( val: path.clone(), span: expr.span, }), - Expr::Var(var_id) => match working_set.find_constant(*var_id) { + Expr::Var(var_id) => match working_set.get_variable(*var_id).const_val.as_ref() { Some(val) => Ok(val.clone()), None => Err(ParseError::NotAConstant(expr.span)), }, diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 7e08598f65..1baba6f88c 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -2958,6 +2958,7 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin .trim_start_matches('$') .to_string(); + // TODO: Remove the hard-coded variables, too error-prone if ["in", "nu", "env", "nothing"].contains(&var_name.as_str()) { working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span)) } @@ -2982,7 +2983,25 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin match eval_constant(working_set, &rvalue) { Ok(val) => { - working_set.add_constant(var_id, val); + // In case rhs is parsed as 'any' but is evaluated to a concrete + // type: + let const_type = val.get_type(); + + if let Some(explicit_type) = &explicit_type { + if !type_compatible(explicit_type, &const_type) { + working_set.error(ParseError::TypeMismatch( + explicit_type.clone(), + const_type.clone(), + nu_protocol::span(&spans[(span.0 + 1)..]), + )); + } + } + + working_set.set_variable_type(var_id, const_type); + + // Assign the constant value to the variable + working_set.set_variable_const_val(var_id, val); + // working_set.add_constant(var_id, val); } Err(err) => working_set.error(err), } @@ -3527,7 +3546,7 @@ pub fn parse_register(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipe pub fn find_dirs_var(working_set: &StateWorkingSet, var_name: &str) -> Option { working_set .find_variable(format!("${}", var_name).as_bytes()) - .filter(|var_id| working_set.find_constant(*var_id).is_some()) + .filter(|var_id| working_set.get_variable(*var_id).const_val.is_some()) } /// This helper function is used to find files during parsing @@ -3595,7 +3614,9 @@ pub fn find_in_dirs( // Look up relative path from NU_LIB_DIRS working_set - .find_constant(find_dirs_var(working_set, dirs_var_name)?)? + .get_variable(find_dirs_var(working_set, dirs_var_name)?) + .const_val + .as_ref()? .as_list() .ok()? .iter() diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 84240f520c..4fdd75ed00 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -233,9 +233,6 @@ impl EngineState { for item in delta_overlay.vars.into_iter() { existing_overlay.vars.insert(item.0, item.1); } - for item in delta_overlay.constants.into_iter() { - existing_overlay.constants.insert(item.0, item.1); - } for item in delta_overlay.modules.into_iter() { existing_overlay.modules.insert(item.0, item.1); } @@ -727,16 +724,6 @@ impl EngineState { output } - pub fn find_constant(&self, var_id: VarId, removed_overlays: &[Vec]) -> Option<&Value> { - for overlay_frame in self.active_overlays(removed_overlays).rev() { - if let Some(val) = overlay_frame.constants.get(&var_id) { - return Some(val); - } - } - - None - } - pub fn get_span_contents(&self, span: &Span) -> &[u8] { for (contents, start, finish) in &self.file_contents { if span.start >= *start && span.end <= *finish { @@ -1639,23 +1626,13 @@ impl<'a> StateWorkingSet<'a> { } } - pub fn add_constant(&mut self, var_id: VarId, val: Value) { - self.last_overlay_mut().constants.insert(var_id, val); - } - - pub fn find_constant(&self, var_id: VarId) -> Option<&Value> { - let mut removed_overlays = vec![]; - - for scope_frame in self.delta.scope.iter().rev() { - for overlay_frame in scope_frame.active_overlays(&mut removed_overlays).rev() { - if let Some(val) = overlay_frame.constants.get(&var_id) { - return Some(val); - } - } + 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 { + 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.permanent_state - .find_constant(var_id, &removed_overlays) } pub fn get_variable(&self, var_id: VarId) -> &Variable { diff --git a/crates/nu-protocol/src/engine/overlay.rs b/crates/nu-protocol/src/engine/overlay.rs index 5134ddc9da..f4884d3b6f 100644 --- a/crates/nu-protocol/src/engine/overlay.rs +++ b/crates/nu-protocol/src/engine/overlay.rs @@ -1,4 +1,4 @@ -use crate::{DeclId, ModuleId, OverlayId, Value, VarId}; +use crate::{DeclId, ModuleId, OverlayId, VarId}; use std::collections::HashMap; pub static DEFAULT_OVERLAY_NAME: &str = "zero"; @@ -177,7 +177,6 @@ impl ScopeFrame { #[derive(Debug, Clone)] pub struct OverlayFrame { pub vars: HashMap, VarId>, - pub constants: HashMap, pub predecls: HashMap, DeclId>, // temporary storage for predeclarations pub decls: HashMap, DeclId>, pub modules: HashMap, ModuleId>, @@ -190,7 +189,6 @@ impl OverlayFrame { pub fn from_origin(origin: ModuleId, prefixed: bool) -> Self { Self { vars: HashMap::new(), - constants: HashMap::new(), predecls: HashMap::new(), decls: HashMap::new(), modules: HashMap::new(), diff --git a/crates/nu-protocol/src/variable.rs b/crates/nu-protocol/src/variable.rs index 563a24248c..8f14a26bc9 100644 --- a/crates/nu-protocol/src/variable.rs +++ b/crates/nu-protocol/src/variable.rs @@ -1,10 +1,11 @@ -use crate::{Span, Type}; +use crate::{Span, Type, Value}; #[derive(Clone, Debug)] pub struct Variable { pub declaration_span: Span, pub ty: Type, pub mutable: bool, + pub const_val: Option, } impl Variable { @@ -13,6 +14,7 @@ impl Variable { declaration_span, ty, mutable, + const_val: None, } } } diff --git a/tests/const_/mod.rs b/tests/const_/mod.rs index ecafabaa39..0de07b316d 100644 --- a/tests/const_/mod.rs +++ b/tests/const_/mod.rs @@ -102,3 +102,12 @@ fn const_unsupported() { assert!(actual.err.contains("not_a_constant")); } + +#[test] +fn const_in_scope() { + let inp = &["do { const x = 'x'; $x }"]; + + let actual = nu!(&inp.join("; ")); + + assert_eq!(actual.out, "x"); +}