diff --git a/CHANGELOG.md b/CHANGELOG.md index eee62da7..d0941dd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,13 @@ Version 1.14.0 The code hacks that attempt to optimize branch prediction performance are removed because benchmarks do not show any material speed improvements. -Buf fixes +Bug fixes ---------- * `is_shared` is a reserved keyword and is now handled properly (e.g. it cannot be the target of a function pointer). * Re-optimizing an AST via `optimize_ast` with constants now works correctly for closures. Previously the hidden `Share` nodes are not removed and causes variable-not-found errors during runtime if the constants are not available in the scope. * Expressions such as `(v[0].func()).prop` now parse correctly. +* Shadowed variable exports are now handled correctly. New features ------------ diff --git a/src/module/mod.rs b/src/module/mod.rs index eeb6874e..059028e2 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -2211,8 +2211,17 @@ impl Module { }); // Variables with an alias left in the scope become module variables - while scope.len() > orig_scope_len { - let (_name, mut value, mut aliases) = scope.pop_entry().expect("not empty"); + let mut i = scope.len(); + while i > 0 { + i -= 1; + + let (mut value, mut aliases) = if i >= orig_scope_len { + let (_, v, a) = scope.pop_entry().expect("not empty"); + (v, a) + } else { + let (_, v, a) = scope.get_entry_by_index(i); + (v.clone(), a.to_vec()) + }; value.deep_scan(|v| { if let Some(fn_ptr) = v.downcast_mut::() { @@ -2224,15 +2233,28 @@ impl Module { 0 => (), 1 => { let alias = aliases.pop().unwrap(); - module.set_var(alias, value); + if !module.contains_var(&alias) { + module.set_var(alias, value); + } } _ => { - let last_alias = aliases.pop().unwrap(); - for alias in aliases { - module.set_var(alias, value.clone()); - } // Avoid cloning the last value - module.set_var(last_alias, value); + let mut first_alias = None; + + for alias in aliases { + if module.contains_var(&alias) { + continue; + } + if first_alias.is_none() { + first_alias = Some(alias); + } else { + module.set_var(alias, value.clone()); + } + } + + if let Some(alias) = first_alias { + module.set_var(alias, value); + } } } } diff --git a/src/parser.rs b/src/parser.rs index a34c851c..d512cbad 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -55,7 +55,7 @@ pub struct ParseState<'e, 's> { /// Global runtime state. pub global: Option>, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. - pub stack: Option>>, + pub stack: Option>, /// Size of the local variables stack upon entry of the current block scope. pub block_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). @@ -143,7 +143,7 @@ impl<'e, 's> ParseState<'e, 's> { let index = self .stack - .as_deref() + .as_ref() .into_iter() .flat_map(Scope::iter_rev_raw) .enumerate() @@ -2890,7 +2890,7 @@ impl Engine { settings.flags |= ParseSettingFlags::BREAKABLE; let body = self.parse_block(input, state, lib, settings)?.into(); - state.stack.as_deref_mut().unwrap().rewind(prev_stack_len); + state.stack.as_mut().unwrap().rewind(prev_stack_len); let branch = StmtBlock::NONE; @@ -2971,15 +2971,21 @@ impl Engine { let (existing, hit_barrier) = state.find_var(&name); - let stack = state.stack.as_deref_mut().unwrap(); + let stack = state.stack.as_mut().unwrap(); let existing = if !hit_barrier && existing > 0 { let offset = stack.len() - existing; - if offset < state.block_stack_len { - // Defined in parent block + + if !stack.get_entry_by_index(offset).2.is_empty() { + // Variable has been aliased None } else { - Some(offset) + if offset < state.block_stack_len { + // Defined in parent block + None + } else { + Some(offset) + } } } else { None @@ -2993,6 +2999,10 @@ impl Engine { None }; + if is_export { + stack.add_alias_by_index(stack.len() - 1, name.clone()); + } + let var_def = (Ident { name, pos }, expr, idx).into(); Ok(match access { @@ -3077,18 +3087,25 @@ impl Engine { let (id, id_pos) = parse_var_name(input)?; let (alias, alias_pos) = if match_token(input, Token::As).0 { - parse_var_name(input).map(|(name, pos)| (Some(name), pos))? + parse_var_name(input).map(|(name, pos)| (state.get_interned_string(name), pos))? } else { - (None, Position::NONE) + (state.get_interned_string(""), Position::NONE) }; + let (existing, hit_barrier) = state.find_var(&id); + + if !hit_barrier && existing > 0 { + let stack = state.stack.as_mut().unwrap(); + stack.add_alias_by_index(stack.len() - existing, alias.clone()); + } + let export = ( Ident { name: state.get_interned_string(id), pos: id_pos, }, Ident { - name: state.get_interned_string(alias.as_deref().unwrap_or("")), + name: alias, pos: alias_pos, }, ); @@ -3137,7 +3154,7 @@ impl Engine { } let prev_entry_stack_len = state.block_stack_len; - state.block_stack_len = state.stack.as_deref().map_or(0, Scope::len); + state.block_stack_len = state.stack.as_ref().map_or(0, Scope::len); #[cfg(not(feature = "no_module"))] let orig_imports_len = state.imports.as_deref().map_or(0, StaticVec::len); @@ -3580,7 +3597,7 @@ impl Engine { Expr::Unit(catch_var.pos) } else { // Remove the error variable from the stack - state.stack.as_deref_mut().unwrap().pop(); + state.stack.as_mut().unwrap().pop(); Expr::Variable( (None, Namespace::default(), 0, catch_var.name).into(), diff --git a/src/types/scope.rs b/src/types/scope.rs index 7d370c1e..22626d02 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -635,6 +635,22 @@ impl Scope<'_> { pub fn get(&self, name: &str) -> Option<&Dynamic> { self.search(name).map(|index| &self.values[index]) } + /// Get a reference to an entry in the [`Scope`] based on the index. + /// + /// # Panics + /// + /// Panics if the index is out of bounds. + #[inline(always)] + pub(crate) fn get_entry_by_index( + &mut self, + index: usize, + ) -> (&Identifier, &Dynamic, &[ImmutableString]) { + ( + &self.names[index], + &self.values[index], + &self.aliases[index], + ) + } /// Remove the last entry in the [`Scope`] by the specified name and return its value. /// /// If the entry by the specified name is not found, [`None`] is returned. @@ -670,7 +686,7 @@ impl Scope<'_> { self.values.remove(index).try_cast() }) } - /// Get a mutable reference to an entry in the [`Scope`]. + /// Get a mutable reference to the value of an entry in the [`Scope`]. /// /// If the entry by the specified name is not found, or if it is read-only, /// [`None`] is returned. @@ -702,14 +718,14 @@ impl Scope<'_> { AccessMode::ReadOnly => None, }) } - /// Get a mutable reference to an entry in the [`Scope`] based on the index. + /// Get a mutable reference to the value of an entry in the [`Scope`] based on the index. /// /// # Panics /// /// Panics if the index is out of bounds. - #[inline] + #[inline(always)] pub(crate) fn get_mut_by_index(&mut self, index: usize) -> &mut Dynamic { - self.values.get_mut(index).unwrap() + &mut self.values[index] } /// Add an alias to an entry in the [`Scope`]. /// @@ -728,12 +744,14 @@ impl Scope<'_> { /// Add an alias to a variable in the [`Scope`] so that it is exported under that name. /// This is an advanced API. /// + /// Variable aliases are used, for example, in [`Module::eval_ast_as_new`][crate::Module::eval_ast_as_new] + /// to create a new module with exported variables under different names. + /// /// If the alias is empty, then the variable is exported under its original name. /// /// Multiple aliases can be added to any variable. /// - /// Only the last variable matching the name (and not other shadowed versions) is aliased by - /// this call. + /// Only the last variable matching the name (and not other shadowed versions) is aliased by this call. #[cfg(not(feature = "no_module"))] #[inline] pub fn set_alias( diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 5a3120ba..776bf215 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -1,4 +1,4 @@ -use rhai::{Dynamic, Engine, EvalAltResult, ParseErrorType, Position, Scope, INT}; +use rhai::{Dynamic, Engine, EvalAltResult, Module, ParseErrorType, Position, Scope, INT}; #[test] fn test_var_scope() -> Result<(), Box> { @@ -93,6 +93,41 @@ fn test_var_scope() -> Result<(), Box> { Ok(()) } +#[cfg(not(feature = "no_module"))] +#[test] +fn test_var_scope_alias() -> Result<(), Box> { + let engine = Engine::new(); + let mut scope = Scope::new(); + + scope.push("x", 42 as INT); + scope.set_alias("x", "a"); + scope.set_alias("x", "b"); + scope.set_alias("x", "y"); + scope.push("x", 123 as INT); + scope.set_alias("x", "b"); + scope.set_alias("x", "c"); + + let ast = engine.compile( + " + let x = 999; + export x as a; + export x as c; + let x = 0; + export x as z; + ", + )?; + + let m = Module::eval_ast_as_new(scope, &ast, &engine)?; + + assert_eq!(m.get_var_value::("a").unwrap(), 999); + assert_eq!(m.get_var_value::("b").unwrap(), 123); + assert_eq!(m.get_var_value::("c").unwrap(), 999); + assert_eq!(m.get_var_value::("y").unwrap(), 42); + assert_eq!(m.get_var_value::("z").unwrap(), 0); + + Ok(()) +} + #[test] fn test_var_is_def() -> Result<(), Box> { let engine = Engine::new();