Fix export shadowing bug.

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
 ------------

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 mut i = scope.len();
-            let (_name, mut value, mut aliases) = scope.pop_entry().expect("not empty");
+        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::<crate::FnPtr>() {
@@ -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);
+                    }
                 }
             }
         }

src/parser.rs

@@ -55,7 +55,7 @@ pub struct ParseState<'e, 's> {
     /// Global runtime state.
     pub global: Option<Box<GlobalRuntimeState>>,
     /// Encapsulates a local stack with variable names to simulate an actual runtime scope.
-    pub stack: Option<Box<Scope<'e>>>,
+    pub stack: Option<Scope<'e>>,
     /// 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(),

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
+    /// Only the last variable matching the name (and not other shadowed versions) is aliased by this call.
-    /// this call.
     #[cfg(not(feature = "no_module"))]
     #[inline]
     pub fn set_alias(

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<EvalAltResult>> {
@@ -93,6 +93,41 @@ fn test_var_scope() -> Result<(), Box<EvalAltResult>> {
     Ok(())
 }
 
+#[cfg(not(feature = "no_module"))]
+#[test]
+fn test_var_scope_alias() -> Result<(), Box<EvalAltResult>> {
+    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::<INT>("a").unwrap(), 999);
+    assert_eq!(m.get_var_value::<INT>("b").unwrap(), 123);
+    assert_eq!(m.get_var_value::<INT>("c").unwrap(), 999);
+    assert_eq!(m.get_var_value::<INT>("y").unwrap(), 42);
+    assert_eq!(m.get_var_value::<INT>("z").unwrap(), 0);
+
+    Ok(())
+}
+
 #[test]
 fn test_var_is_def() -> Result<(), Box<EvalAltResult>> {
     let engine = Engine::new();