Fix bug with eager optimization of method calls.

CHANGELOG.md

@@ -15,6 +15,7 @@ Bug fixes
 * Invalid property or method access such as `a.b::c.d` or `a.b::func()` no longer panics but properly returns a syntax error.
 * `Scope::is_constant` now returns the correct value.
 * Exporting a variable that contains a local function pointer (including anonymous function or closure) now raises a runtime error.
+* Full optimization is now skipped for method calls.
 
 Breaking changes
 ----------------

src/ast/expr.rs

@@ -197,16 +197,18 @@ impl fmt::Debug for FnCallExpr {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut ff = f.debug_struct("FnCallExpr");
         #[cfg(not(feature = "no_module"))]
-        self.namespace.as_ref().map(|ns| ff.field("namespace", ns));
+        if let Some(ref ns) = self.namespace {
-        ff.field("name", &self.name)
+            ff.field("namespace", ns);
-            .field("hash", &self.hashes)
-            .field("arg_exprs", &self.args);
-        if !self.constants.is_empty() {
-            ff.field("constant_args", &self.constants);
         }
         if self.capture_parent_scope {
             ff.field("capture_parent_scope", &self.capture_parent_scope);
         }
+        ff.field("hash", &self.hashes)
+            .field("name", &self.name)
+            .field("args", &self.args);
+        if !self.constants.is_empty() {
+            ff.field("constants", &self.constants);
+        }
         ff.field("pos", &self.pos);
         ff.finish()
     }
@@ -405,6 +407,8 @@ pub enum Expr {
         Box<((Identifier, u64), (Identifier, u64), ImmutableString)>,
         Position,
     ),
+    /// xxx `.` method `(` expr `,` ... `)`
+    MethodCall(Box<FnCallExpr>, Position),
     /// Stack slot for function calls.  See [`FnCallExpr`] for more details.
     ///
     /// This variant does not map to any language structure.  It is used in function calls with
@@ -485,6 +489,7 @@ impl fmt::Debug for Expr {
                 f.write_str(")")
             }
             Self::Property(x, ..) => write!(f, "Property({})", x.2),
+            Self::MethodCall(x, ..) => f.debug_tuple("MethodCall").field(x).finish(),
             Self::Stack(x, ..) => write!(f, "ConstantArg[{}]", x),
             Self::Stmt(x) => {
                 let pos = x.span();
@@ -708,7 +713,7 @@ impl Expr {
             | Self::InterpolatedString(.., pos)
             | Self::Property(.., pos) => *pos,
 
-            Self::FnCall(x, ..) => x.pos,
+            Self::FnCall(x, ..) | Self::MethodCall(x, ..) => x.pos,
 
             Self::Stmt(x) => x.position(),
         }
@@ -756,6 +761,7 @@ impl Expr {
             | Self::Variable(.., pos, _)
             | Self::Stack(.., pos)
             | Self::FnCall(.., pos)
+            | Self::MethodCall(.., pos)
             | Self::Custom(.., pos)
             | Self::InterpolatedString(.., pos)
             | Self::Property(.., pos) => *pos = new_pos,
@@ -839,6 +845,7 @@ impl Expr {
             | Self::StringConstant(..)
             | Self::InterpolatedString(..)
             | Self::FnCall(..)
+            | Self::MethodCall(..)
             | Self::Stmt(..)
             | Self::Dot(..)
             | Self::Index(..)

src/eval/chaining.rs

@@ -252,7 +252,7 @@ impl Engine {
             ChainType::Dotting => {
                 match rhs {
                     // xxx.fn_name(arg_expr_list)
-                    Expr::FnCall(x, pos) if !x.is_qualified() && new_val.is_none() => {
+                    Expr::MethodCall(x, pos) if !x.is_qualified() && new_val.is_none() => {
                         let crate::ast::FnCallExpr { name, hashes, .. } = x.as_ref();
                         let call_args = &mut idx_val.into_fn_call_args();
 
@@ -271,11 +271,11 @@ impl Engine {
                         result
                     }
                     // xxx.fn_name(...) = ???
-                    Expr::FnCall(..) if new_val.is_some() => {
+                    Expr::MethodCall(..) if new_val.is_some() => {
                         unreachable!("method call cannot be assigned to")
                     }
                     // xxx.module::fn_name(...) - syntax error
-                    Expr::FnCall(..) => {
+                    Expr::MethodCall(..) => {
                         unreachable!("function call in dot chain should not be namespace-qualified")
                     }
                     // {xxx:map}.id op= ???
@@ -428,7 +428,7 @@ impl Engine {
                                 )?
                             }
                             // {xxx:map}.fn_name(arg_expr_list)[expr] | {xxx:map}.fn_name(arg_expr_list).expr
-                            Expr::FnCall(ref x, pos) if !x.is_qualified() => {
+                            Expr::MethodCall(ref x, pos) if !x.is_qualified() => {
                                 let crate::ast::FnCallExpr { name, hashes, .. } = x.as_ref();
                                 let call_args = &mut idx_val.into_fn_call_args();
 
@@ -448,7 +448,7 @@ impl Engine {
                                 result?.0.into()
                             }
                             // {xxx:map}.module::fn_name(...) - syntax error
-                            Expr::FnCall(..) => unreachable!(
+                            Expr::MethodCall(..) => unreachable!(
                                 "function call in dot chain should not be namespace-qualified"
                             ),
                             // Others - syntax error
@@ -549,7 +549,7 @@ impl Engine {
                                 Ok((result, may_be_changed))
                             }
                             // xxx.fn_name(arg_expr_list)[expr] | xxx.fn_name(arg_expr_list).expr
-                            Expr::FnCall(ref f, pos) if !f.is_qualified() => {
+                            Expr::MethodCall(ref f, pos) if !f.is_qualified() => {
                                 let crate::ast::FnCallExpr { name, hashes, .. } = f.as_ref();
                                 let rhs_chain = rhs.into();
                                 let args = &mut idx_val.into_fn_call_args();
@@ -576,7 +576,7 @@ impl Engine {
                                 .map_err(|err| err.fill_position(pos))
                             }
                             // xxx.module::fn_name(...) - syntax error
-                            Expr::FnCall(..) => unreachable!(
+                            Expr::MethodCall(..) => unreachable!(
                                 "function call in dot chain should not be namespace-qualified"
                             ),
                             // Others - syntax error
@@ -681,7 +681,7 @@ impl Engine {
 
         match expr {
             #[cfg(not(feature = "no_object"))]
-            Expr::FnCall(x, ..)
+            Expr::MethodCall(x, ..)
                 if _parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
             {
                 let crate::ast::FnCallExpr {
@@ -705,7 +705,7 @@ impl Engine {
                 idx_values.push(super::ChainArgument::from_fn_call_args(values, pos));
             }
             #[cfg(not(feature = "no_object"))]
-            Expr::FnCall(..) if _parent_chain_type == ChainType::Dotting => {
+            Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => {
                 unreachable!("function call in dot chain should not be namespace-qualified")
             }
 
@@ -729,7 +729,7 @@ impl Engine {
                     Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"),
 
                     #[cfg(not(feature = "no_object"))]
-                    Expr::FnCall(x, ..)
+                    Expr::MethodCall(x, ..)
                         if _parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
                     {
                         let crate::ast::FnCallExpr {
@@ -752,7 +752,7 @@ impl Engine {
                         super::ChainArgument::from_fn_call_args(values, pos)
                     }
                     #[cfg(not(feature = "no_object"))]
-                    Expr::FnCall(..) if _parent_chain_type == ChainType::Dotting => {
+                    Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => {
                         unreachable!("function call in dot chain should not be namespace-qualified")
                     }
                     #[cfg(not(feature = "no_object"))]

src/optimizer.rs

@@ -1143,7 +1143,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
 
         // Eagerly call functions
         Expr::FnCall(x, pos)
-                if !x.is_qualified() // Non-qualified
+                if !x.is_qualified() // non-qualified
                 && state.optimization_level == OptimizationLevel::Full // full optimizations
                 && x.args.iter().all(Expr::is_constant) // all arguments are constants
         => {
@@ -1176,8 +1176,8 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
             x.args.iter_mut().for_each(|a| optimize_expr(a, state, false));
         }
 
-        // id(args ..) -> optimize function call arguments
+        // id(args ..) or xxx.id(args ..) -> optimize function call arguments
-        Expr::FnCall(x, ..) => for arg in x.args.iter_mut() {
+        Expr::FnCall(x, ..) | Expr::MethodCall(x, ..) => for arg in x.args.iter_mut() {
             optimize_expr(arg, state, false);
 
             // Move constant arguments

src/parser.rs

@@ -521,8 +521,8 @@ impl Engine {
                     namespace,
                     hashes,
                     args,
+                    constants: StaticVec::new_const(),
                     pos: settings.pos,
-                    ..Default::default()
                 }
                 .into_fn_call_expr(settings.pos));
             }
@@ -586,8 +586,8 @@ impl Engine {
                         namespace,
                         hashes,
                         args,
+                        constants: StaticVec::new_const(),
                         pos: settings.pos,
-                        ..Default::default()
                     }
                     .into_fn_call_expr(settings.pos));
                 }
@@ -1990,7 +1990,7 @@ impl Engine {
                     calc_fn_hash(&func.name, func.args.len() + 1),
                 );
 
-                let rhs = Expr::FnCall(func, func_pos);
+                let rhs = Expr::MethodCall(func, func_pos);
                 Ok(Expr::Dot(
                     BinaryExpr { lhs, rhs }.into(),
                     ASTFlags::NONE,
@@ -2046,7 +2046,7 @@ impl Engine {
                         );
 
                         let new_lhs = BinaryExpr {
-                            lhs: Expr::FnCall(func, func_pos),
+                            lhs: Expr::MethodCall(func, func_pos),
                             rhs: x.rhs,
                         }
                         .into();

tests/method_call.rs

@@ -2,23 +2,23 @@
 
 use rhai::{Engine, EvalAltResult, INT};
 
+#[derive(Debug, Clone, Eq, PartialEq)]
+struct TestStruct {
+    x: INT,
+}
+
+impl TestStruct {
+    fn update(&mut self, n: INT) {
+        self.x += n;
+    }
+
+    fn new() -> Self {
+        Self { x: 1 }
+    }
+}
+
 #[test]
 fn test_method_call() -> Result<(), Box<EvalAltResult>> {
-    #[derive(Debug, Clone, Eq, PartialEq)]
-    struct TestStruct {
-        x: INT,
-    }
-
-    impl TestStruct {
-        fn update(&mut self, n: INT) {
-            self.x += n;
-        }
-
-        fn new() -> Self {
-            Self { x: 1 }
-        }
-    }
-
     let mut engine = Engine::new();
 
     engine
@@ -47,3 +47,31 @@ fn test_method_call_style() -> Result<(), Box<EvalAltResult>> {
 
     Ok(())
 }
+
+#[cfg(not(feature = "no_optimize"))]
+#[test]
+fn test_method_call_with_full_optimization() -> Result<(), Box<EvalAltResult>> {
+    let mut engine = Engine::new();
+
+    engine.set_optimization_level(rhai::OptimizationLevel::Full);
+
+    engine
+        .register_fn("new_ts", || TestStruct::new())
+        .register_fn("ymd", |_: INT, _: INT, _: INT| 42 as INT)
+        .register_fn("range", |_: &mut TestStruct, _: INT, _: INT| {
+            TestStruct::new()
+        });
+
+    assert_eq!(
+        engine.eval::<TestStruct>(
+            "
+                let xs = new_ts();
+                let ys = xs.range(ymd(2022, 2, 1), ymd(2022, 2, 2));
+                ys
+            "
+        )?,
+        TestStruct::new()
+    );
+
+    Ok(())
+}