Merge pull request #733 from schungx/master

Fix bug.

CHANGELOG.md

@@ -1,6 +1,24 @@
 Rhai Release Notes
 ==================
 
+Version 1.16.0
+==============
+
+Bug fixes
+---------
+
+* Fixes a panic when using `this` as the first parameter in a namespace-qualified function call.
+
+
+Version 1.15.1
+==============
+
+Bug fixes
+---------
+
+* `Dynamic::deep_scan` is fixed so now it properly scans arrays, object maps and function pointers embedded inside data.
+
+
 Version 1.15.0
 ==============
 
@@ -14,6 +32,7 @@ Enhancements
 
 * Expressions involving `this` should now run slightly faster due to a dedicated `AST` node `ThisPtr`.
 * A `take` function is added to the standard library to take ownership of any data (replacing with `()`) in order to avoid cloning.
+* `Dynamic::take` is added to take ownership of the data (replacing with `()`) in order to avoid cloning.
 * `EvalAltResult::ErrorMismatchOutputType` now gives a better name for the requested generic type (e.g. `&str` is now `&str` and not `string`).
 
 

Cargo.toml

@@ -3,7 +3,7 @@ members = [".", "codegen"]
 
 [package]
 name = "rhai"
-version = "1.15.0"
+version = "1.16.0"
 rust-version = "1.61.0"
 edition = "2018"
 resolver = "2"

codegen/ui_tests/non_clonable.stderr

@@ -11,5 +11,6 @@ note: required by a bound in `rhai::Dynamic::cast`
    |                          ^^^^^ required by this bound in `Dynamic::cast`
 help: consider annotating `NonClonable` with `#[derive(Clone)]`
      |
