From 005692ef7898abb0f86e796d3dbce93c64b9f5ed Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 6 Jun 2022 08:54:19 +0800 Subject: [PATCH 01/14] Change volatile API message. --- src/api/events.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/events.rs b/src/api/events.rs index ec7b5022..f77633c4 100644 --- a/src/api/events.rs +++ b/src/api/events.rs @@ -68,7 +68,7 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[deprecated = "This API is volatile and may change in the future."] + #[deprecated = "This API is NOT deprecated, but it is considered volatile and may change in the future."] #[inline(always)] pub fn on_var( &mut self, @@ -130,7 +130,7 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[deprecated = "This API is volatile and may change in the future."] + #[deprecated = "This API is NOT deprecated, but it is considered volatile and may change in the future."] #[inline(always)] pub fn on_def_var( &mut self, @@ -188,7 +188,7 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[deprecated = "This API is volatile and may change in the future."] + #[deprecated = "This API is NOT deprecated, but it is considered volatile and may change in the future."] #[cfg(feature = "internals")] #[inline(always)] pub fn on_parse_token( @@ -343,7 +343,7 @@ impl Engine { /// # WARNING - Unstable API /// /// This API is volatile and may change in the future. - #[deprecated = "This API is volatile and may change in the future."] + #[deprecated = "This API is NOT deprecated, but it is considered volatile and may change in the future."] #[cfg(feature = "debugging")] #[inline(always)] pub fn register_debugger( From 84e329655939ac240eae4a21089ee92726be10b1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 7 Jun 2022 11:31:46 +0800 Subject: [PATCH 02/14] Fix bug on chaining function calls returning shared values. --- CHANGELOG.md | 1 + src/eval/chaining.rs | 9 ++++++++- tests/closures.rs | 30 +++++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b953235b..11d4a28b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Bug fixes * Parsing of index expressions is relaxed and many cases no longer result in an index-type error to allow for custom indexers. * Merging or combining a self-contained `AST` into another `AST` now works properly. * Plugin modules/functions no longer generate errors under `#![deny(missing_docs)]`. +* Calling a property on a function call that returns a shared value no longer causes an error. Deprecated API's ---------------- diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 2d9899c5..a5426ca1 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -652,7 +652,9 @@ impl Engine { _ if is_assignment => unreachable!("cannot assign to an expression"), // {expr}.??? or {expr}[???] expr => { - let value = self.eval_expr(scope, global, caches, lib, this_ptr, expr, level)?; + let value = self + .eval_expr(scope, global, caches, lib, this_ptr, expr, level)? + .flatten(); let obj_ptr = &mut value.into(); let root = ("", expr.start_position()); self.eval_dot_index_chain_helper( @@ -1043,6 +1045,11 @@ impl Engine { }) } + #[cfg(not(feature = "no_closure"))] + Dynamic(Union::Shared(..)) => { + unreachable!("`get_indexed_mut` cannot handle shared values") + } + _ if use_indexers => self .call_indexer_get(global, caches, lib, target, &mut idx, level) .map(Into::into), diff --git a/tests/closures.rs b/tests/closures.rs index 36ae9e33..f7c1b291 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Engine, EvalAltResult, FnPtr, ParseErrorType, Scope, INT}; +use rhai::{Dynamic, Engine, EvalAltResult, FnPtr, ParseErrorType, Scope, INT}; use std::any::TypeId; use std::cell::RefCell; use std::mem::take; @@ -217,6 +217,34 @@ fn test_closures_sharing() -> Result<(), Box> { 6 ); + #[cfg(not(feature = "no_object"))] + { + let mut m = Map::new(); + m.insert("hello".into(), "world".into()); + let m = Dynamic::from(m).into_shared(); + + engine.register_fn("baz", move || m.clone()); + + assert!(!engine.eval::( + " + let m = baz(); + m.is_shared() + " + )?); + + assert_eq!( + engine.eval::( + " + let m = baz(); + m.hello + " + )?, + "world" + ); + + assert_eq!(engine.eval::("baz().hello")?, "world"); + } + Ok(()) } From 8501d9d33f09eebaed6b7ffd77ca0eed9d16709f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 7 Jun 2022 20:38:05 +0800 Subject: [PATCH 03/14] Improve speed on common dot/index expressions. --- src/ast/expr.rs | 4 ++++ src/eval/chaining.rs | 51 +++++++++++++++++++++++++++++++++----------- src/eval/stmt.rs | 4 +--- src/eval/target.rs | 14 ++++++------ src/func/call.rs | 4 +--- 5 files changed, 51 insertions(+), 26 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 263b4eca..75d70dbc 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -401,6 +401,10 @@ pub enum Expr { /// func `(` expr `,` ... `)` FnCall(Box, Position), /// lhs `.` rhs + /// + /// ### Flags + /// + /// No flags are defined at this time. Use [`NONE`][ASTFlags::NONE]. Dot(Box, ASTFlags, Position), /// lhs `[` rhs `]` /// diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index a5426ca1..159a7a54 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -118,7 +118,7 @@ impl ChainArgument { impl Engine { /// Chain-evaluate a dot/index chain. - /// [`Position`] in [`EvalAltResult`] is [`NONE`][Position::NONE] and must be set afterwards. + /// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards. fn eval_dot_index_chain_helper( &self, global: &mut GlobalRuntimeState, @@ -620,11 +620,41 @@ impl Engine { let idx_values = &mut StaticVec::new_const(); - self.eval_dot_index_chain_arguments( - scope, global, caches, lib, this_ptr, rhs, options, chain_type, idx_values, 0, level, - )?; + match rhs { + // Short-circuit for simple property access: {expr}.prop + Expr::Property(..) if chain_type == ChainType::Dotting => { + idx_values.push(ChainArgument::Property(rhs.position())) + } + Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), + // Short-circuit for simple method call: {expr}.func() + Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => { + idx_values.push(ChainArgument::MethodCallArgs( + Default::default(), + Position::NONE, + )) + } + // Short-circuit for method call with all literal arguments: {expr}.func(1, 2, 3) + Expr::FnCall(x, ..) + if chain_type == ChainType::Dotting && x.args.iter().all(Expr::is_constant) => + { + let args: Vec<_> = x + .args + .iter() + .map(|expr| expr.get_literal_value().unwrap()) + .collect(); - let is_assignment = new_val.is_some(); + idx_values.push(ChainArgument::MethodCallArgs( + args.into_boxed_slice(), + x.args[0].position(), + )) + } + _ => { + self.eval_dot_index_chain_arguments( + scope, global, caches, lib, this_ptr, rhs, options, chain_type, idx_values, 0, + level, + )?; + } + } match lhs { // id.??? or id[???] @@ -645,11 +675,9 @@ impl Engine { global, caches, lib, &mut None, obj_ptr, root, expr, rhs, options, idx_values, chain_type, level, new_val, ) - .map(|(v, ..)| v) - .map_err(|err| err.fill_position(op_pos)) } // {expr}.??? = ??? or {expr}[???] = ??? - _ if is_assignment => unreachable!("cannot assign to an expression"), + _ if new_val.is_some() => unreachable!("cannot assign to an expression"), // {expr}.??? or {expr}[???] expr => { let value = self @@ -657,14 +685,15 @@ impl Engine { .flatten(); let obj_ptr = &mut value.into(); let root = ("", expr.start_position()); + self.eval_dot_index_chain_helper( global, caches, lib, this_ptr, obj_ptr, root, expr, rhs, options, idx_values, chain_type, level, new_val, ) - .map(|(v, ..)| if is_assignment { Dynamic::UNIT } else { v }) - .map_err(|err| err.fill_position(op_pos)) } } + .map(|(v, ..)| v) + .map_err(|err| err.fill_position(op_pos)) } /// Evaluate a chain of indexes and store the results in a [`StaticVec`]. @@ -798,7 +827,6 @@ impl Engine { } /// Call a get indexer. - /// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards. #[inline(always)] fn call_indexer_get( &self, @@ -822,7 +850,6 @@ impl Engine { } /// Call a set indexer. - /// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards. #[inline(always)] fn call_indexer_set( &self, diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index b8d11bd7..0ae9c2fb 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -184,9 +184,7 @@ impl Engine { *target.as_mut() = new_val; } - target - .propagate_changed_value() - .map_err(|err| err.fill_position(op_info.pos)) + target.propagate_changed_value(op_info.pos) } /// Evaluate a statement. diff --git a/src/eval/target.rs b/src/eval/target.rs index 69d67440..2e462677 100644 --- a/src/eval/target.rs +++ b/src/eval/target.rs @@ -1,7 +1,7 @@ //! Type to hold a mutable reference to the target of an evaluation. use crate::types::dynamic::Variant; -use crate::{Dynamic, RhaiResultOf}; +use crate::{Dynamic, Position, RhaiResultOf}; use std::ops::{Deref, DerefMut}; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -272,10 +272,8 @@ impl<'a> Target<'a> { } /// Propagate a changed value back to the original source. /// This has no effect for direct references. - /// - /// [`Position`] in [`EvalAltResult`] is [`NONE`][Position::NONE] and should be set afterwards. #[inline] - pub fn propagate_changed_value(&mut self) -> RhaiResultOf<()> { + pub fn propagate_changed_value(&mut self, pos: Position) -> RhaiResultOf<()> { match self { Self::RefMut(..) | Self::TempValue(..) => (), #[cfg(not(feature = "no_closure"))] @@ -287,7 +285,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "bool".to_string(), err.to_string(), - crate::Position::NONE, + pos, )) })?; @@ -317,7 +315,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "integer".to_string(), err.to_string(), - crate::Position::NONE, + pos, )) })?; @@ -338,7 +336,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "INT".to_string(), err.to_string(), - crate::Position::NONE, + pos, )) })?; @@ -363,7 +361,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "char".to_string(), err.to_string(), - crate::Position::NONE, + pos, )) })?; diff --git a/src/func/call.rs b/src/func/call.rs index ea1355d4..9852d18b 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -920,9 +920,7 @@ impl Engine { // Propagate the changed value back to the source if necessary if updated { - target - .propagate_changed_value() - .map_err(|err| err.fill_position(pos))?; + target.propagate_changed_value(pos)?; } Ok((result, updated)) From 8615960cd69230c379b6f0824c42ef7d538058fe Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 7 Jun 2022 20:52:04 +0800 Subject: [PATCH 04/14] Fix feature. --- src/eval/chaining.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 159a7a54..013b472f 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -622,11 +622,14 @@ impl Engine { match rhs { // Short-circuit for simple property access: {expr}.prop + #[cfg(not(feature = "no_object"))] Expr::Property(..) if chain_type == ChainType::Dotting => { idx_values.push(ChainArgument::Property(rhs.position())) } + #[cfg(not(feature = "no_object"))] Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), // Short-circuit for simple method call: {expr}.func() + #[cfg(not(feature = "no_object"))] Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => { idx_values.push(ChainArgument::MethodCallArgs( Default::default(), @@ -634,6 +637,7 @@ impl Engine { )) } // Short-circuit for method call with all literal arguments: {expr}.func(1, 2, 3) + #[cfg(not(feature = "no_object"))] Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.iter().all(Expr::is_constant) => { From f4ebaa7abfbd8546a242a64799131ab3b2d9126b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 8 Jun 2022 09:19:21 +0800 Subject: [PATCH 05/14] Improve chaining speed. --- CHANGELOG.md | 1 + src/eval/chaining.rs | 235 +++++++++++++------------------------------ src/eval/mod.rs | 2 +- 3 files changed, 70 insertions(+), 168 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11d4a28b..546da8b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Deprecated API's Enhancements ------------ +* Indexing and property access are now faster. * `EvalAltResult::IndexNotFound` is added to aid in raising errors for indexers. * `Engine::def_tag`, `Engine::def_tag_mut` and `Engine::set_tag` are added to manage a default value for the custom evaluation state, accessible via `EvalState::tag()` (which is the same as `NativeCallContext::tag()`). * Originally, the debugger's custom state uses the same state as `EvalState::tag()` (which is the same as `NativeCallContext::tag()`). It is now split into its own variable accessible under `Debugger::state()`. diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 013b472f..9b1bc2bf 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -4,7 +4,7 @@ use super::{Caches, GlobalRuntimeState, Target}; use crate::ast::{ASTFlags, Expr, OpAssignment}; use crate::types::dynamic::Union; -use crate::{Dynamic, Engine, Module, Position, RhaiResult, RhaiResultOf, Scope, StaticVec, ERR}; +use crate::{Dynamic, Engine, FnArgsVec, Module, Position, RhaiResult, RhaiResultOf, Scope, ERR}; use std::hash::Hash; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -33,89 +33,6 @@ impl From<&Expr> for ChainType { } } -/// Value of a chaining argument. -#[derive(Debug, Clone, Hash)] -pub enum ChainArgument { - /// Dot-property access. - #[cfg(not(feature = "no_object"))] - Property(Position), - /// Arguments to a dot method call. - /// Wrapped values are the arguments plus the [position][Position] of the first argument. - /// - /// Since many dotted function calls have no arguments (e.g. `string.len()`), it is better to - /// reduce the size of [`ChainArgument`] by using a boxed slice. - #[cfg(not(feature = "no_object"))] - MethodCallArgs(Box<[Dynamic]>, Position), - /// Index value and [position][Position]. - #[cfg(not(feature = "no_index"))] - IndexValue(Dynamic, Position), -} - -impl ChainArgument { - /// Return the index value. - #[inline(always)] - #[cfg(not(feature = "no_index"))] - #[must_use] - pub fn into_index_value(self) -> Option { - match self { - Self::IndexValue(value, ..) => Some(value), - #[cfg(not(feature = "no_object"))] - _ => None, - } - } - /// Return the list of method call arguments. - /// - /// # Panics - /// - /// Panics if the [`ChainArgument`] is not [`MethodCallArgs`][ChainArgument::MethodCallArgs]. - #[inline(always)] - #[cfg(not(feature = "no_object"))] - #[must_use] - pub fn into_fn_call_args(self) -> (crate::FnArgsVec, Position) { - match self { - Self::MethodCallArgs(values, pos) if values.is_empty() => { - (crate::FnArgsVec::new_const(), pos) - } - Self::MethodCallArgs(mut values, pos) => { - (values.iter_mut().map(std::mem::take).collect(), pos) - } - x => unreachable!("ChainArgument::MethodCallArgs expected but gets {:?}", x), - } - } - /// Return the [position][Position]. - #[inline(always)] - #[must_use] - #[allow(dead_code)] - pub const fn position(&self) -> Position { - match self { - #[cfg(not(feature = "no_object"))] - ChainArgument::Property(pos) => *pos, - #[cfg(not(feature = "no_object"))] - ChainArgument::MethodCallArgs(.., pos) => *pos, - #[cfg(not(feature = "no_index"))] - ChainArgument::IndexValue(.., pos) => *pos, - } - } - /// Create n [`MethodCallArgs`][ChainArgument::MethodCallArgs]. - #[inline(always)] - #[cfg(not(feature = "no_object"))] - #[must_use] - pub fn from_fn_call_args(values: crate::FnArgsVec, pos: Position) -> Self { - if values.is_empty() { - Self::MethodCallArgs(Box::default(), pos) - } else { - Self::MethodCallArgs(values.into_boxed_slice(), pos) - } - } - /// Create an [`IndexValue`][ChainArgument::IndexValue]. - #[inline(always)] - #[cfg(not(feature = "no_index"))] - #[must_use] - pub const fn from_index_value(value: Dynamic, pos: Position) -> Self { - Self::IndexValue(value, pos) - } -} - impl Engine { /// Chain-evaluate a dot/index chain. /// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards. @@ -130,16 +47,13 @@ impl Engine { _parent: &Expr, rhs: &Expr, _parent_options: ASTFlags, - idx_values: &mut StaticVec, + idx_values: &mut FnArgsVec, chain_type: ChainType, level: usize, new_val: Option<(Dynamic, OpAssignment)>, ) -> RhaiResultOf<(Dynamic, bool)> { let is_ref_mut = target.is_ref(); - // Pop the last index value - let idx_val = idx_values.pop().unwrap(); - #[cfg(feature = "debugging")] let scope = &mut Scope::new(); @@ -147,7 +61,6 @@ impl Engine { #[cfg(not(feature = "no_index"))] ChainType::Indexing => { let pos = rhs.start_position(); - let idx_val = idx_val.into_index_value().expect("`ChainType::Index`"); match rhs { // xxx[idx].expr... | xxx[idx][expr]... @@ -157,6 +70,7 @@ impl Engine { #[cfg(feature = "debugging")] self.run_debugger(scope, global, lib, this_ptr, _parent, level)?; + let idx_val = idx_values.pop().unwrap(); let mut idx_val_for_setter = idx_val.clone(); let idx_pos = x.lhs.start_position(); let rhs_chain = rhs.into(); @@ -201,6 +115,7 @@ impl Engine { self.run_debugger(scope, global, lib, this_ptr, _parent, level)?; let (new_val, op_info) = new_val.expect("`Some`"); + let idx_val = idx_values.pop().unwrap(); let mut idx_val2 = idx_val.clone(); let try_setter = match self.get_indexed_mut( @@ -259,6 +174,8 @@ impl Engine { #[cfg(feature = "debugging")] self.run_debugger(scope, global, lib, this_ptr, _parent, level)?; + let idx_val = idx_values.pop().unwrap(); + self.get_indexed_mut( global, caches, lib, target, idx_val, pos, false, true, level, ) @@ -272,13 +189,19 @@ impl Engine { match rhs { // xxx.fn_name(arg_expr_list) 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(); - #[cfg(feature = "debugging")] let reset_debugger = self.run_debugger_with_reset(scope, global, lib, this_ptr, rhs, level)?; + let crate::ast::FnCallExpr { + name, hashes, args, .. + } = x.as_ref(); + + let call_args = &mut ( + idx_values.drain(idx_values.len() - args.len()..).collect(), + args.get(0).map_or(Position::NONE, Expr::position), + ); + let result = self.make_method_call( global, caches, lib, name, *hashes, target, call_args, *pos, level, ); @@ -440,14 +363,20 @@ impl Engine { } // {xxx:map}.fn_name(arg_expr_list)[expr] | {xxx:map}.fn_name(arg_expr_list).expr 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(); - #[cfg(feature = "debugging")] let reset_debugger = self.run_debugger_with_reset( scope, global, lib, this_ptr, _node, level, )?; + let crate::ast::FnCallExpr { + name, hashes, args, .. + } = x.as_ref(); + + let call_args = &mut ( + idx_values.drain(idx_values.len() - args.len()..).collect(), + args.get(0).map_or(Position::NONE, Expr::position), + ); + let result = self.make_method_call( global, caches, lib, name, *hashes, target, call_args, pos, level, @@ -558,17 +487,24 @@ impl Engine { } // xxx.fn_name(arg_expr_list)[expr] | xxx.fn_name(arg_expr_list).expr 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(); - #[cfg(feature = "debugging")] let reset_debugger = self.run_debugger_with_reset( scope, global, lib, this_ptr, _node, level, )?; + let crate::ast::FnCallExpr { + name, hashes, args, .. + } = f.as_ref(); + let rhs_chain = rhs.into(); + + let call_args = &mut ( + idx_values.drain(idx_values.len() - args.len()..).collect(), + args.get(0).map_or(Position::NONE, Expr::position), + ); + let result = self.make_method_call( - global, caches, lib, name, *hashes, target, args, pos, level, + global, caches, lib, name, *hashes, target, call_args, pos, + level, ); #[cfg(feature = "debugging")] @@ -618,39 +554,26 @@ impl Engine { expr => unreachable!("Expr::Index or Expr::Dot expected but gets {:?}", expr), }; - let idx_values = &mut StaticVec::new_const(); + let idx_values = &mut FnArgsVec::new_const(); match rhs { // Short-circuit for simple property access: {expr}.prop #[cfg(not(feature = "no_object"))] - Expr::Property(..) if chain_type == ChainType::Dotting => { - idx_values.push(ChainArgument::Property(rhs.position())) - } + Expr::Property(..) if chain_type == ChainType::Dotting => (), #[cfg(not(feature = "no_object"))] Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), // Short-circuit for simple method call: {expr}.func() #[cfg(not(feature = "no_object"))] - Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => { - idx_values.push(ChainArgument::MethodCallArgs( - Default::default(), - Position::NONE, - )) - } + Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => (), // Short-circuit for method call with all literal arguments: {expr}.func(1, 2, 3) #[cfg(not(feature = "no_object"))] Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.iter().all(Expr::is_constant) => { - let args: Vec<_> = x - .args + x.args .iter() .map(|expr| expr.get_literal_value().unwrap()) - .collect(); - - idx_values.push(ChainArgument::MethodCallArgs( - args.into_boxed_slice(), - x.args[0].position(), - )) + .for_each(|v| idx_values.push(v)) } _ => { self.eval_dot_index_chain_arguments( @@ -713,7 +636,7 @@ impl Engine { expr: &Expr, parent_options: ASTFlags, _parent_chain_type: ChainType, - idx_values: &mut StaticVec, + idx_values: &mut FnArgsVec, size: usize, level: usize, ) -> RhaiResultOf<()> { @@ -725,22 +648,11 @@ impl Engine { Expr::MethodCall(x, ..) if _parent_chain_type == ChainType::Dotting && !x.is_qualified() => { - let crate::ast::FnCallExpr { args, .. } = x.as_ref(); - - let (values, pos) = args.iter().try_fold( - (crate::FnArgsVec::with_capacity(args.len()), Position::NONE), - |(mut values, mut pos), expr| { - let (value, arg_pos) = - self.get_arg_value(scope, global, caches, lib, this_ptr, expr, level)?; - if values.is_empty() { - pos = arg_pos; - } - values.push(value.flatten()); - Ok::<_, crate::RhaiError>((values, pos)) - }, - )?; - - idx_values.push(ChainArgument::from_fn_call_args(values, pos)); + for arg_expr in x.args.as_ref() { + let (value, _) = + self.get_arg_value(scope, global, caches, lib, this_ptr, arg_expr, level)?; + idx_values.push(value.flatten()); + } } #[cfg(not(feature = "no_object"))] Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => { @@ -748,9 +660,7 @@ impl Engine { } #[cfg(not(feature = "no_object"))] - Expr::Property(.., pos) if _parent_chain_type == ChainType::Dotting => { - idx_values.push(ChainArgument::Property(*pos)) - } + Expr::Property(..) if _parent_chain_type == ChainType::Dotting => (), Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), Expr::Index(x, options, ..) | Expr::Dot(x, options, ..) @@ -758,34 +668,24 @@ impl Engine { { let crate::ast::BinaryExpr { lhs, rhs, .. } = x.as_ref(); + let mut arg_values = FnArgsVec::new(); + // Evaluate in left-to-right order - let lhs_arg_val = match lhs { + match lhs { #[cfg(not(feature = "no_object"))] - Expr::Property(.., pos) if _parent_chain_type == ChainType::Dotting => { - ChainArgument::Property(*pos) - } + Expr::Property(..) if _parent_chain_type == ChainType::Dotting => (), Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), #[cfg(not(feature = "no_object"))] Expr::MethodCall(x, ..) if _parent_chain_type == ChainType::Dotting && !x.is_qualified() => { - let crate::ast::FnCallExpr { args, .. } = x.as_ref(); - - let (values, pos) = args.iter().try_fold( - (crate::FnArgsVec::with_capacity(args.len()), Position::NONE), - |(mut values, mut pos), expr| { - let (value, arg_pos) = self.get_arg_value( - scope, global, caches, lib, this_ptr, expr, level, - )?; - if values.is_empty() { - pos = arg_pos - } - values.push(value.flatten()); - Ok::<_, crate::RhaiError>((values, pos)) - }, - )?; - ChainArgument::from_fn_call_args(values, pos) + for arg_expr in x.args.as_ref() { + let (value, _) = self.get_arg_value( + scope, global, caches, lib, this_ptr, arg_expr, level, + )?; + arg_values.push(value.flatten()); + } } #[cfg(not(feature = "no_object"))] Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => { @@ -796,13 +696,14 @@ impl Engine { unreachable!("invalid dot expression: {:?}", expr); } #[cfg(not(feature = "no_index"))] - _ if _parent_chain_type == ChainType::Indexing => self - .eval_expr(scope, global, caches, lib, this_ptr, lhs, level) - .map(|v| { - ChainArgument::from_index_value(v.flatten(), lhs.start_position()) - })?, + _ if _parent_chain_type == ChainType::Indexing => { + arg_values.push( + self.eval_expr(scope, global, caches, lib, this_ptr, lhs, level)? + .flatten(), + ); + } expr => unreachable!("unknown chained expression: {:?}", expr), - }; + } // Push in reverse order let chain_type = expr.into(); @@ -812,7 +713,7 @@ impl Engine { size, level, )?; - idx_values.push(lhs_arg_val); + idx_values.extend(arg_values); } #[cfg(not(feature = "no_object"))] @@ -821,8 +722,8 @@ impl Engine { } #[cfg(not(feature = "no_index"))] _ if _parent_chain_type == ChainType::Indexing => idx_values.push( - self.eval_expr(scope, global, caches, lib, this_ptr, expr, level) - .map(|v| ChainArgument::from_index_value(v.flatten(), expr.start_position()))?, + self.eval_expr(scope, global, caches, lib, this_ptr, expr, level)? + .flatten(), ), _ => unreachable!("unknown chained expression: {:?}", expr), } diff --git a/src/eval/mod.rs b/src/eval/mod.rs index ea11a5d8..2daf2432 100644 --- a/src/eval/mod.rs +++ b/src/eval/mod.rs @@ -10,7 +10,7 @@ mod target; pub use cache::{Caches, FnResolutionCache, FnResolutionCacheEntry}; #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] -pub use chaining::{ChainArgument, ChainType}; +pub use chaining::ChainType; #[cfg(feature = "debugging")] pub use debugger::{ BreakPoint, CallStackFrame, Debugger, DebuggerCommand, DebuggerEvent, DebuggerStatus, From bbaad8dfcbbf056fe41f6d081bf3a15107b229ac Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 8 Jun 2022 16:34:56 +0800 Subject: [PATCH 06/14] Speed up method calls. --- src/eval/chaining.rs | 68 ++++++++++++++++++++++++++------------------ src/eval/debugger.rs | 2 +- src/eval/target.rs | 2 +- src/func/call.rs | 60 +++++++++++++++++++++++++++++++------- 4 files changed, 91 insertions(+), 41 deletions(-) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 9b1bc2bf..1e1e8de5 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -197,15 +197,17 @@ impl Engine { name, hashes, args, .. } = x.as_ref(); - let call_args = &mut ( - idx_values.drain(idx_values.len() - args.len()..).collect(), - args.get(0).map_or(Position::NONE, Expr::position), - ); + let offset = idx_values.len() - args.len(); + let call_args = &mut idx_values[offset..]; + let pos1 = args.get(0).map_or(Position::NONE, Expr::position); let result = self.make_method_call( - global, caches, lib, name, *hashes, target, call_args, *pos, level, + global, caches, lib, name, *hashes, target, call_args, pos1, *pos, + level, ); + idx_values.truncate(offset); + #[cfg(feature = "debugging")] global.debugger.reset_status(reset_debugger); @@ -372,16 +374,17 @@ impl Engine { name, hashes, args, .. } = x.as_ref(); - let call_args = &mut ( - idx_values.drain(idx_values.len() - args.len()..).collect(), - args.get(0).map_or(Position::NONE, Expr::position), - ); + let offset = idx_values.len() - args.len(); + let call_args = &mut idx_values[offset..]; + let pos1 = args.get(0).map_or(Position::NONE, Expr::position); let result = self.make_method_call( - global, caches, lib, name, *hashes, target, call_args, pos, - level, + global, caches, lib, name, *hashes, target, call_args, pos1, + pos, level, ); + idx_values.truncate(offset); + #[cfg(feature = "debugging")] global.debugger.reset_status(reset_debugger); @@ -497,16 +500,17 @@ impl Engine { } = f.as_ref(); let rhs_chain = rhs.into(); - let call_args = &mut ( - idx_values.drain(idx_values.len() - args.len()..).collect(), - args.get(0).map_or(Position::NONE, Expr::position), - ); + let offset = idx_values.len() - args.len(); + let call_args = &mut idx_values[offset..]; + let pos1 = args.get(0).map_or(Position::NONE, Expr::position); let result = self.make_method_call( - global, caches, lib, name, *hashes, target, call_args, pos, - level, + global, caches, lib, name, *hashes, target, call_args, pos1, + pos, level, ); + idx_values.truncate(offset); + #[cfg(feature = "debugging")] global.debugger.reset_status(reset_debugger); @@ -570,11 +574,14 @@ impl Engine { Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.iter().all(Expr::is_constant) => { - x.args - .iter() - .map(|expr| expr.get_literal_value().unwrap()) - .for_each(|v| idx_values.push(v)) + idx_values.extend(x.args.iter().map(|expr| expr.get_literal_value().unwrap())) } + // Short-circuit for indexing with literal: {expr}[1] + #[cfg(not(feature = "no_index"))] + _ if chain_type == ChainType::Indexing && rhs.is_constant() => { + idx_values.push(rhs.get_literal_value().unwrap()) + } + // All other patterns - evaluate the arguments chain _ => { self.eval_dot_index_chain_arguments( scope, global, caches, lib, this_ptr, rhs, options, chain_type, idx_values, 0, @@ -649,9 +656,11 @@ impl Engine { if _parent_chain_type == ChainType::Dotting && !x.is_qualified() => { for arg_expr in x.args.as_ref() { - let (value, _) = - self.get_arg_value(scope, global, caches, lib, this_ptr, arg_expr, level)?; - idx_values.push(value.flatten()); + idx_values.push( + self.get_arg_value(scope, global, caches, lib, this_ptr, arg_expr, level)? + .0 + .flatten(), + ); } } #[cfg(not(feature = "no_object"))] @@ -681,10 +690,13 @@ impl Engine { if _parent_chain_type == ChainType::Dotting && !x.is_qualified() => { for arg_expr in x.args.as_ref() { - let (value, _) = self.get_arg_value( - scope, global, caches, lib, this_ptr, arg_expr, level, - )?; - arg_values.push(value.flatten()); + arg_values.push( + self.get_arg_value( + scope, global, caches, lib, this_ptr, arg_expr, level, + )? + .0 + .flatten(), + ); } } #[cfg(not(feature = "no_object"))] diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index 617b1db4..21f9e739 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -315,7 +315,7 @@ impl Debugger { /// Change the current status to [`CONTINUE`][DebuggerStatus::CONTINUE] and return the previous status. pub(crate) fn clear_status_if( &mut self, - filter: impl Fn(&DebuggerStatus) -> bool, + filter: impl FnOnce(&DebuggerStatus) -> bool, ) -> Option { if filter(&self.status) { Some(mem::replace(&mut self.status, DebuggerStatus::CONTINUE)) diff --git a/src/eval/target.rs b/src/eval/target.rs index 2e462677..a2c4d34c 100644 --- a/src/eval/target.rs +++ b/src/eval/target.rs @@ -46,7 +46,7 @@ pub fn calc_index( length: usize, start: crate::INT, negative_count_from_end: bool, - err: impl Fn() -> Result, + err: impl FnOnce() -> Result, ) -> Result { if start < 0 { if negative_count_from_end { diff --git a/src/func/call.rs b/src/func/call.rs index 9852d18b..6cee575e 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -782,8 +782,9 @@ impl Engine { fn_name: &str, mut hash: FnCallHashes, target: &mut crate::eval::Target, - (call_args, call_arg_pos): &mut (FnArgsVec, Position), - pos: Position, + mut call_args: &mut [Dynamic], + first_arg_pos: Position, + fn_call_pos: Position, level: usize, ) -> RhaiResultOf<(Dynamic, bool)> { let is_ref_mut = target.is_ref(); @@ -806,7 +807,16 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - None, global, caches, lib, fn_name, new_hash, &mut args, false, false, pos, + None, + global, + caches, + lib, + fn_name, + new_hash, + &mut args, + false, + false, + fn_call_pos, level, ) } @@ -814,15 +824,17 @@ impl Engine { if !call_args.is_empty() { if !call_args[0].is::() { let typ = self.map_type_name(call_args[0].type_name()); - return Err(self.make_type_mismatch_err::(typ, *call_arg_pos)); + return Err(self.make_type_mismatch_err::(typ, first_arg_pos)); } } else { let typ = self.map_type_name(target.type_name()); - return Err(self.make_type_mismatch_err::(typ, pos)); + return Err(self.make_type_mismatch_err::(typ, fn_call_pos)); } // FnPtr call on object - let fn_ptr = call_args.remove(0).cast::(); + let fn_ptr = mem::take(&mut call_args[0]).cast::(); + call_args = &mut call_args[1..]; + // Redirect function name let fn_name = fn_ptr.fn_name(); let args_len = call_args.len() + fn_ptr.curry().len(); @@ -842,14 +854,23 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - None, global, caches, lib, fn_name, new_hash, &mut args, is_ref_mut, true, pos, + None, + global, + caches, + lib, + fn_name, + new_hash, + &mut args, + is_ref_mut, + true, + fn_call_pos, level, ) } KEYWORD_FN_PTR_CURRY => { if !target.is::() { let typ = self.map_type_name(target.type_name()); - return Err(self.make_type_mismatch_err::(typ, pos)); + return Err(self.make_type_mismatch_err::(typ, fn_call_pos)); } let fn_ptr = target.read_lock::().expect("`FnPtr`"); @@ -883,6 +904,8 @@ impl Engine { _ => { let mut fn_name = fn_name; let _redirected; + let mut call_args = call_args; + let mut arg_values: FnArgsVec<_>; // Check if it is a map method call in OOP style #[cfg(not(feature = "no_object"))] @@ -894,7 +917,13 @@ impl Engine { fn_name = &_redirected; // Add curried arguments if fn_ptr.is_curried() { - call_args.insert_many(0, fn_ptr.curry().iter().cloned()); + arg_values = fn_ptr + .curry() + .iter() + .cloned() + .chain(call_args.iter_mut().map(mem::take)) + .collect(); + call_args = &mut arg_values; } // Recalculate the hash based on the new function name and new arguments hash = FnCallHashes::from_all( @@ -912,7 +941,16 @@ impl Engine { args.extend(call_args.iter_mut()); self.exec_fn_call( - None, global, caches, lib, fn_name, hash, &mut args, is_ref_mut, true, pos, + None, + global, + caches, + lib, + fn_name, + hash, + &mut args, + is_ref_mut, + true, + fn_call_pos, level, ) } @@ -920,7 +958,7 @@ impl Engine { // Propagate the changed value back to the source if necessary if updated { - target.propagate_changed_value(pos)?; + target.propagate_changed_value(fn_call_pos)?; } Ok((result, updated)) From e5f6b28abd8ab25d356f51d1f7bef15e0a2113bf Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 8 Jun 2022 17:06:49 +0800 Subject: [PATCH 07/14] Fix warnings. --- src/eval/chaining.rs | 17 ++++++++++------- src/eval/target.rs | 10 +++++----- src/func/call.rs | 6 +++--- src/func/register.rs | 8 ++++---- src/lib.rs | 2 ++ src/optimizer.rs | 8 ++++---- src/parser.rs | 2 +- 7 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 1e1e8de5..ba946764 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -550,11 +550,12 @@ impl Engine { level: usize, new_val: Option<(Dynamic, OpAssignment)>, ) -> RhaiResult { - let (crate::ast::BinaryExpr { lhs, rhs }, chain_type, options, op_pos) = match expr { + let chain_type = ChainType::from(expr); + let (crate::ast::BinaryExpr { lhs, rhs }, options, op_pos) = match expr { #[cfg(not(feature = "no_index"))] - Expr::Index(x, options, pos) => (x.as_ref(), ChainType::Indexing, *options, *pos), + Expr::Index(x, options, pos) => (x.as_ref(), *options, *pos), #[cfg(not(feature = "no_object"))] - Expr::Dot(x, options, pos) => (x.as_ref(), ChainType::Dotting, *options, *pos), + Expr::Dot(x, options, pos) => (x.as_ref(), *options, *pos), expr => unreachable!("Expr::Index or Expr::Dot expected but gets {:?}", expr), }; @@ -677,7 +678,7 @@ impl Engine { { let crate::ast::BinaryExpr { lhs, rhs, .. } = x.as_ref(); - let mut arg_values = FnArgsVec::new(); + let mut _arg_values = FnArgsVec::new_const(); // Evaluate in left-to-right order match lhs { @@ -690,7 +691,7 @@ impl Engine { if _parent_chain_type == ChainType::Dotting && !x.is_qualified() => { for arg_expr in x.args.as_ref() { - arg_values.push( + _arg_values.push( self.get_arg_value( scope, global, caches, lib, this_ptr, arg_expr, level, )? @@ -709,7 +710,7 @@ impl Engine { } #[cfg(not(feature = "no_index"))] _ if _parent_chain_type == ChainType::Indexing => { - arg_values.push( + _arg_values.push( self.eval_expr(scope, global, caches, lib, this_ptr, lhs, level)? .flatten(), ); @@ -725,7 +726,9 @@ impl Engine { size, level, )?; - idx_values.extend(arg_values); + if !_arg_values.is_empty() { + idx_values.extend(_arg_values); + } } #[cfg(not(feature = "no_object"))] diff --git a/src/eval/target.rs b/src/eval/target.rs index a2c4d34c..10456439 100644 --- a/src/eval/target.rs +++ b/src/eval/target.rs @@ -273,7 +273,7 @@ impl<'a> Target<'a> { /// Propagate a changed value back to the original source. /// This has no effect for direct references. #[inline] - pub fn propagate_changed_value(&mut self, pos: Position) -> RhaiResultOf<()> { + pub fn propagate_changed_value(&mut self, _pos: Position) -> RhaiResultOf<()> { match self { Self::RefMut(..) | Self::TempValue(..) => (), #[cfg(not(feature = "no_closure"))] @@ -285,7 +285,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "bool".to_string(), err.to_string(), - pos, + _pos, )) })?; @@ -315,7 +315,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "integer".to_string(), err.to_string(), - pos, + _pos, )) })?; @@ -336,7 +336,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "INT".to_string(), err.to_string(), - pos, + _pos, )) })?; @@ -361,7 +361,7 @@ impl<'a> Target<'a> { Box::new(crate::ERR::ErrorMismatchDataType( "char".to_string(), err.to_string(), - pos, + _pos, )) })?; diff --git a/src/func/call.rs b/src/func/call.rs index 6cee575e..62f92c0b 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -904,8 +904,8 @@ impl Engine { _ => { let mut fn_name = fn_name; let _redirected; + let mut _arg_values: FnArgsVec<_>; let mut call_args = call_args; - let mut arg_values: FnArgsVec<_>; // Check if it is a map method call in OOP style #[cfg(not(feature = "no_object"))] @@ -917,13 +917,13 @@ impl Engine { fn_name = &_redirected; // Add curried arguments if fn_ptr.is_curried() { - arg_values = fn_ptr + _arg_values = fn_ptr .curry() .iter() .cloned() .chain(call_args.iter_mut().map(mem::take)) .collect(); - call_args = &mut arg_values; + call_args = &mut _arg_values; } // Recalculate the hash based on the new function name and new arguments hash = FnCallHashes::from_all( diff --git a/src/func/register.rs b/src/func/register.rs index 5b08ab31..99e460f3 100644 --- a/src/func/register.rs +++ b/src/func/register.rs @@ -134,9 +134,9 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type() -> TypeId { TypeId::of::() } #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { - CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { + CallableFunction::$abi(Box::new(move |_ctx: NativeCallContext, args: &mut FnCallArgs| { // The arguments are assumed to be of the correct number and types! - check_constant!(ctx, args); + check_constant!(_ctx, args); let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().expect(EXPECT_ARGS)); )* @@ -186,9 +186,9 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type() -> TypeId { TypeId::of::>() } #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::>() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { - CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { + CallableFunction::$abi(Box::new(move |_ctx: NativeCallContext, args: &mut FnCallArgs| { // The arguments are assumed to be of the correct number and types! - check_constant!(ctx, args); + check_constant!(_ctx, args); let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().expect(EXPECT_ARGS)); )* diff --git a/src/lib.rs b/src/lib.rs index 8cd8b472..129b1f84 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -130,6 +130,7 @@ const INT_BITS: usize = std::mem::size_of::() * 8; /// Number of bytes that make up an [`INT`]. /// /// It is 8 unless the `only_i32` feature is enabled when it will be 4. +#[cfg(not(feature = "no_index"))] const INT_BYTES: usize = std::mem::size_of::(); /// The system floating-point type. It is defined as [`f64`]. @@ -155,6 +156,7 @@ pub type FLOAT = f32; /// /// It is 8 unless the `f32_float` feature is enabled when it will be 4. #[cfg(not(feature = "no_float"))] +#[cfg(not(feature = "no_index"))] const FLOAT_BYTES: usize = std::mem::size_of::(); /// An exclusive integer range. diff --git a/src/optimizer.rs b/src/optimizer.rs index fe3e9101..bf5f5e1e 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -10,7 +10,7 @@ use crate::tokenizer::{Span, Token}; use crate::types::dynamic::AccessMode; use crate::{ calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, FnPtr, Identifier, - Position, Scope, StaticVec, AST, INT, INT_BITS, + Position, Scope, StaticVec, AST, INT, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -966,16 +966,16 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { .unwrap_or_else(|| Expr::Unit(*pos)); } // int[int] - (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && (*i as usize) < INT_BITS => { + (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && (*i as usize) < crate::INT_BITS => { // Bit-field literal indexing - get the bit state.set_dirty(); *expr = Expr::BoolConstant((*n & (1 << (*i as usize))) != 0, *pos); } // int[-int] - (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.checked_abs().map(|i| i as usize <= INT_BITS).unwrap_or(false) => { + (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.checked_abs().map(|i| i as usize <= crate::INT_BITS).unwrap_or(false) => { // Bit-field literal indexing - get the bit state.set_dirty(); - *expr = Expr::BoolConstant((*n & (1 << (INT_BITS - i.abs() as usize))) != 0, *pos); + *expr = Expr::BoolConstant((*n & (1 << (crate::INT_BITS - i.abs() as usize))) != 0, *pos); } // string[int] (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && (*i as usize) < s.chars().count() => { diff --git a/src/parser.rs b/src/parser.rs index b25073f7..d689ce83 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2904,7 +2904,7 @@ impl Engine { } }; - let mut statements = StaticVec::new(); + let mut statements = StaticVec::new_const(); let prev_entry_stack_len = state.block_stack_len; state.block_stack_len = state.stack.len(); From 285bf23dfafcc0cc7a65c902b361072c97ab1f25 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 9 Jun 2022 08:41:51 +0800 Subject: [PATCH 08/14] Minor refactor. --- src/api/call_fn.rs | 6 +++--- src/ast/ast.rs | 25 ++++++++----------------- src/parser.rs | 4 ++-- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/api/call_fn.rs b/src/api/call_fn.rs index 1f2ec085..dcbfbf99 100644 --- a/src/api/call_fn.rs +++ b/src/api/call_fn.rs @@ -162,7 +162,7 @@ impl Engine { ) -> RhaiResult { let mut arg_values = arg_values; - self.call_fn_internal( + self._call_fn( scope, &mut GlobalRuntimeState::new(self), &mut Caches::new(), @@ -215,7 +215,7 @@ impl Engine { this_ptr: Option<&mut Dynamic>, arg_values: &mut [Dynamic], ) -> RhaiResult { - self.call_fn_internal( + self._call_fn( scope, global, caches, @@ -228,7 +228,7 @@ impl Engine { ) } /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments. - fn call_fn_internal( + fn _call_fn( &self, scope: &mut Scope, global: &mut GlobalRuntimeState, diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 0b9ec418..108b204e 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -809,30 +809,21 @@ impl AST { /// Return `false` from the callback to terminate the walk. #[cfg(not(feature = "internals"))] #[cfg(not(feature = "no_module"))] - #[inline] + #[inline(always)] pub(crate) fn walk(&self, on_node: &mut impl FnMut(&[ASTNode]) -> bool) -> bool { - let path = &mut Vec::new(); - - for stmt in self.statements() { - if !stmt.walk(path, on_node) { - return false; - } - } - #[cfg(not(feature = "no_function"))] - for stmt in self.iter_fn_def().flat_map(|f| f.body.iter()) { - if !stmt.walk(path, on_node) { - return false; - } - } - - true + self.walk_raw(on_node) } /// _(internals)_ Recursively walk the [`AST`], including function bodies (if any). /// Return `false` from the callback to terminate the walk. /// Exported under the `internals` feature only. #[cfg(feature = "internals")] - #[inline] + #[inline(always)] pub fn walk(&self, on_node: &mut impl FnMut(&[ASTNode]) -> bool) -> bool { + self._walk(on_node) + } + /// Recursively walk the [`AST`], including function bodies (if any). + /// Return `false` from the callback to terminate the walk. + fn _walk(&self, on_node: &mut impl FnMut(&[ASTNode]) -> bool) -> bool { let path = &mut Vec::new(); for stmt in self.statements() { diff --git a/src/parser.rs b/src/parser.rs index d689ce83..0feeaf19 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -57,7 +57,7 @@ pub struct ParseState<'e> { pub block_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] - pub external_vars: Vec, + pub external_vars: crate::FnArgsVec, /// An indicator that disables variable capturing into externals one single time /// up until the nearest consumed Identifier token. /// If set to false the next call to [`access_var`][ParseState::access_var] will not capture the variable. @@ -80,7 +80,7 @@ impl<'e> ParseState<'e> { Self { tokenizer_control, #[cfg(not(feature = "no_closure"))] - external_vars: Vec::new(), + external_vars: crate::FnArgsVec::new_const(), #[cfg(not(feature = "no_closure"))] allow_capture: true, interned_strings: StringsInterner::new(), From dcaac20eb99002c6983ac27a6e2799b590ce0a91 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 9 Jun 2022 17:59:28 +0800 Subject: [PATCH 09/14] Strict mode in functions check for static modules. --- CHANGELOG.md | 2 ++ src/eval/expr.rs | 16 ++++++---- src/parser.rs | 69 ++++++++++++++++++++------------------------ src/types/dynamic.rs | 4 +-- tests/options.rs | 11 ++++--- 5 files changed, 50 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 546da8b6..381ca6ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Bug fixes * Merging or combining a self-contained `AST` into another `AST` now works properly. * Plugin modules/functions no longer generate errors under `#![deny(missing_docs)]`. * Calling a property on a function call that returns a shared value no longer causes an error. +* _Strict Variables Mode_ now checks for module namespaces within functions as well. +* Module defined via `Engine::register_static_module` are now checked in _Strict Variables Mode_. Deprecated API's ---------------- diff --git a/src/eval/expr.rs b/src/eval/expr.rs index d0dce935..7077637c 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -32,13 +32,17 @@ impl Engine { if let Some(index) = index { let offset = global.num_imports() - index.get(); - Some(global.get_shared_import(offset).unwrap()) - } else { - global - .find_import(root) - .map(|n| global.get_shared_import(n).unwrap()) - .or_else(|| self.global_sub_modules.get(root).cloned()) + + if let m @ Some(_) = global.get_shared_import(offset) { + return m; + } } + + // Do a text-match search if the index doesn't work + global.find_import(root).map_or_else( + || self.global_sub_modules.get(root).cloned(), + |offset| global.get_shared_import(offset), + ) } /// Search for a variable within the scope or within imports, diff --git a/src/parser.rs b/src/parser.rs index 0feeaf19..f3350933 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -57,7 +57,7 @@ pub struct ParseState<'e> { pub block_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] - pub external_vars: crate::FnArgsVec, + pub external_vars: Vec, /// An indicator that disables variable capturing into externals one single time /// up until the nearest consumed Identifier token. /// If set to false the next call to [`access_var`][ParseState::access_var] will not capture the variable. @@ -80,7 +80,7 @@ impl<'e> ParseState<'e> { Self { tokenizer_control, #[cfg(not(feature = "no_closure"))] - external_vars: crate::FnArgsVec::new_const(), + external_vars: Vec::new(), #[cfg(not(feature = "no_closure"))] allow_capture: true, interned_strings: StringsInterner::new(), @@ -494,19 +494,16 @@ impl Engine { #[cfg(not(feature = "no_module"))] let hash = if !namespace.is_empty() { - let index = state.find_module(namespace.root()); + let root = namespace.root(); + let index = state.find_module(root); - #[cfg(not(feature = "no_function"))] - let relax = settings.is_function_scope; - #[cfg(feature = "no_function")] - let relax = false; - - if !relax - && settings.options.contains(LangOptions::STRICT_VAR) - && index.is_none() - { - return Err(PERR::ModuleUndefined(namespace.root().to_string()) - .into_err(namespace.position())); + if settings.options.contains(LangOptions::STRICT_VAR) && index.is_none() { + if root != crate::engine::KEYWORD_GLOBAL + && !self.global_sub_modules.contains_key(root) + { + return Err(PERR::ModuleUndefined(root.to_string()) + .into_err(namespace.position())); + } } namespace.set_index(index); @@ -557,19 +554,16 @@ impl Engine { #[cfg(not(feature = "no_module"))] let hash = if !namespace.is_empty() { - let index = state.find_module(namespace.root()); + let root = namespace.root(); + let index = state.find_module(root); - #[cfg(not(feature = "no_function"))] - let relax = settings.is_function_scope; - #[cfg(feature = "no_function")] - let relax = false; - - if !relax - && settings.options.contains(LangOptions::STRICT_VAR) - && index.is_none() - { - return Err(PERR::ModuleUndefined(namespace.root().to_string()) - .into_err(namespace.position())); + if settings.options.contains(LangOptions::STRICT_VAR) && index.is_none() { + if root != crate::engine::KEYWORD_GLOBAL + && !self.global_sub_modules.contains_key(root) + { + return Err(PERR::ModuleUndefined(root.to_string()) + .into_err(namespace.position())); + } } namespace.set_index(index); @@ -1268,6 +1262,7 @@ impl Engine { Token::Pipe | Token::Or if settings.options.contains(LangOptions::ANON_FN) => { let mut new_state = ParseState::new(self, state.scope, state.tokenizer_control.clone()); + new_state.imports = state.imports.clone(); #[cfg(not(feature = "unchecked"))] { @@ -1678,19 +1673,16 @@ impl Engine { #[cfg(not(feature = "no_module"))] { - let index = state.find_module(namespace.root()); + let root = namespace.root(); + let index = state.find_module(root); - #[cfg(not(feature = "no_function"))] - let relax = settings.is_function_scope; - #[cfg(feature = "no_function")] - let relax = false; - - if !relax - && settings.options.contains(LangOptions::STRICT_VAR) - && index.is_none() - { - return Err(PERR::ModuleUndefined(namespace.root().to_string()) - .into_err(namespace.position())); + if settings.options.contains(LangOptions::STRICT_VAR) && index.is_none() { + if root != crate::engine::KEYWORD_GLOBAL + && !self.global_sub_modules.contains_key(root) + { + return Err(PERR::ModuleUndefined(root.to_string()) + .into_err(namespace.position())); + } } namespace.set_index(index); @@ -3080,6 +3072,7 @@ impl Engine { (Token::Fn, pos) => { let mut new_state = ParseState::new(self, state.scope, state.tokenizer_control.clone()); + new_state.imports = state.imports.clone(); #[cfg(not(feature = "unchecked"))] { diff --git a/src/types/dynamic.rs b/src/types/dynamic.rs index 0b5e891c..8c71de89 100644 --- a/src/types/dynamic.rs +++ b/src/types/dynamic.rs @@ -52,7 +52,7 @@ pub trait Variant: Any + private::Sealed { #[must_use] fn as_any_mut(&mut self) -> &mut dyn Any; - /// Convert this [`Variant`] trait object to [`Box`]. + /// Convert this [`Variant`] trait object to [`Box`][Any]. #[must_use] fn as_boxed_any(self: Box) -> Box; @@ -79,7 +79,7 @@ pub trait Variant: Any + Send + Sync + private::Sealed { #[must_use] fn as_any_mut(&mut self) -> &mut dyn Any; - /// Convert this [`Variant`] trait object to [`Box`]. + /// Convert this [`Variant`] trait object to [`Box`][Any]. #[must_use] fn as_boxed_any(self: Box) -> Box; diff --git a/tests/options.rs b/tests/options.rs index 5725a76f..cbe0f70a 100644 --- a/tests/options.rs +++ b/tests/options.rs @@ -91,18 +91,17 @@ fn test_options_strict_var() -> Result<(), Box> { #[cfg(not(feature = "no_function"))] assert!(engine.compile("fn foo(x) { x + y }").is_err()); + #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_module"))] { assert!(engine.compile("print(h::y::z);").is_err()); - engine.compile(r#"import "hello" as h; print(h::y::z);"#)?; + assert!(engine.compile("fn foo() { h::y::z }").is_err()); + assert!(engine.compile("fn foo() { h::y::foo() }").is_err()); + engine.compile(r#"import "hello" as h; fn foo() { h::a::b::c } print(h::y::z);"#)?; assert!(engine.compile("let x = h::y::foo();").is_err()); - engine.compile(r#"import "hello" as h; let x = h::y::foo();"#)?; + engine.compile(r#"import "hello" as h; fn foo() { h::a::b::c() } let x = h::y::foo();"#)?; } - #[cfg(not(feature = "no_function"))] - #[cfg(not(feature = "no_module"))] - engine.compile("fn foo() { h::y::foo() }")?; - #[cfg(not(feature = "no_function"))] { assert!(engine.compile("let f = |y| x * y;").is_err()); From a31a4e48875ae3773c2dd291df071b2f6af9966f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 9 Jun 2022 18:06:00 +0800 Subject: [PATCH 10/14] Fix builds. --- src/ast/ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 108b204e..f7e17414 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -811,7 +811,7 @@ impl AST { #[cfg(not(feature = "no_module"))] #[inline(always)] pub(crate) fn walk(&self, on_node: &mut impl FnMut(&[ASTNode]) -> bool) -> bool { - self.walk_raw(on_node) + self._walk(on_node) } /// _(internals)_ Recursively walk the [`AST`], including function bodies (if any). /// Return `false` from the callback to terminate the walk. From 09e19790fe664c1cb1f9ff8abec54bcb20f0225b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 9 Jun 2022 18:22:53 +0800 Subject: [PATCH 11/14] Fix builds. --- src/api/eval.rs | 4 ++-- src/parser.rs | 38 +++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/api/eval.rs b/src/api/eval.rs index aec12d99..a90d88bc 100644 --- a/src/api/eval.rs +++ b/src/api/eval.rs @@ -6,9 +6,9 @@ use crate::types::dynamic::Variant; use crate::{ Dynamic, Engine, OptimizationLevel, Position, RhaiResult, RhaiResultOf, Scope, AST, ERR, }; +use std::any::type_name; #[cfg(feature = "no_std")] use std::prelude::v1::*; -use std::{any::type_name, mem}; impl Engine { /// Evaluate a string. @@ -222,7 +222,7 @@ impl Engine { global.source = ast.source_raw().clone(); #[cfg(not(feature = "no_module"))] - let orig_embedded_module_resolver = mem::replace( + let orig_embedded_module_resolver = std::mem::replace( &mut global.embedded_module_resolver, ast.resolver().cloned(), ); diff --git a/src/parser.rs b/src/parser.rs index f3350933..0dfdcf4b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -497,10 +497,14 @@ impl Engine { let root = namespace.root(); let index = state.find_module(root); + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_module"))] + let is_global = root == crate::engine::KEYWORD_GLOBAL; + #[cfg(any(feature = "no_function", feature = "no_module"))] + let is_global = false; + if settings.options.contains(LangOptions::STRICT_VAR) && index.is_none() { - if root != crate::engine::KEYWORD_GLOBAL - && !self.global_sub_modules.contains_key(root) - { + if !is_global && !self.global_sub_modules.contains_key(root) { return Err(PERR::ModuleUndefined(root.to_string()) .into_err(namespace.position())); } @@ -557,10 +561,14 @@ impl Engine { let root = namespace.root(); let index = state.find_module(root); + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_module"))] + let is_global = root == crate::engine::KEYWORD_GLOBAL; + #[cfg(any(feature = "no_function", feature = "no_module"))] + let is_global = false; + if settings.options.contains(LangOptions::STRICT_VAR) && index.is_none() { - if root != crate::engine::KEYWORD_GLOBAL - && !self.global_sub_modules.contains_key(root) - { + if !is_global && !self.global_sub_modules.contains_key(root) { return Err(PERR::ModuleUndefined(root.to_string()) .into_err(namespace.position())); } @@ -1262,7 +1270,9 @@ impl Engine { Token::Pipe | Token::Or if settings.options.contains(LangOptions::ANON_FN) => { let mut new_state = ParseState::new(self, state.scope, state.tokenizer_control.clone()); - new_state.imports = state.imports.clone(); + + #[cfg(not(feature = "no_module"))] + new_state.imports.clone_from(&state.imports); #[cfg(not(feature = "unchecked"))] { @@ -1676,10 +1686,14 @@ impl Engine { let root = namespace.root(); let index = state.find_module(root); + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_module"))] + let is_global = root == crate::engine::KEYWORD_GLOBAL; + #[cfg(any(feature = "no_function", feature = "no_module"))] + let is_global = false; + if settings.options.contains(LangOptions::STRICT_VAR) && index.is_none() { - if root != crate::engine::KEYWORD_GLOBAL - && !self.global_sub_modules.contains_key(root) - { + if !is_global && !self.global_sub_modules.contains_key(root) { return Err(PERR::ModuleUndefined(root.to_string()) .into_err(namespace.position())); } @@ -3072,7 +3086,9 @@ impl Engine { (Token::Fn, pos) => { let mut new_state = ParseState::new(self, state.scope, state.tokenizer_control.clone()); - new_state.imports = state.imports.clone(); + + #[cfg(not(feature = "no_module"))] + new_state.imports.clone_from(&state.imports); #[cfg(not(feature = "unchecked"))] { From 206318e14cf9cec1e4a9d00a6618a35b0e6f464e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 10 Jun 2022 08:47:22 +0800 Subject: [PATCH 12/14] Add new reserved symbols. --- CHANGELOG.md | 5 +++++ src/tokenizer.rs | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 381ca6ac..0f91b71a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ Bug fixes * _Strict Variables Mode_ now checks for module namespaces within functions as well. * Module defined via `Engine::register_static_module` are now checked in _Strict Variables Mode_. +Reserved Symbols +---------------- + +* `?`, `?.` and `!.` are now reserved symbols. + Deprecated API's ---------------- diff --git a/src/tokenizer.rs b/src/tokenizer.rs index f75269f6..42e8a3b9 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -1992,6 +1992,7 @@ fn get_next_token_inner( return Some((Token::NotEqualsTo, start_pos)); } + ('!', '.') => return Some((Token::Reserved("!.".into()), start_pos)), ('!', ..) => return Some((Token::Bang, start_pos)), ('|', '|') => { @@ -2032,6 +2033,9 @@ fn get_next_token_inner( ('$', ..) => return Some((Token::Reserved("$".into()), start_pos)), + ('?', '.') => return Some((Token::Reserved("?.".into()), start_pos)), + ('?', ..) => return Some((Token::Reserved("?".into()), start_pos)), + (ch, ..) if ch.is_whitespace() => (), (ch, ..) => { From 0f1e51b1c93c68c5322ac3ff79d3f39026162666 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 10 Jun 2022 10:26:06 +0800 Subject: [PATCH 13/14] Support Elvis operator. --- CHANGELOG.md | 5 ++++ src/ast/expr.rs | 45 +++++++++++++++++++------------ src/eval/chaining.rs | 5 ++++ src/optimizer.rs | 10 +++++-- src/parser.rs | 63 +++++++++++++++++++++----------------------- src/tokenizer.rs | 24 +++++++++++------ tests/get_set.rs | 12 +++++++++ 7 files changed, 104 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f91b71a..4a47c966 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ Deprecated API's * `FnPtr::num_curried` is deprecated in favor of `FnPtr::curry().len()`. +New features +------------ + +* The _Elvis operator_ (`?.`) is now supported for property access and method calls. + Enhancements ------------ diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 75d70dbc..e7fdd3fd 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -400,18 +400,18 @@ pub enum Expr { Stmt(Box), /// func `(` expr `,` ... `)` FnCall(Box, Position), - /// lhs `.` rhs + /// lhs `.` rhs | lhs `?.` rhs /// /// ### Flags /// - /// No flags are defined at this time. Use [`NONE`][ASTFlags::NONE]. + /// [`NEGATED`][ASTFlags::NEGATED] = `?.` (`.` if unset) + /// [`BREAK`][ASTFlags::BREAK] = terminate the chain (recurse into the chain if unset) Dot(Box, ASTFlags, Position), /// lhs `[` rhs `]` /// /// ### Flags /// - /// [`NONE`][ASTFlags::NONE] = recurse into the indexing chain - /// [`BREAK`][ASTFlags::BREAK] = terminate the indexing chain + /// [`BREAK`][ASTFlags::BREAK] = terminate the chain (recurse into the chain if unset) Index(Box, ASTFlags, Position), /// lhs `&&` rhs And(Box, Position), @@ -484,26 +484,37 @@ impl fmt::Debug for Expr { f.debug_list().entries(x.iter()).finish() } Self::FnCall(x, ..) => fmt::Debug::fmt(x, f), - Self::Index(x, term, pos) => { + Self::Index(x, options, pos) => { if !pos.is_none() { display_pos = format!(" @ {:?}", pos); } - f.debug_struct("Index") - .field("lhs", &x.lhs) - .field("rhs", &x.rhs) - .field("terminate", term) - .finish() + let mut f = f.debug_struct("Index"); + + f.field("lhs", &x.lhs).field("rhs", &x.rhs); + if !options.is_empty() { + f.field("options", options); + } + f.finish() } - Self::Dot(x, _, pos) | Self::And(x, pos) | Self::Or(x, pos) => { + Self::Dot(x, options, pos) => { + if !pos.is_none() { + display_pos = format!(" @ {:?}", pos); + } + + let mut f = f.debug_struct("Dot"); + + f.field("lhs", &x.lhs).field("rhs", &x.rhs); + if !options.is_empty() { + f.field("options", options); + } + f.finish() + } + Self::And(x, pos) | Self::Or(x, pos) => { let op_name = match self { - Self::Dot(..) => "Dot", Self::And(..) => "And", Self::Or(..) => "Or", - expr => unreachable!( - "Self::Dot or Self::And or Self::Or expected but gets {:?}", - expr - ), + expr => unreachable!("Self::And or Self::Or expected but gets {:?}", expr), }; if !pos.is_none() { @@ -802,7 +813,7 @@ impl Expr { pub const fn is_valid_postfix(&self, token: &Token) -> bool { match token { #[cfg(not(feature = "no_object"))] - Token::Period => return true, + Token::Period | Token::Elvis => return true, #[cfg(not(feature = "no_index"))] Token::LeftBracket => return true, _ => (), diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index ba946764..af62ebe5 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -186,6 +186,11 @@ impl Engine { #[cfg(not(feature = "no_object"))] ChainType::Dotting => { + // Check for existence with the Elvis operator + if _parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() { + return Ok((Dynamic::UNIT, false)); + } + match rhs { // xxx.fn_name(arg_expr_list) Expr::MethodCall(x, pos) if !x.is_qualified() && new_val.is_none() => { diff --git a/src/optimizer.rs b/src/optimizer.rs index bf5f5e1e..ac4a811e 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -912,9 +912,15 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { _ => () } } + // ()?.rhs + #[cfg(not(feature = "no_object"))] + Expr::Dot(x, options, ..) if options.contains(ASTFlags::NEGATED) && matches!(x.lhs, Expr::Unit(..)) => { + state.set_dirty(); + *expr = mem::take(&mut x.lhs); + } // lhs.rhs #[cfg(not(feature = "no_object"))] - Expr::Dot(x,_, ..) if !_chaining => match (&mut x.lhs, &mut x.rhs) { + Expr::Dot(x, ..) if !_chaining => match (&mut x.lhs, &mut x.rhs) { // map.string (Expr::Map(m, pos), Expr::Property(p, ..)) if m.0.iter().all(|(.., x)| x.is_pure()) => { let prop = p.2.as_str(); @@ -932,7 +938,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { } // ....lhs.rhs #[cfg(not(feature = "no_object"))] - Expr::Dot(x,_, ..) => { optimize_expr(&mut x.lhs, state, false); optimize_expr(&mut x.rhs, state, _chaining); } + Expr::Dot(x,..) => { optimize_expr(&mut x.lhs, state, false); optimize_expr(&mut x.rhs, state, _chaining); } // lhs[rhs] #[cfg(not(feature = "no_index"))] diff --git a/src/parser.rs b/src/parser.rs index 0dfdcf4b..e7d63458 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1639,7 +1639,7 @@ impl Engine { } // Property access #[cfg(not(feature = "no_object"))] - (expr, Token::Period) => { + (expr, op @ Token::Period) | (expr, op @ Token::Elvis) => { // Expression after dot must start with an identifier match input.peek().expect(NEVER_ENDS) { (Token::Identifier(..), ..) => { @@ -1654,7 +1654,12 @@ impl Engine { } let rhs = self.parse_primary(input, state, lib, settings.level_up())?; - Self::make_dot_expr(state, expr, ASTFlags::NONE, rhs, tail_pos)? + let op_flags = match op { + Token::Period => ASTFlags::NONE, + Token::Elvis => ASTFlags::NEGATED, + _ => unreachable!(), + }; + Self::make_dot_expr(state, expr, rhs, ASTFlags::NONE, op_flags, tail_pos)? } // Unknown postfix operator (expr, token) => unreachable!( @@ -1959,14 +1964,22 @@ impl Engine { fn make_dot_expr( state: &mut ParseState, lhs: Expr, - parent_options: ASTFlags, rhs: Expr, + parent_options: ASTFlags, + op_flags: ASTFlags, op_pos: Position, ) -> ParseResult { match (lhs, rhs) { // lhs[idx_expr].rhs (Expr::Index(mut x, options, pos), rhs) => { - x.rhs = Self::make_dot_expr(state, x.rhs, options | parent_options, rhs, op_pos)?; + x.rhs = Self::make_dot_expr( + state, + x.rhs, + rhs, + options | parent_options, + op_flags, + op_pos, + )?; Ok(Expr::Index(x, ASTFlags::NONE, pos)) } // lhs.module::id - syntax error @@ -1977,16 +1990,12 @@ impl Engine { // lhs.id (lhs, var_expr @ Expr::Variable(..)) => { let rhs = var_expr.into_property(state); - Ok(Expr::Dot( - BinaryExpr { lhs, rhs }.into(), - ASTFlags::NONE, - op_pos, - )) + Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), op_flags, op_pos)) } // lhs.prop (lhs, prop @ Expr::Property(..)) => Ok(Expr::Dot( BinaryExpr { lhs, rhs: prop }.into(), - ASTFlags::NONE, + op_flags, op_pos, )), // lhs.nnn::func(...) - syntax error @@ -2023,17 +2032,13 @@ impl Engine { ); let rhs = Expr::MethodCall(func, func_pos); - Ok(Expr::Dot( - BinaryExpr { lhs, rhs }.into(), - ASTFlags::NONE, - op_pos, - )) + Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), op_flags, op_pos)) } // lhs.dot_lhs.dot_rhs or lhs.dot_lhs[idx_rhs] (lhs, rhs @ (Expr::Dot(..) | Expr::Index(..))) => { - let (x, term, pos, is_dot) = match rhs { - Expr::Dot(x, term, pos) => (x, term, pos, true), - Expr::Index(x, term, pos) => (x, term, pos, false), + let (x, options, pos, is_dot) = match rhs { + Expr::Dot(x, options, pos) => (x, options, pos, true), + Expr::Index(x, options, pos) => (x, options, pos, false), expr => unreachable!("Expr::Dot or Expr::Index expected but gets {:?}", expr), }; @@ -2050,22 +2055,18 @@ impl Engine { } // lhs.id.dot_rhs or lhs.id[idx_rhs] Expr::Variable(..) | Expr::Property(..) => { - let new_lhs = BinaryExpr { + let new_binary = BinaryExpr { lhs: x.lhs.into_property(state), rhs: x.rhs, } .into(); let rhs = if is_dot { - Expr::Dot(new_lhs, term, pos) + Expr::Dot(new_binary, options, pos) } else { - Expr::Index(new_lhs, term, pos) + Expr::Index(new_binary, options, pos) }; - Ok(Expr::Dot( - BinaryExpr { lhs, rhs }.into(), - ASTFlags::NONE, - op_pos, - )) + Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), op_flags, op_pos)) } // lhs.func().dot_rhs or lhs.func()[idx_rhs] Expr::FnCall(mut func, func_pos) => { @@ -2083,15 +2084,11 @@ impl Engine { .into(); let rhs = if is_dot { - Expr::Dot(new_lhs, term, pos) + Expr::Dot(new_lhs, options, pos) } else { - Expr::Index(new_lhs, term, pos) + Expr::Index(new_lhs, options, pos) }; - Ok(Expr::Dot( - BinaryExpr { lhs, rhs }.into(), - ASTFlags::NONE, - op_pos, - )) + Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), op_flags, op_pos)) } expr => unreachable!("invalid dot expression: {:?}", expr), } diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 42e8a3b9..99aa7319 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -420,6 +420,8 @@ pub enum Token { Comma, /// `.` Period, + /// `?.` + Elvis, /// `..` ExclusiveRange, /// `..=` @@ -576,6 +578,7 @@ impl Token { Underscore => "_", Comma => ",", Period => ".", + Elvis => "?.", ExclusiveRange => "..", InclusiveRange => "..=", MapStart => "#{", @@ -771,6 +774,7 @@ impl Token { "_" => Underscore, "," => Comma, "." => Period, + "?." => Elvis, ".." => ExclusiveRange, "..=" => InclusiveRange, "#{" => MapStart, @@ -877,11 +881,12 @@ impl Token { use Token::*; match self { - LexError(..) | + LexError(..) | SemiColon | // ; - is unary Colon | // #{ foo: - is unary Comma | // ( ... , -expr ) - is unary //Period | + //Elvis | ExclusiveRange | // .. - is unary InclusiveRange | // ..= - is unary LeftBrace | // { -expr } - is unary @@ -987,12 +992,12 @@ impl Token { match self { LeftBrace | RightBrace | LeftParen | RightParen | LeftBracket | RightBracket | Plus | UnaryPlus | Minus | UnaryMinus | Multiply | Divide | Modulo | PowerOf | LeftShift - | RightShift | SemiColon | Colon | DoubleColon | Comma | Period | ExclusiveRange - | InclusiveRange | MapStart | Equals | LessThan | GreaterThan | LessThanEqualsTo - | GreaterThanEqualsTo | EqualsTo | NotEqualsTo | Bang | Pipe | Or | XOr | Ampersand - | And | PlusAssign | MinusAssign | MultiplyAssign | DivideAssign | LeftShiftAssign - | RightShiftAssign | AndAssign | OrAssign | XOrAssign | ModuloAssign - | PowerOfAssign => true, + | RightShift | SemiColon | Colon | DoubleColon | Comma | Period | Elvis + | ExclusiveRange | InclusiveRange | MapStart | Equals | LessThan | GreaterThan + | LessThanEqualsTo | GreaterThanEqualsTo | EqualsTo | NotEqualsTo | Bang | Pipe + | Or | XOr | Ampersand | And | PlusAssign | MinusAssign | MultiplyAssign + | DivideAssign | LeftShiftAssign | RightShiftAssign | AndAssign | OrAssign + | XOrAssign | ModuloAssign | PowerOfAssign => true, _ => false, } @@ -2033,7 +2038,10 @@ fn get_next_token_inner( ('$', ..) => return Some((Token::Reserved("$".into()), start_pos)), - ('?', '.') => return Some((Token::Reserved("?.".into()), start_pos)), + ('?', '.') => { + eat_next(stream, pos); + return Some((Token::Elvis, start_pos)); + } ('?', ..) => return Some((Token::Reserved("?".into()), start_pos)), (ch, ..) if ch.is_whitespace() => (), diff --git a/tests/get_set.rs b/tests/get_set.rs index c7efc3e1..2879d56b 100644 --- a/tests/get_set.rs +++ b/tests/get_set.rs @@ -390,3 +390,15 @@ fn test_get_set_indexer() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_get_set_elvis() -> Result<(), Box> { + let engine = Engine::new(); + + assert_eq!(engine.eval::<()>("let x = (); x?.foo.bar.baz")?, ()); + assert_eq!(engine.eval::<()>("let x = (); x?.foo(1,2,3)")?, ()); + assert_eq!(engine.eval::<()>("let x = #{a:()}; x.a?.foo.bar.baz")?, ()); + assert_eq!(engine.eval::("let x = 'x'; x?.type_of()")?, "char"); + + Ok(()) +} From 8999872d629e507866a81fd6be14d0868a6d0bdf Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 10 Jun 2022 11:22:33 +0800 Subject: [PATCH 14/14] Add null coalescing operator. --- CHANGELOG.md | 3 ++- src/ast/expr.rs | 31 ++++++++++++++++++++++++------- src/eval/expr.rs | 11 +++++++++++ src/optimizer.rs | 10 ++++++++++ src/parser.rs | 16 ++++++++++++++-- src/tokenizer.rs | 21 ++++++++++++++++----- tests/binary_ops.rs | 10 ++++++++++ 7 files changed, 87 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a47c966..15545a79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Bug fixes Reserved Symbols ---------------- -* `?`, `?.` and `!.` are now reserved symbols. +* `?`, `??`, `?.` and `!.` are now reserved symbols. Deprecated API's ---------------- @@ -30,6 +30,7 @@ New features ------------ * The _Elvis operator_ (`?.`) is now supported for property access and method calls. +* The _null-coalescing operator_ (`??`) is now supported to short-circuit `()` values. Enhancements ------------ diff --git a/src/ast/expr.rs b/src/ast/expr.rs index e7fdd3fd..42cc91d2 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -417,6 +417,8 @@ pub enum Expr { And(Box, Position), /// lhs `||` rhs Or(Box, Position), + /// lhs `??` rhs + Coalesce(Box, Position), /// Custom syntax Custom(Box, Position), } @@ -510,11 +512,12 @@ impl fmt::Debug for Expr { } f.finish() } - Self::And(x, pos) | Self::Or(x, pos) => { + Self::And(x, pos) | Self::Or(x, pos) | Self::Coalesce(x, pos) => { let op_name = match self { Self::And(..) => "And", Self::Or(..) => "Or", - expr => unreachable!("Self::And or Self::Or expected but gets {:?}", expr), + Self::Coalesce(..) => "Coalesce", + expr => unreachable!("`And`, `Or` or `Coalesce` expected but gets {:?}", expr), }; if !pos.is_none() { @@ -696,6 +699,7 @@ impl Expr { | Self::Variable(.., pos) | Self::And(.., pos) | Self::Or(.., pos) + | Self::Coalesce(.., pos) | Self::Index(.., pos) | Self::Dot(.., pos) | Self::Custom(.., pos) @@ -721,10 +725,15 @@ impl Expr { self.position() } } - Self::And(x, ..) | Self::Or(x, ..) | Self::Index(x, ..) | Self::Dot(x, ..) => { - x.lhs.start_position() - } + + Self::And(x, ..) + | Self::Or(x, ..) + | Self::Coalesce(x, ..) + | Self::Index(x, ..) + | Self::Dot(x, ..) => x.lhs.start_position(), + Self::FnCall(.., pos) => *pos, + _ => self.position(), } } @@ -745,6 +754,7 @@ impl Expr { | Self::Map(.., pos) | Self::And(.., pos) | Self::Or(.., pos) + | Self::Coalesce(.., pos) | Self::Dot(.., pos) | Self::Index(.., pos) | Self::Variable(.., pos) @@ -770,7 +780,9 @@ impl Expr { Self::Map(x, ..) => x.0.iter().map(|(.., v)| v).all(Self::is_pure), - Self::And(x, ..) | Self::Or(x, ..) => x.lhs.is_pure() && x.rhs.is_pure(), + Self::And(x, ..) | Self::Or(x, ..) | Self::Coalesce(x, ..) => { + x.lhs.is_pure() && x.rhs.is_pure() + } Self::Stmt(x) => x.iter().all(Stmt::is_pure), @@ -828,6 +840,7 @@ impl Expr { | Self::CharConstant(..) | Self::And(..) | Self::Or(..) + | Self::Coalesce(..) | Self::Unit(..) => false, Self::IntegerConstant(..) @@ -892,7 +905,11 @@ impl Expr { } } } - Self::Index(x, ..) | Self::Dot(x, ..) | Expr::And(x, ..) | Expr::Or(x, ..) => { + Self::Index(x, ..) + | Self::Dot(x, ..) + | Expr::And(x, ..) + | Expr::Or(x, ..) + | Expr::Coalesce(x, ..) => { if !x.lhs.walk(path, on_node) { return false; } diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 7077637c..9e9aa7be 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -464,6 +464,17 @@ impl Engine { } } + Expr::Coalesce(x, ..) => { + let lhs = self.eval_expr(scope, global, caches, lib, this_ptr, &x.lhs, level); + + match lhs { + Ok(value) if value.is::<()>() => { + self.eval_expr(scope, global, caches, lib, this_ptr, &x.rhs, level) + } + Ok(_) | Err(_) => lhs, + } + } + Expr::Custom(custom, pos) => { let expressions: StaticVec<_> = custom.inputs.iter().map(Into::into).collect(); // The first token acts as the custom syntax's key diff --git a/src/optimizer.rs b/src/optimizer.rs index ac4a811e..1313495f 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -1073,6 +1073,16 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { // lhs || rhs (lhs, rhs) => { optimize_expr(lhs, state, false); optimize_expr(rhs, state, false); } }, + // () ?? rhs -> rhs + Expr::Coalesce(x, ..) if matches!(x.lhs, Expr::Unit(..)) => { + state.set_dirty(); + *expr = mem::take(&mut x.rhs); + }, + // lhs:constant ?? rhs -> lhs + Expr::Coalesce(x, ..) if x.lhs.is_constant() => { + state.set_dirty(); + *expr = mem::take(&mut x.lhs); + }, // eval! Expr::FnCall(x, ..) if x.name == KEYWORD_EVAL => { diff --git a/src/parser.rs b/src/parser.rs index e7d63458..4a1f1c16 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1917,8 +1917,8 @@ impl Engine { } } } - // ??? && ??? = rhs, ??? || ??? = rhs - Expr::And(..) | Expr::Or(..) => Err(LexError::ImproperSymbol( + // ??? && ??? = rhs, ??? || ??? = rhs, xxx ?? xxx = rhs + Expr::And(..) | Expr::Or(..) | Expr::Coalesce(..) => Err(LexError::ImproperSymbol( "=".to_string(), "Possibly a typo of '=='?".to_string(), ) @@ -2223,6 +2223,18 @@ impl Engine { pos, ) } + Token::DoubleQuestion => { + let rhs = args.pop().unwrap(); + let current_lhs = args.pop().unwrap(); + Expr::Coalesce( + BinaryExpr { + lhs: current_lhs, + rhs, + } + .into(), + pos, + ) + } Token::In => { // Swap the arguments let current_lhs = args.remove(0); diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 99aa7319..1dd89768 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -422,6 +422,8 @@ pub enum Token { Period, /// `?.` Elvis, + /// `??` + DoubleQuestion, /// `..` ExclusiveRange, /// `..=` @@ -579,6 +581,7 @@ impl Token { Comma => ",", Period => ".", Elvis => "?.", + DoubleQuestion => "??", ExclusiveRange => "..", InclusiveRange => "..=", MapStart => "#{", @@ -775,6 +778,7 @@ impl Token { "," => Comma, "." => Period, "?." => Elvis, + "??" => DoubleQuestion, ".." => ExclusiveRange, "..=" => InclusiveRange, "#{" => MapStart, @@ -887,6 +891,7 @@ impl Token { Comma | // ( ... , -expr ) - is unary //Period | //Elvis | + //DoubleQuestion | ExclusiveRange | // .. - is unary InclusiveRange | // ..= - is unary LeftBrace | // { -expr } - is unary @@ -957,6 +962,8 @@ impl Token { LessThan | LessThanEqualsTo | GreaterThan | GreaterThanEqualsTo => 130, + DoubleQuestion => 135, + ExclusiveRange | InclusiveRange => 140, Plus | Minus => 150, @@ -993,11 +1000,11 @@ impl Token { LeftBrace | RightBrace | LeftParen | RightParen | LeftBracket | RightBracket | Plus | UnaryPlus | Minus | UnaryMinus | Multiply | Divide | Modulo | PowerOf | LeftShift | RightShift | SemiColon | Colon | DoubleColon | Comma | Period | Elvis - | ExclusiveRange | InclusiveRange | MapStart | Equals | LessThan | GreaterThan - | LessThanEqualsTo | GreaterThanEqualsTo | EqualsTo | NotEqualsTo | Bang | Pipe - | Or | XOr | Ampersand | And | PlusAssign | MinusAssign | MultiplyAssign - | DivideAssign | LeftShiftAssign | RightShiftAssign | AndAssign | OrAssign - | XOrAssign | ModuloAssign | PowerOfAssign => true, + | DoubleQuestion | ExclusiveRange | InclusiveRange | MapStart | Equals | LessThan + | GreaterThan | LessThanEqualsTo | GreaterThanEqualsTo | EqualsTo | NotEqualsTo + | Bang | Pipe | Or | XOr | Ampersand | And | PlusAssign | MinusAssign + | MultiplyAssign | DivideAssign | LeftShiftAssign | RightShiftAssign | AndAssign + | OrAssign | XOrAssign | ModuloAssign | PowerOfAssign => true, _ => false, } @@ -2042,6 +2049,10 @@ fn get_next_token_inner( eat_next(stream, pos); return Some((Token::Elvis, start_pos)); } + ('?', '?') => { + eat_next(stream, pos); + return Some((Token::DoubleQuestion, start_pos)); + } ('?', ..) => return Some((Token::Reserved("?".into()), start_pos)), (ch, ..) if ch.is_whitespace() => (), diff --git a/tests/binary_ops.rs b/tests/binary_ops.rs index ca5c79db..a470a155 100644 --- a/tests/binary_ops.rs +++ b/tests/binary_ops.rs @@ -135,3 +135,13 @@ fn test_binary_ops() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_binary_ops_null_coalesce() -> Result<(), Box> { + let engine = Engine::new(); + + assert_eq!(engine.eval::("let x = 42; x ?? 123")?, 42); + assert_eq!(engine.eval::("let x = (); x ?? 123")?, 123); + + Ok(()) +}