Fix broken constants in scopes (#9679)

This commit is contained in:
Jakub Žádník 2023-07-14 00:02:05 +03:00 committed by GitHub
parent 30904bd095
commit e66139e6bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 55 additions and 48 deletions

View File

@ -47,15 +47,17 @@ impl Command for Const {
call: &Call,
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
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())

View File

@ -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");

View File

@ -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)),
},

View File

@ -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<VarId> {
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()

View File

@ -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<u8>]) -> 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 {

View File

@ -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<Vec<u8>, VarId>,
pub constants: HashMap<VarId, Value>,
pub predecls: HashMap<Vec<u8>, DeclId>, // temporary storage for predeclarations
pub decls: HashMap<Vec<u8>, DeclId>,
pub modules: HashMap<Vec<u8>, 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(),

View File

@ -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<Value>,
}
impl Variable {
@ -13,6 +14,7 @@ impl Variable {
declaration_span,
ty,
mutable,
const_val: None,
}
}
}

View File

@ -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");
}