-3    | #[derive(Clone)]
+3    + #[derive(Clone)]
+4    | struct NonClonable {
      |

codegen/ui_tests/non_clonable_second.stderr

@@ -11,5 +11,6 @@ note: required by a bound in `rhai::Dynamic::cast`
    |                          ^^^^^ required by this bound in `Dynamic::cast`
 help: consider annotating `NonClonable` with `#[derive(Clone)]`
      |
-3    | #[derive(Clone)]
+3    + #[derive(Clone)]
+4    | struct NonClonable {
      |

codegen/ui_tests/rhai_fn_non_clonable_return.stderr

@@ -14,5 +14,6 @@ note: required by a bound in `rhai::Dynamic::from`
    = note: this error originates in the attribute macro `export_fn` (in Nightly builds, run with -Z macro-backtrace for more info)
 help: consider annotating `NonClonable` with `#[derive(Clone)]`
      |
-3    | #[derive(Clone)]
+3    + #[derive(Clone)]
+4    | struct NonClonable {
      |

codegen/ui_tests/rhai_mod_non_clonable_return.stderr

@@ -15,5 +15,6 @@ note: required by a bound in `rhai::Dynamic::from`
    = note: this error originates in the attribute macro `export_module` (in Nightly builds, run with -Z macro-backtrace for more info)
 help: consider annotating `NonClonable` with `#[derive(Clone)]`
      |
-3    | #[derive(Clone)]
+3    + #[derive(Clone)]
+4    | struct NonClonable {
      |

codegen/ui_tests/rhai_mod_unknown_type.stderr

@@ -13,9 +13,9 @@ help: a struct with a similar name exists
    |                           ~~~~~
 help: consider importing one of these items
    |
-11 |     use core::fmt::Pointer;
+11 +     use core::fmt::Pointer;
    |
-11 |     use std::fmt::Pointer;
+11 +     use std::fmt::Pointer;
    |
-11 |     use syn::__private::fmt::Pointer;
+11 +     use syn::__private::fmt::Pointer;
    |

src/func/call.rs

@@ -1289,15 +1289,17 @@ impl Engine {
 
         // Call with blank scope
         #[cfg(not(feature = "no_closure"))]
-        let this_ptr_not_shared = this_ptr.as_ref().map_or(false, |v| !v.is_shared());
+        let has_non_shared_this_ptr = this_ptr.as_ref().map_or(false, |v| !v.is_shared());
         #[cfg(feature = "no_closure")]
-        let this_ptr_not_shared = true;
+        let has_non_shared_this_ptr = this_ptr.is_some();
 
         // If the first argument is a variable, and there are no curried arguments,
         // convert to method-call style in order to leverage potential &mut first argument
         // and avoid cloning the value.
         match first_arg {
-            Some(_first_expr @ Expr::ThisPtr(pos)) if curry.is_empty() && this_ptr_not_shared => {
+            Some(_first_expr @ Expr::ThisPtr(pos))
+                if curry.is_empty() && has_non_shared_this_ptr =>
+            {
                 // Turn it into a method call only if the object is not shared
                 self.track_operation(global, *pos)?;
 
@@ -1381,21 +1383,30 @@ impl Engine {
         let mut first_arg_value = None;
 
         #[cfg(not(feature = "no_closure"))]
-        let this_ptr_not_shared = this_ptr.as_ref().map_or(false, |v| !v.is_shared());
+        let has_non_shared_this_ptr = this_ptr.as_ref().map_or(false, |v| !v.is_shared());
         #[cfg(feature = "no_closure")]
-        let this_ptr_not_shared = true;
+        let has_non_shared_this_ptr = this_ptr.is_some();
 
         // See if the first argument is a variable.
         // If so, convert to method-call style in order to leverage potential
         // &mut first argument and avoid cloning the value.
         match args_expr.get(0) {
-            Some(_first_expr @ Expr::ThisPtr(pos)) if this_ptr_not_shared => {
+            Some(_first_expr @ Expr::ThisPtr(pos)) if has_non_shared_this_ptr => {
                 self.track_operation(global, *pos)?;
 
                 #[cfg(feature = "debugging")]
                 self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), _first_expr)?;
 
-                // Turn it into a method call only if the object is not shared
+                // The first value is a placeholder (for later if it needs to be cloned)
+                arg_values.push(Dynamic::UNIT);
+
+                for expr in args_expr.iter().skip(1) {
+                    let (value, ..) =
+                        self.get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), expr)?;
+                    arg_values.push(value.flatten());
+                }
+
+                // func(x, ...) -> x.func(...)
                 let (first, rest) = arg_values.split_first_mut().unwrap();
                 first_arg_value = Some(first);
                 args.push(this_ptr.unwrap());
@@ -1407,7 +1418,7 @@ impl Engine {
                 #[cfg(feature = "debugging")]
                 self.run_debugger(global, caches, scope, this_ptr.as_deref_mut(), first_expr)?;
 
-                // func(x, ...) -> x.func(...)
+                // The first value is a placeholder (for later if it needs to be cloned)
                 arg_values.push(Dynamic::UNIT);
 
                 for expr in args_expr.iter().skip(1) {
@@ -1423,6 +1434,7 @@ impl Engine {
                     args.extend(arg_values.iter_mut());
                 } else {
                     // Turn it into a method call only if the object is not shared and not a simple value
+                    // func(x, ...) -> x.func(...)
                     let (first, rest) = arg_values.split_first_mut().unwrap();
                     first_arg_value = Some(first);
                     let obj_ref = target.take_ref().expect("ref");

src/types/dynamic.rs

@@ -246,10 +246,7 @@ impl Dynamic {
     #[inline(always)]
     #[must_use]
     pub const fn is_shared(&self) -> bool {
-        #[cfg(not(feature = "no_closure"))]
+        matches!(self.0, Union::Shared(..))
-        return matches!(self.0, Union::Shared(..));
-        #[cfg(feature = "no_closure")]
-        return false;
     }
     /// Is the value held by this [`Dynamic`] a particular type?
     ///
@@ -2107,6 +2104,8 @@ impl Dynamic {
     #[allow(clippy::only_used_in_recursion)]
     pub fn deep_scan(&mut self, mut filter: impl FnMut(&mut Self)) {
         fn scan_inner(value: &mut Dynamic, filter: &mut impl FnMut(&mut Dynamic)) {
+            filter(value);
+
             match &mut value.0 {
                 #[cfg(not(feature = "no_index"))]
                 Union::Array(a, ..) => a.iter_mut().for_each(|v| scan_inner(v, filter)),
@@ -2117,7 +2116,6 @@ impl Dynamic {
             }
         }
 
-        filter(self);
         scan_inner(self, &mut filter);
     }
 }