Deprecate call_fn_dynamic into call_fn_raw.

CHANGELOG.md

@@ -19,8 +19,8 @@ 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.
+* `AST::iter_literal_variables` is added to extract 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.
+* `Engine::call_fn_dynamic` is deprecated and `Engine::call_fn_raw` is added which allows keeping new variables in the custom scope.
 
 Enhancements
 ------------
@@ -31,7 +31,8 @@ Enhancements
 * Array adds a `sort` method with no parameters which sorts homogeneous arrays of built-in comparable types (e.g. `INT`).
 * Inlining is disabled for error-path functions because errors are exceptional and scripts usually fail completely when an error is encountered.
 * The `optimize` module is completely eliminated under `no_optimize`, which should yield smaller code size.
-* Add `NativeCallContext::position` to return the position of the function call.
+* `NativeCallContext::position` is added to return the position of the function call.
+* `Scope::clone_visible` is added that copies only the last instance of each variable, omitting all shadowed variables.
 
 Deprecated API's
 ----------------

src/api/deprecated.rs

@@ -111,6 +111,79 @@ impl Engine {
     ) -> Result<(), Box<EvalAltResult>> {
         self.run_ast_with_scope(scope, ast)
     }
+    /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments
+    /// and optionally a value for binding to the `this` pointer.
+    ///
+    /// Not available under `no_function`.
+    ///
+    /// There is an option to evaluate the [`AST`] to load necessary modules before calling the function.
+    ///
+    /// # Deprecated
+    ///
+    /// This method is deprecated. Use [`run_ast_with_scope`][Engine::run_ast_with_scope] instead.
+    ///
+    /// This method will be removed in the next major version.
+    ///
+    /// # WARNING
+    ///
+    /// All the arguments are _consumed_, meaning that they're replaced by `()`.
+    /// This is to avoid unnecessarily cloning the arguments.
+    /// Do not use the arguments after this call. If they are needed afterwards,
+    /// clone them _before_ calling this function.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # fn main() -> Result<(), Box<rhai::EvalAltResult>> {
+    /// # #[cfg(not(feature = "no_function"))]
+    /// # {
+    /// use rhai::{Engine, Scope, Dynamic};
+    ///
+    /// let engine = Engine::new();
+    ///
+    /// let ast = engine.compile("
+    ///     fn add(x, y) { len(x) + y + foo }
+    ///     fn add1(x)   { len(x) + 1 + foo }
+    ///     fn bar()     { foo/2 }
+    ///     fn action(x) { this += x; }         // function using 'this' pointer
+    /// ")?;
+    ///
+    /// let mut scope = Scope::new();
+    /// scope.push("foo", 42_i64);
+    ///
+    /// // Call the script-defined function
+    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "add", None, [ "abc".into(), 123_i64.into() ])?;
+    /// //                                                                 ^^^^ no 'this' pointer
+    /// assert_eq!(result.cast::<i64>(), 168);
+    ///
+    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "add1", None, [ "abc".into() ])?;
+    /// assert_eq!(result.cast::<i64>(), 46);
+    ///
+    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "bar", None, [])?;
+    /// assert_eq!(result.cast::<i64>(), 21);
+    ///
+    /// let mut value: Dynamic = 1_i64.into();
+    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "action", Some(&mut value), [ 41_i64.into() ])?;
+    /// //                                                                    ^^^^^^^^^^^^^^^^ binding the 'this' pointer
+    /// assert_eq!(value.as_int().expect("value should be INT"), 42);
+    /// # }
+    /// # Ok(())
+    /// # }
+    /// ```
+    #[deprecated(since = "1.1.0", note = "use `call_fn_raw` instead")]
+    #[cfg(not(feature = "no_function"))]
+    #[inline(always)]
+    pub fn call_fn_dynamic(
+        &self,
+        scope: &mut Scope,
+        ast: &AST,
+        eval_ast: bool,
+        name: impl AsRef<str>,
+        this_ptr: Option<&mut Dynamic>,
+        arg_values: impl AsMut<[Dynamic]>,
+    ) -> RhaiResult {
+        self.call_fn_raw(scope, ast, eval_ast, true, name, this_ptr, arg_values)
+    }
 }
 
 impl Dynamic {

src/api/public.rs

@@ -1901,10 +1901,8 @@ impl Engine {
     ) -> Result<T, Box<EvalAltResult>> {
         let mut arg_values = crate::StaticVec::new();
         args.parse(&mut arg_values);
-        let mut args: crate::StaticVec<_> = arg_values.iter_mut().collect();
-        let name = name.as_ref();
 
-        let result = self.call_fn_dynamic_raw(scope, ast, true, name, &mut None, &mut args)?;
+        let result = self.call_fn_raw(scope, ast, true, true, name, None, arg_values)?;
 
         let typ = self.map_type_name(result.type_name());
 
@@ -1918,12 +1916,14 @@ impl Engine {
         })
     }
     /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments
