diff --git a/CHANGELOG.md b/CHANGELOG.md index 812cb2ee..ac63f7ed 100644 --- a/CHANGELOG.md +++ b/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 ---------------- diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 07fc028c..af352bdf 100644 --- a/src/ast/expr.rs +++ b/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)); - ff.field("name", &self.name) - .field("hash", &self.hashes) - .field("arg_exprs", &self.args); - if !self.constants.is_empty() { - ff.field("constant_args", &self.constants); + if let Some(ref ns) = self.namespace { + ff.field("namespace", ns); } 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, 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(..) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index d3b05744..8b87e79c 100644 --- a/src/eval/chaining.rs +++ b/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"))] diff --git a/src/optimizer.rs b/src/optimizer.rs index d069f7e2..fc37aebe 100644 --- a/src/optimizer.rs +++ b/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 - Expr::FnCall(x, ..) => for arg in x.args.iter_mut() { + // id(args ..) or xxx.id(args ..) -> optimize function call arguments + Expr::FnCall(x, ..) | Expr::MethodCall(x, ..) => for arg in x.args.iter_mut() { optimize_expr(arg, state, false); // Move constant arguments diff --git a/src/parser.rs b/src/parser.rs index d51ea346..f96c2e1c 100644 --- a/src/parser.rs +++ b/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(); diff --git a/tests/method_call.rs b/tests/method_call.rs index a65b511a..a9f2c4b7 100644 --- a/tests/method_call.rs +++ b/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> { - #[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> { Ok(()) } + +#[cfg(not(feature = "no_optimize"))] +#[test] +fn test_method_call_with_full_optimization() -> Result<(), Box> { + 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::( + " + let xs = new_ts(); + let ys = xs.range(ymd(2022, 2, 1), ymd(2022, 2, 2)); + ys + " + )?, + TestStruct::new() + ); + + Ok(()) +}