Extract copy/restore of first argument in method call.

README.md

@@ -47,7 +47,7 @@ It doesn't attempt to be a new language. For example:
 
 * No classes.  Well, Rust doesn't either. On the other hand...
 * No traits...  so it is also not Rust. Do your Rusty stuff in Rust.
-* No structures - definte your types in Rust instead; Rhai can seamless work with _any Rust type_.
+* No structures - define your types in Rust instead; Rhai can seamlessly work with _any Rust type_.
 * No first-class functions - Code your functions in Rust instead, and register them with Rhai.
 * No closures - do your closure magic in Rust instead; [turn a Rhai scripted function into a Rust closure](#calling-rhai-functions-from-rust).
 * It is best to expose an API in Rhai for scripts to call.  All your core functionalities should be in Rust.
@@ -2094,14 +2094,24 @@ Members and methods
 -------------------
 
 Properties and methods in a Rust custom type registered with the [`Engine`] can be called just like in Rust.
+Unlike functions defined in script (for which all arguments are passed by _value_),
+native Rust functions may mutate the object (or the first argument if called in normal function call style).
 
 ```rust
 let a = new_ts();           // constructor function
-a.field = 500;              // property access
+a.field = 500;              // property setter
-a.update();                 // method call, 'a' can be changed
+a.update();                 // method call, 'a' can be modified
 
-update(a);                  // this works, but 'a' is unchanged because only
+update(a);                  // <- this de-sugars to 'a.update()' this if 'a' is a simple variable
-                            // a COPY of 'a' is passed to 'update' by VALUE
+                            //    unlike scripted functions, 'a' can be modified and is not a copy
+
+let array = [ a ];
+
+update(array[0]);           // <- 'array[0]' is an expression returning a calculated value,
+                            //    a transient (i.e. a copy) so this statement has no effect
+                            //    except waste a lot of time cloning
+
+array[0].update();          // <- call this method-call style will update 'a'
 ```
 
 Custom types, properties and methods can be disabled via the [`no_object`] feature.

RELEASES.md

@@ -22,13 +22,16 @@ Breaking changes
   This is to avoid excessive cloning of strings.  All native-Rust functions taking string parameters
   should switch to `rhai::ImmutableString` (which is either `Rc<String>` or `Arc<String>` depending on
   whether the `sync` feature is used).
+* Native Rust functions registered with the `Engine` also mutates the first argument when called in
+  normal function-call style (previously the first argument will be passed by _value_ if not called
+  in method-call style).  Of course, if the first argument is a calculated value (e.g. result of an
+  expression), then mutating it has no effect, but at least it is not cloned.
 
 New features
 ------------
 
 * Set limit on maximum level of nesting expressions and statements to avoid panics during parsing.
 * New `EvalPackage` to disable `eval`.
-* More benchmarks.
 
 Speed enhancements
 ------------------
@@ -43,6 +46,11 @@ Speed enhancements
   (and therefore the `CorePackage`) because they are now always available, even under `Engine::new_raw`.
 * Operator-assignment statements (e.g. `+=`) are now handled directly and much faster.
 * Strings are now _immutable_ and use the `rhai::ImmutableString` type, eliminating large amounts of cloning.
+* For Native Rust functions taking a first `&mut` parameter, the first argument is passed by reference instead of
+  by value, even if not called in method-call style.  This allows many functions declared with `&mut` parameter to avoid
+  excessive cloning. For example, if `a` is a large array, getting its length in this manner: `len(a)` used to result
+  in a full clone of `a` before taking the length and throwing the copy away. Now, `a` is simply passed by reference,
+  avoiding the cloning altogether.
 
 
 Version 0.14.1

src/engine.rs

@@ -615,33 +615,54 @@ impl Engine {
                 return Err(Box::new(EvalAltResult::ErrorStackOverflow(pos)));
             }
         }