-    /// and optionally a value for binding to the `this` pointer.
+    /// and the following options:
+    ///
+    /// * whether to evaluate the [`AST`] to load necessary modules before calling the function
+    /// * whether to rewind the [`Scope`] after the function call
+    /// * a value for binding to the `this` pointer (if any)
     ///
     /// Not available under `no_function`.
     ///
-    /// There is an option to evaluate the [`AST`] to load necessary modules before calling the function.
-    ///
     /// # WARNING
     ///
     /// All the arguments are _consumed_, meaning that they're replaced by `()`.
@@ -1946,77 +1946,67 @@ impl Engine {
     ///     fn add1(x)   { len(x) + 1 + foo }
     ///     fn bar()     { foo/2 }
     ///     fn action(x) { this += x; }         // function using 'this' pointer
+    ///     fn decl(x)   { let hello = x; }     // declaring variables
     /// ")?;
     ///
     /// let mut scope = Scope::new();
     /// scope.push("foo", 42_i64);
     ///
     /// // Call the script-defined function
-    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "add", None, [ "abc".into(), 123_i64.into() ])?;
+    /// let result = engine.call_fn_raw(&mut scope, &ast, true, true, "add", None, [ "abc".into(), 123_i64.into() ])?;
-    /// //                                                                 ^^^^ no 'this' pointer
+    /// //                                                                   ^^^^ no 'this' pointer
     /// assert_eq!(result.cast::<i64>(), 168);
     ///
-    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "add1", None, [ "abc".into() ])?;
+    /// let result = engine.call_fn_raw(&mut scope, &ast, true, true, "add1", None, [ "abc".into() ])?;
     /// assert_eq!(result.cast::<i64>(), 46);
     ///
-    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "bar", None, [])?;
+    /// let result = engine.call_fn_raw(&mut scope, &ast, true, true, "bar", None, [])?;
     /// assert_eq!(result.cast::<i64>(), 21);
     ///
     /// let mut value: Dynamic = 1_i64.into();
-    /// let result = engine.call_fn_dynamic(&mut scope, &ast, true, "action", Some(&mut value), [ 41_i64.into() ])?;
+    /// let result = engine.call_fn_raw(&mut scope, &ast, true, true, "action", Some(&mut value), [ 41_i64.into() ])?;
-    /// //                                                                    ^^^^^^^^^^^^^^^^ binding the 'this' pointer
+    /// //                                                                      ^^^^^^^^^^^^^^^^ binding the 'this' pointer
     /// assert_eq!(value.as_int().expect("value should be INT"), 42);
+    ///
+    /// engine.call_fn_raw(&mut scope, &ast, true, false, "decl", None, [ 42_i64.into() ])?;
+    /// //                                         ^^^^^ do not rewind scope
+    /// assert_eq!(scope.get_value::<i64>("hello").unwrap(), 42);
     /// # }
     /// # Ok(())
     /// # }
     /// ```
     #[cfg(not(feature = "no_function"))]
     #[inline]
