Use actual outer scope for function-bang calls.

CHANGELOG.md

@@ -4,6 +4,11 @@ Rhai Release Notes
 Version 1.2.0
 =============
 
+Breaking changes
+----------------
+
+* As originally intended, function calls with a bang (`!`) now operates directly on the caller's scope, allowing variables inside the scope to be mutated.
+
 Bug fixes
 ---------
 
@@ -15,6 +20,7 @@ New features
 * `#[cfg(...)]` attributes can now be put directly on plugin functions or function defined in a plugin module.
 * A custom syntax parser can now return a symbol starting with `$$` to inform the implementation function which syntax variant was actually parsed.
 * `AST::iter_literal_variables` extracts all top-level literal constant/variable definitions from a script without running it.
+* `Scope::clone_visible` is added that copies only the last instance of each variable, omitting all shadowed variables.
 
 Enhancements
 ------------

src/ast.rs

@@ -1836,6 +1836,8 @@ impl FnCallHashes {
 pub struct FnCallExpr {
     /// Namespace of the function, if any.
     pub namespace: Option<NamespaceRef>,
+    /// Function name.
+    pub name: Identifier,
     /// Pre-calculated hashes.
     pub hashes: FnCallHashes,
     /// List of function call argument expressions.
@@ -1851,8 +1853,6 @@ pub struct FnCallExpr {
     /// an [`Expr::DynamicConstant`] involves an additional allocation.  Keeping the constant
     /// values in an inlined array avoids these extra allocations.
     pub constants: smallvec::SmallVec<[Dynamic; 2]>,
-    /// Function name.
-    pub name: Identifier,
     /// Does this function call capture the parent scope?
     pub capture_parent_scope: bool,
 }

src/func/call.rs

@@ -649,7 +649,7 @@ impl Engine {
         is_ref_mut: bool,
         is_method_call: bool,
         pos: Position,
-        captured_scope: Option<Scope>,
+        scope: Option<&mut Scope>,
         _level: usize,
     ) -> Result<(Dynamic, bool), Box<EvalAltResult>> {
         fn no_method_err(name: &str, pos: Position) -> Result<(Dynamic, bool), Box<EvalAltResult>> {
@@ -728,7 +728,14 @@ impl Engine {
                 return Ok((Dynamic::UNIT, false));
             }
 
-            let mut scope = captured_scope.unwrap_or_else(|| Scope::new());
+            let mut empty_scope;
+            let scope = if let Some(scope) = scope {
+                scope
+            } else {
+                empty_scope = Scope::new();
+                &mut empty_scope
+            };
+            let orig_scope_len = scope.len();
 
             let result = if _is_method_call {
                 // Method call of script function - map first argument to `this`
@@ -740,7 +747,7 @@ impl Engine {
                 let level = _level + 1;
 
                 let result = self.call_script_fn(
-                    &mut scope,
+                    scope,
                     mods,
                     state,
                     lib,
@@ -772,9 +779,8 @@ impl Engine {
 
                 let level = _level + 1;
 
-                let result = self.call_script_fn(
+                let result =
-                    &mut scope, mods, state, lib, &mut None, func, args, pos, level,
+                    self.call_script_fn(scope, mods, state, lib, &mut None, func, args, pos, level);
-                );
 
                 // Restore the original source
                 mods.source = orig_source;
@@ -787,6 +793,8 @@ impl Engine {
                 result?
             };
 
+            scope.rewind(orig_scope_len);
+
             return Ok((result, false));
         }
 
@@ -1225,12 +1233,30 @@ impl Engine {
         let mut arg_values = StaticVec::with_capacity(args_expr.len());
         let mut args = StaticVec::with_capacity(args_expr.len() + curry.len());
         let mut is_ref_mut = false;
-        let capture = if capture_scope && !scope.is_empty() {
-            Some(scope.clone_visible())
-        } else {
-            None
-        };
 
+        // Capture parent scope?
+        if capture_scope && !scope.is_empty() {
+            // func(..., ...)
+            for index in 0..args_expr.len() {
+                let (value, _) = self.get_arg_value(
+                    scope, mods, state, lib, this_ptr, level, args_expr, constants, index,
+                )?;
+                arg_values.push(value.flatten());
+            }
+            args.extend(curry.iter_mut());
+            args.extend(arg_values.iter_mut());
+
+            // Use parent scope
+            let scope = Some(scope);
+
+            return self
+                .exec_fn_call(
+                    mods, state, lib, name, hashes, &mut args, is_ref_mut, false, pos, scope, level,
+                )
+                .map(|(v, _)| v);
+        }
+
+        // Call with blank scope
         if args_expr.is_empty() && curry.is_empty() {
             // No arguments
         } else {
@@ -1287,7 +1313,7 @@ impl Engine {
         }
 
         self.exec_fn_call(
-            mods, state, lib, name, hashes, &mut args, is_ref_mut, false, pos, capture, level,
+            mods, state, lib, name, hashes, &mut args, is_ref_mut, false, pos, None, level,
         )
         .map(|(v, _)| v)
     }

src/parser.rs

@@ -1801,7 +1801,7 @@ fn make_dot_expr(
         // lhs.func!(...)
         (_, Expr::FnCall(x, pos)) if x.capture_parent_scope => {
             return Err(PERR::MalformedCapture(
-                "method-call style does not support capturing".into(),
+                "method-call style does not support running within the caller's scope".into(),
             )
             .into_err(pos))
         }

src/types/scope.rs

@@ -517,7 +517,7 @@ impl<'a> Scope<'a> {
     /// Shadowed variables are omitted in the copy.
     #[inline]
     #[must_use]
-    pub(crate) fn clone_visible(&self) -> Self {
+    pub fn clone_visible(&self) -> Self {
         let mut entries = Self::new();
 
         self.names

tests/functions.rs

@@ -164,5 +164,22 @@ fn test_functions_bang() -> Result<(), Box<EvalAltResult>> {
         165
     );
 
+    assert_eq!(
+        engine.eval::<INT>(
+            "
+                fn foo() {
+                    hello = 0;
+                    hello + bar
+                }
+
+                let hello = 42;
+                let bar = 123;
+
+                foo!()
+            ",
+        )?,
+        123
+    );
+
     Ok(())
 }

tests/internal_fn.rs

@@ -204,8 +204,7 @@ fn test_function_pointers() -> Result<(), Box<EvalAltResult>> {
 }
 
 #[test]
-#[cfg(not(feature = "no_closure"))]
+fn test_internal_fn_bang() -> Result<(), Box<EvalAltResult>> {
-fn test_internal_fn_captures() -> Result<(), Box<EvalAltResult>> {
     let engine = Engine::new();
 
     assert_eq!(
@@ -219,7 +218,7 @@ fn test_internal_fn_captures() -> Result<(), Box<EvalAltResult>> {
                 foo!(1) + x
             "
         )?,
-        83
+        84
     );
 
     assert!(engine