+
+        let mut this_copy: Dynamic = Default::default();
+        let mut old_this_ptr: Option<&mut Dynamic> = None;
+
+        /// This function replaces the first argument of a method call with a clone copy.
+        /// This is to prevent a pure function unintentionally consuming the first argument.
+        fn normalize_first_arg<'a>(
+            normalize: bool,
+            this_copy: &mut Dynamic,
+            old_this_ptr: &mut Option<&'a mut Dynamic>,
+            args: &mut FnCallArgs<'a>,
+        ) {
+            // Only do it for method calls with arguments.
+            if !normalize || args.is_empty() {
+                return;
+            }
+
+            // Clone the original value.
+            *this_copy = args[0].clone();
+
+            // Replace the first reference with a reference to the clone, force-casting the lifetime.
+            // Keep the original reference.  Must remember to restore it later with `restore_first_arg_of_method_call`.
+            let this_pointer = mem::replace(
+                args.get_mut(0).unwrap(),
+                unsafe_mut_cast_to_lifetime(this_copy),
+            );
+
+            *old_this_ptr = Some(this_pointer);
+        }
+
+        /// This function restores the first argument that was replaced by `normalize_first_arg_of_method_call`.
+        fn restore_first_arg<'a>(old_this_ptr: Option<&'a mut Dynamic>, args: &mut FnCallArgs<'a>) {
+            if let Some(this_pointer) = old_this_ptr {
+                mem::replace(args.get_mut(0).unwrap(), this_pointer);
+            }
+        }
+
         // First search in script-defined functions (can override built-in)
         if !native_only {
             if let Some(fn_def) = lib.get(&hashes.1) {
-                let mut this_copy: Option<Dynamic>;
+                normalize_first_arg(is_ref, &mut this_copy, &mut old_this_ptr, args);
-                let mut this_pointer: Option<&mut Dynamic> = None;
-
-                if is_ref && !args.is_empty() {
-                    // Clone the original value.  It'll be consumed because the function
-                    // is pure and doesn't know that the first value is a reference (i.e. `is_ref`)
-                    this_copy = Some(args[0].clone());
-
-                    // Replace the first reference with a reference to the clone, force-casting the lifetime.
-                    // Keep the original reference.  Must remember to restore it before existing this function.
-                    this_pointer = Some(mem::replace(
-                        args.get_mut(0).unwrap(),
-                        unsafe_mut_cast_to_lifetime(this_copy.as_mut().unwrap()),
-                    ));
-                }
 
                 // Run scripted function
                 let result =
                     self.call_script_fn(scope, state, lib, fn_name, fn_def, args, pos, level)?;
 
                 // Restore the original reference
-                if let Some(this_pointer) = this_pointer {
+                restore_first_arg(old_this_ptr, args);
-                    mem::replace(args.get_mut(0).unwrap(), this_pointer);
-                }
 
                 return Ok((result, false));
             }
@@ -654,29 +675,18 @@ impl Engine {
             .or_else(|| self.packages.get_fn(hashes.0))
         {
             // Calling pure function in method-call?
-            let mut this_copy: Option<Dynamic>;
+            normalize_first_arg(
-            let mut this_pointer: Option<&mut Dynamic> = None;
+                func.is_pure() && is_ref,
-
+                &mut this_copy,
-            if func.is_pure() && is_ref && !args.is_empty() {
+                &mut old_this_ptr,
-                // Clone the original value.  It'll be consumed because the function
+                args,
-                // is pure and doesn't know that the first value is a reference (i.e. `is_ref`)
+            );
-                this_copy = Some(args[0].clone());
-
-                // Replace the first reference with a reference to the clone, force-casting the lifetime.
-                // Keep the original reference.  Must remember to restore it before existing this function.
-                this_pointer = Some(mem::replace(
-                    args.get_mut(0).unwrap(),
-                    unsafe_mut_cast_to_lifetime(this_copy.as_mut().unwrap()),
-                ));
-            }
 
             // Run external function
             let result = func.get_native_fn()(args);
 
             // Restore the original reference
-            if let Some(this_pointer) = this_pointer {
+            restore_first_arg(old_this_ptr, args);
-                mem::replace(args.get_mut(0).unwrap(), this_pointer);
-            }
 
             let result = result.map_err(|err| err.new_position(pos))?;
 

src/module.rs

@@ -544,7 +544,7 @@ impl Module {
     /// use rhai::Module;
     ///
     /// let mut module = Module::new();
-    /// let hash = module.set_fn_3("calc", |x: i64, y: String, z: i64, _w: ()| {
+    /// let hash = module.set_fn_4("calc", |x: i64, y: String, z: i64, _w: ()| {
     ///     Ok(x + y.len() as i64 + z)
     /// });
     /// assert!(module.get_fn(hash).is_some());
@@ -583,7 +583,7 @@ impl Module {
         )
     }
 
-    /// Set a Rust function taking three parameters (the first one mutable) into the module,
+    /// Set a Rust function taking four parameters (the first one mutable) into the module,
     /// returning a hash key.
     ///
     /// If there is a similar existing Rust function, it is replaced.
@@ -594,7 +594,7 @@ impl Module {
     /// use rhai::Module;
     ///
     /// let mut module = Module::new();
-    /// let hash = module.set_fn_3_mut("calc", |x: &mut i64, y: String, z: i64, _w: ()| {
+    /// let hash = module.set_fn_4_mut("calc", |x: &mut i64, y: String, z: i64, _w: ()| {
     ///     *x += y.len() as i64 + z; Ok(*x)
     /// });
     /// assert!(module.get_fn(hash).is_some());

tests/method_call.rs

@@ -33,7 +33,7 @@ fn test_method_call() -> Result<(), Box<EvalAltResult>> {
 
     assert_eq!(
         engine.eval::<TestStruct>("let x = new_ts(); update(x, 1000); x")?,
-        TestStruct { x: 1 }
+        TestStruct { x: 1001 }
     );
 
     Ok(())