-    pub fn call_fn_dynamic(
+    pub fn call_fn_raw(
         &self,
         scope: &mut Scope,
         ast: &AST,
         eval_ast: bool,
+        rewind_scope: bool,
         name: impl AsRef<str>,
-        mut this_ptr: Option<&mut Dynamic>,
+        this_ptr: Option<&mut Dynamic>,
-        mut arg_values: impl AsMut<[Dynamic]>,
+        arg_values: impl AsMut<[Dynamic]>,
-    ) -> RhaiResult {
-        let name = name.as_ref();
-        let mut args: crate::StaticVec<_> = arg_values.as_mut().iter_mut().collect();
-
-        self.call_fn_dynamic_raw(scope, ast, eval_ast, name, &mut this_ptr, &mut args)
-    }
-    /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments.
-    ///
-    /// # WARNING
-    ///
-    /// All the arguments are _consumed_, meaning that they're replaced by `()`.
-    /// This is to avoid unnecessarily cloning the arguments.
-    /// Do not use the arguments after this call. If they are needed afterwards,
-    /// clone them _before_ calling this function.
-    #[cfg(not(feature = "no_function"))]
-    #[inline]
-    pub(crate) fn call_fn_dynamic_raw(
-        &self,
-        scope: &mut Scope,
-        ast: &AST,
-        eval_ast: bool,
-        name: &str,
-        this_ptr: &mut Option<&mut Dynamic>,
-        args: &mut FnCallArgs,
     ) -> RhaiResult {
         let state = &mut EvalState::new();
         let mods = &mut Imports::new();
-        let lib = &[ast.lib()];
         let statements = ast.statements();
 
+        let orig_scope_len = scope.len();
+
         if eval_ast && !statements.is_empty() {
             // Make sure new variables introduced at global level do not _spill_ into the function call
-            let orig_scope_len = scope.len();
+            self.eval_global_statements(scope, mods, state, statements, &[ast.lib()], 0)?;
-            self.eval_global_statements(scope, mods, state, statements, lib, 0)?;
+
-            scope.rewind(orig_scope_len);
+            if rewind_scope {
+                scope.rewind(orig_scope_len);
+            }
         }
 
+        let name = name.as_ref();
+        let mut this_ptr = this_ptr;
+        let mut arg_values = arg_values;
+        let mut args: crate::StaticVec<_> = arg_values.as_mut().iter_mut().collect();
+
         let fn_def = ast
             .lib()
             .get_script_fn(name, args.len())
@@ -2024,19 +2014,27 @@ impl Engine {
 
         // Check for data race.
         #[cfg(not(feature = "no_closure"))]
-        crate::func::call::ensure_no_data_race(name, args, false)?;
+        crate::func::call::ensure_no_data_race(name, &mut args, false)?;
 
-        self.call_script_fn(
+        let result = self.call_script_fn(
             scope,
             mods,
             state,
-            lib,
+            &[ast.lib()],
-            this_ptr,
+            &mut this_ptr,
             fn_def,
-            args,
+            &mut args,
             Position::NONE,
+            rewind_scope,
             0,
-        )
+        );
+
+        // Remove arguments
+        if !rewind_scope && !args.is_empty() {
+            scope.remove_range(orig_scope_len, args.len())
+        }
+
+        result
     }
     /// Optimize the [`AST`] with constants defined in an external Scope.
     /// An optimized copy of the [`AST`] is returned while the original [`AST`] is consumed.

src/engine.rs

@@ -2205,7 +2205,7 @@ impl Engine {
             // Statement block
             Expr::Stmt(x) if x.is_empty() => Ok(Dynamic::UNIT),
             Expr::Stmt(x) => {
-                self.eval_stmt_block(scope, mods, state, lib, this_ptr, x, true, level)
+                self.eval_stmt_block(scope, mods, state, lib, this_ptr, x, true, true, level)
             }
 
             // lhs[idx_expr]
@@ -2374,6 +2374,7 @@ impl Engine {
         this_ptr: &mut Option<&mut Dynamic>,
         statements: &[Stmt],
         restore_prev_state: bool,
+        rewind_scope: bool,
         level: usize,
     ) -> RhaiResult {
         if statements.is_empty() {
@@ -2385,7 +2386,7 @@ impl Engine {
         let prev_scope_len = scope.len();
         let prev_mods_len = mods.len();
 
-        if restore_prev_state {
+        if rewind_scope {
             state.scope_level += 1;
         }
 
@@ -2427,10 +2428,12 @@ impl Engine {
             state.pop_fn_resolution_cache();
         }
 
-        if restore_prev_state {
+        if rewind_scope {
             scope.rewind(prev_scope_len);
-            mods.truncate(prev_mods_len);
             state.scope_level -= 1;
+        }
+        if restore_prev_state {
+            mods.truncate(prev_mods_len);
 
             // The impact of new local variables goes away at the end of a block
             // because any new variables introduced will go out of scope
@@ -2617,9 +2620,9 @@ impl Engine {
 
             // Block scope
             Stmt::Block(statements, _) if statements.is_empty() => Ok(Dynamic::UNIT),
-            Stmt::Block(statements, _) => {
+            Stmt::Block(statements, _) => self.eval_stmt_block(
-                self.eval_stmt_block(scope, mods, state, lib, this_ptr, statements, true, level)
+                scope, mods, state, lib, this_ptr, statements, true, true, level,
-            }
+            ),
 
             // If statement
             Stmt::If(expr, x, _) => {
@@ -2630,13 +2633,17 @@ impl Engine {
 
                 if guard_val {
                     if !x.0.is_empty() {
-                        self.eval_stmt_block(scope, mods, state, lib, this_ptr, &x.0, true, level)
+                        self.eval_stmt_block(
+                            scope, mods, state, lib, this_ptr, &x.0, true, true, level,
+                        )
                     } else {
                         Ok(Dynamic::UNIT)
                     }
                 } else {
                     if !x.1.is_empty() {
-                        self.eval_stmt_block(scope, mods, state, lib, this_ptr, &x.1, true, level)
+                        self.eval_stmt_block(
+                            scope, mods, state, lib, this_ptr, &x.1, true, true, level,
+                        )
                     } else {
                         Ok(Dynamic::UNIT)
                     }
@@ -2676,7 +2683,7 @@ impl Engine {
 
                         Some(if !statements.is_empty() {
                             self.eval_stmt_block(
-                                scope, mods, state, lib, this_ptr, statements, true, level,
+                                scope, mods, state, lib, this_ptr, statements, true, true, level,
                             )
                         } else {
                             Ok(Dynamic::UNIT)
@@ -2690,7 +2697,7 @@ impl Engine {
                     // Default match clause
                     if !def_stmt.is_empty() {
                         self.eval_stmt_block(
-                            scope, mods, state, lib, this_ptr, def_stmt, true, level,
+                            scope, mods, state, lib, this_ptr, def_stmt, true, true, level,
                         )
                     } else {
                         Ok(Dynamic::UNIT)
@@ -2701,7 +2708,8 @@ impl Engine {
             // Loop
             Stmt::While(Expr::Unit(_), body, _) => loop {
                 if !body.is_empty() {
-                    match self.eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level)
+                    match self
+                        .eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, true, level)
                     {
                         Ok(_) => (),
                         Err(err) => match *err {
@@ -2727,7 +2735,8 @@ impl Engine {
                     return Ok(Dynamic::UNIT);
                 }
                 if !body.is_empty() {
-                    match self.eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level)
+                    match self
+                        .eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, true, level)
                     {
                         Ok(_) => (),
                         Err(err) => match *err {
@@ -2744,7 +2753,8 @@ impl Engine {
                 let is_while = !options.contains(AST_OPTION_NEGATED);
 
                 if !body.is_empty() {
-                    match self.eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level)
+                    match self
+                        .eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, true, level)
                     {
                         Ok(_) => (),
                         Err(err) => match *err {
@@ -2843,7 +2853,7 @@ impl Engine {
                         }
 
                         let result = self.eval_stmt_block(
-                            scope, mods, state, lib, this_ptr, statements, true, level,
+                            scope, mods, state, lib, this_ptr, statements, true, true, level,
                         );
 
                         match result {
@@ -2907,7 +2917,9 @@ impl Engine {
                 let (try_stmt, err_var, catch_stmt) = x.as_ref();
 
                 let result = self
-                    .eval_stmt_block(scope, mods, state, lib, this_ptr, try_stmt, true, level)
+                    .eval_stmt_block(
+                        scope, mods, state, lib, this_ptr, try_stmt, true, true, level,
+                    )
                     .map(|_| Dynamic::UNIT);
 
                 match result {
@@ -2959,7 +2971,7 @@ impl Engine {
                         });
 
                         let result = self.eval_stmt_block(
-                            scope, mods, state, lib, this_ptr, catch_stmt, true, level,
+                            scope, mods, state, lib, this_ptr, catch_stmt, true, true, level,
                         );
 
                         scope.rewind(orig_scope_len);

src/func/call.rs

@@ -487,6 +487,7 @@ impl Engine {
         fn_def: &crate::ast::ScriptFnDef,
         args: &mut FnCallArgs,
         pos: Position,
+        rewind_scope: bool,
         level: usize,
     ) -> RhaiResult {
         #[inline(never)]
@@ -511,6 +512,8 @@ impl Engine {
             .into())
         }
 
+        assert!(fn_def.params.len() == args.len());
+
         #[cfg(not(feature = "unchecked"))]
         self.inc_operations(&mut mods.num_operations, pos)?;
 
@@ -565,7 +568,17 @@ impl Engine {
         // Evaluate the function
         let body = &fn_def.body;
         let result = self
-            .eval_stmt_block(scope, mods, state, unified_lib, this_ptr, body, true, level)
+            .eval_stmt_block(
+                scope,
+                mods,
+                state,
+                unified_lib,
+                this_ptr,
+                body,
+                true,
+                rewind_scope,
+                level,
+            )
             .or_else(|err| match *err {
                 // Convert return statement to return value
                 EvalAltResult::Return(x, _) => Ok(x),
@@ -589,7 +602,9 @@ impl Engine {
             });
 
         // Remove all local variables
-        scope.rewind(prev_scope_len);
+        if rewind_scope {
+            scope.rewind(prev_scope_len);
+        }
         mods.truncate(prev_mods_len);
 
         if unified {
@@ -735,7 +750,6 @@ impl Engine {
                 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`
@@ -755,6 +769,7 @@ impl Engine {
                     func,
                     rest_args,
                     pos,
+                    true,
                     level,
                 );
 
@@ -779,8 +794,9 @@ impl Engine {
 
                 let level = _level + 1;
 
-                let result =
+                let result = self.call_script_fn(
-                    self.call_script_fn(scope, mods, state, lib, &mut None, func, args, pos, level);
+                    scope, mods, state, lib, &mut None, func, args, pos, true, level,
+                );
 
                 // Restore the original source
                 mods.source = orig_source;
@@ -793,8 +809,6 @@ impl Engine {
                 result?
             };
 
-            scope.rewind(orig_scope_len);
-
             return Ok((result, false));
         }
 
@@ -817,14 +831,16 @@ impl Engine {
         lib: &[&Module],
         level: usize,
     ) -> RhaiResult {
-        self.eval_stmt_block(scope, mods, state, lib, &mut None, statements, false, level)
+        self.eval_stmt_block(
-            .or_else(|err| match *err {
+            scope, mods, state, lib, &mut None, statements, false, false, level,
-                EvalAltResult::Return(out, _) => Ok(out),
+        )
-                EvalAltResult::LoopBreak(_, _) => {
+        .or_else(|err| match *err {
-                    unreachable!("no outer loop scope to break out of")
+            EvalAltResult::Return(out, _) => Ok(out),
-                }
+            EvalAltResult::LoopBreak(_, _) => {
-                _ => Err(err),
+                unreachable!("no outer loop scope to break out of")
-            })
+            }
+            _ => Err(err),
+        })
     }
 
     /// Evaluate a text script in place - used primarily for 'eval'.
@@ -1435,7 +1451,7 @@ impl Engine {
                     let level = level + 1;
 
                     let result = self.call_script_fn(
-                        new_scope, mods, state, lib, &mut None, fn_def, &mut args, pos, level,
+                        new_scope, mods, state, lib, &mut None, fn_def, &mut args, pos, true, level,
                     );
 
                     mods.source = source;

src/types/scope.rs

@@ -587,6 +587,16 @@ impl<'a> Scope<'a> {
             .zip(self.values.iter())
             .map(|((name, _), value)| (name.as_ref(), value.is_read_only(), value))
     }
+    /// Remove a range of entries within the [`Scope`].
+    ///
+    /// # Panics
+    ///
+    /// Panics if the range is out of bounds.
+    #[inline]
+    pub(crate) fn remove_range(&mut self, start: usize, len: usize) {
+        self.values.drain(start..start + len).for_each(|_| {});
+        self.names.drain(start..start + len).for_each(|_| {});
+    }
 }
 
 impl<'a, K: Into<Cow<'a, str>>> Extend<(K, Dynamic)> for Scope<'a> {

tests/call_fn.rs

@@ -22,9 +22,9 @@ fn test_call_fn() -> Result<(), Box<EvalAltResult>> {
             fn hello() {
                 41 + foo
             }
-            fn define_var() {
+            fn define_var(scale) {
                 let bar = 21;
-                bar * 2
+                bar * scale
             }
         ",
     )?;
@@ -38,11 +38,6 @@ fn test_call_fn() -> Result<(), Box<EvalAltResult>> {
     let r: INT = engine.call_fn(&mut scope, &ast, "hello", ())?;
     assert_eq!(r, 42);
 
-    let r: INT = engine.call_fn(&mut scope, &ast, "define_var", ())?;
-    assert_eq!(r, 42);
-
-    assert!(!scope.contains("bar"));
-
     assert_eq!(
         scope
             .get_value::<INT>("foo")
@@ -50,6 +45,27 @@ fn test_call_fn() -> Result<(), Box<EvalAltResult>> {
         1
     );
 
+    let r: INT = engine.call_fn(&mut scope, &ast, "define_var", (2 as INT,))?;
+    assert_eq!(r, 42);
+
+    assert!(!scope.contains("bar"));
+
+    let args = [(2 as INT).into()];
+    let r = engine
+        .call_fn_raw(&mut scope, &ast, false, false, "define_var", None, args)?
+        .as_int()
+        .unwrap();
+    assert_eq!(r, 42);
+
+    assert_eq!(
+        scope
+            .get_value::<INT>("bar")
+            .expect("variable bar should exist"),
+        21
+    );
+
+    assert!(!scope.contains("scale"));
+
     Ok(())
 }