From 795a3afa8107abb39d17b8d19e749ca0da3d7896 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 00:07:05 +0800 Subject: [PATCH 01/19] Use reference for method call parameters, add position info. --- src/engine.rs | 83 ++++++++++++++++++++++++++------------------------ src/fn_call.rs | 62 ++++++++++++++++++++++++++----------- 2 files changed, 88 insertions(+), 57 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 30f6d1a4..ce6dcb13 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -17,7 +17,6 @@ use crate::stdlib::{ collections::{HashMap, HashSet}, fmt, format, hash::{Hash, Hasher}, - iter::empty, num::{NonZeroU64, NonZeroU8, NonZeroUsize}, ops::DerefMut, string::{String, ToString}, @@ -25,12 +24,12 @@ use crate::stdlib::{ use crate::syntax::CustomSyntax; use crate::utils::{get_hasher, StraightHasherBuilder}; use crate::{ - calc_fn_hash, Dynamic, EvalAltResult, FnPtr, ImmutableString, Module, Position, RhaiResult, - Scope, Shared, StaticVec, + Dynamic, EvalAltResult, FnPtr, ImmutableString, Module, Position, RhaiResult, Scope, Shared, + StaticVec, }; #[cfg(not(feature = "no_index"))] -use crate::Array; +use crate::{calc_fn_hash, stdlib::iter::empty, Array}; #[cfg(not(feature = "no_index"))] pub const TYPICAL_ARRAY_SIZE: usize = 8; // Small arrays are typical @@ -223,11 +222,11 @@ pub enum ChainType { #[derive(Debug, Clone, Hash)] pub enum ChainArgument { /// Dot-property access. - Property, + Property(Position), /// Arguments to a dot-function call. - FnCallArgs(StaticVec), + FnCallArgs(StaticVec<(Dynamic, Position)>), /// Index value. - IndexValue(Dynamic), + IndexValue(Dynamic, Position), } #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -241,8 +240,10 @@ impl ChainArgument { #[cfg(not(feature = "no_index"))] pub fn as_index_value(self) -> Dynamic { match self { - Self::Property | Self::FnCallArgs(_) => panic!("expecting ChainArgument::IndexValue"), - Self::IndexValue(value) => value, + Self::Property(_) | Self::FnCallArgs(_) => { + panic!("expecting ChainArgument::IndexValue") + } + Self::IndexValue(value, _) => value, } } /// Return the `StaticVec` value. @@ -252,27 +253,29 @@ impl ChainArgument { /// Panics if not `ChainArgument::FnCallArgs`. #[inline(always)] #[cfg(not(feature = "no_object"))] - pub fn as_fn_call_args(self) -> StaticVec { + pub fn as_fn_call_args(self) -> StaticVec<(Dynamic, Position)> { match self { - Self::Property | Self::IndexValue(_) => panic!("expecting ChainArgument::FnCallArgs"), + Self::Property(_) | Self::IndexValue(_, _) => { + panic!("expecting ChainArgument::FnCallArgs") + } Self::FnCallArgs(value) => value, } } } #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] -impl From> for ChainArgument { +impl From> for ChainArgument { #[inline(always)] - fn from(value: StaticVec) -> Self { + fn from(value: StaticVec<(Dynamic, Position)>) -> Self { Self::FnCallArgs(value) } } #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] -impl From for ChainArgument { +impl From<(Dynamic, Position)> for ChainArgument { #[inline(always)] - fn from(value: Dynamic) -> Self { - Self::IndexValue(value) + fn from((value, pos): (Dynamic, Position)) -> Self { + Self::IndexValue(value, pos) } } @@ -395,7 +398,11 @@ impl<'a> Target<'a> { } /// Update the value of the `Target`. #[cfg(any(not(feature = "no_object"), not(feature = "no_index")))] - pub fn set_value(&mut self, new_val: Dynamic, pos: Position) -> Result<(), Box> { + pub fn set_value( + &mut self, + new_val: Dynamic, + _pos: Position, + ) -> Result<(), Box> { match self { Self::Ref(r) => **r = new_val, #[cfg(not(feature = "no_closure"))] @@ -411,7 +418,7 @@ impl<'a> Target<'a> { Box::new(EvalAltResult::ErrorMismatchDataType( "char".to_string(), err.to_string(), - pos, + _pos, )) })?; @@ -1152,9 +1159,9 @@ impl Engine { // xxx.fn_name(arg_expr_list) Expr::FnCall(x, pos) if x.namespace.is_none() && new_val.is_none() => { let FnCallExpr { name, hash, .. } = x.as_ref(); - let args = idx_val.as_fn_call_args(); + let mut args = idx_val.as_fn_call_args(); self.make_method_call( - mods, state, lib, name, *hash, target, args, *pos, level, + mods, state, lib, name, *hash, target, &mut args, *pos, level, ) } // xxx.fn_name(...) = ??? @@ -1225,9 +1232,9 @@ impl Engine { // {xxx:map}.fn_name(arg_expr_list)[expr] | {xxx:map}.fn_name(arg_expr_list).expr Expr::FnCall(x, pos) if x.namespace.is_none() => { let FnCallExpr { name, hash, .. } = x.as_ref(); - let args = idx_val.as_fn_call_args(); + let mut args = idx_val.as_fn_call_args(); let (val, _) = self.make_method_call( - mods, state, lib, name, *hash, target, args, *pos, level, + mods, state, lib, name, *hash, target, &mut args, *pos, level, )?; val.into() } @@ -1303,9 +1310,9 @@ impl Engine { // xxx.fn_name(arg_expr_list)[expr] | xxx.fn_name(arg_expr_list).expr Expr::FnCall(f, pos) if f.namespace.is_none() => { let FnCallExpr { name, hash, .. } = f.as_ref(); - let args = idx_val.as_fn_call_args(); + let mut args = idx_val.as_fn_call_args(); let (mut val, _) = self.make_method_call( - mods, state, lib, name, *hash, target, args, *pos, level, + mods, state, lib, name, *hash, target, &mut args, *pos, level, )?; let val = &mut val; let target = &mut val.into(); @@ -1427,7 +1434,7 @@ impl Engine { .iter() .map(|arg_expr| { self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(Dynamic::flatten) + .map(|v| (v.flatten(), arg_expr.position())) }) .collect::, _>>()?; @@ -1437,8 +1444,8 @@ impl Engine { unreachable!("function call in dot chain should not be namespace-qualified") } - Expr::Property(_) if parent_chain_type == ChainType::Dot => { - idx_values.push(ChainArgument::Property) + Expr::Property(x) if parent_chain_type == ChainType::Dot => { + idx_values.push(ChainArgument::Property(x.4.pos)) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), @@ -1447,8 +1454,8 @@ impl Engine { // Evaluate in left-to-right order let lhs_val = match lhs { - Expr::Property(_) if parent_chain_type == ChainType::Dot => { - ChainArgument::Property + Expr::Property(x) if parent_chain_type == ChainType::Dot => { + ChainArgument::Property(x.4.pos) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), Expr::FnCall(x, _) @@ -1458,18 +1465,17 @@ impl Engine { .iter() .map(|arg_expr| { self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(Dynamic::flatten) + .map(|v| (v.flatten(), arg_expr.position())) }) - .collect::, _>>()? + .collect::, _>>()? .into() } Expr::FnCall(_, _) if parent_chain_type == ChainType::Dot => { unreachable!("function call in dot chain should not be namespace-qualified") } _ => self - .eval_expr(scope, mods, state, lib, this_ptr, lhs, level)? - .flatten() - .into(), + .eval_expr(scope, mods, state, lib, this_ptr, lhs, level) + .map(|v| (v.flatten(), lhs.position()).into())?, }; // Push in reverse order @@ -1486,9 +1492,8 @@ impl Engine { } _ => idx_values.push( - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? - .flatten() - .into(), + self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) + .map(|v| (v.flatten(), expr.position()).into())?, ), } @@ -1745,13 +1750,13 @@ impl Engine { Expr::FnCall(x, pos) if x.namespace.is_none() => { let FnCallExpr { name, - capture: cap_scope, + capture, hash, args, .. } = x.as_ref(); self.make_function_call( - scope, mods, state, lib, this_ptr, name, args, *hash, *pos, *cap_scope, level, + scope, mods, state, lib, this_ptr, name, args, *hash, *pos, *capture, level, ) } diff --git a/src/fn_call.rs b/src/fn_call.rs index 63fe6079..d4fe037f 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -884,7 +884,7 @@ impl Engine { fn_name: &str, mut hash: FnHash, target: &mut crate::engine::Target, - mut call_args: StaticVec, + call_args: &mut StaticVec<(Dynamic, Position)>, pos: Position, level: usize, ) -> Result<(Dynamic, bool), Box> { @@ -907,7 +907,7 @@ impl Engine { let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = curry .iter_mut() - .chain(call_args.iter_mut()) + .chain(call_args.iter_mut().map(|v| &mut v.0)) .collect::>(); let args = arg_values.as_mut(); @@ -916,9 +916,23 @@ impl Engine { mods, state, lib, fn_name, hash, args, false, false, pos, None, level, ) } - KEYWORD_FN_PTR_CALL if call_args.len() > 0 && call_args[0].is::() => { + KEYWORD_FN_PTR_CALL => { + if call_args.len() > 0 { + if !call_args[0].0.is::() { + return Err(self.make_type_mismatch_err::( + self.map_type_name(obj.type_name()), + call_args[0].1, + )); + } + } else { + return Err(self.make_type_mismatch_err::( + self.map_type_name(obj.type_name()), + pos, + )); + } + // FnPtr call on object - let fn_ptr = call_args.remove(0).cast::(); + let fn_ptr = call_args.remove(0).0.cast::(); // Redirect function name let fn_name = fn_ptr.fn_name(); let args_len = call_args.len() + fn_ptr.curry().len(); @@ -931,7 +945,7 @@ impl Engine { let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = once(obj) .chain(curry.iter_mut()) - .chain(call_args.iter_mut()) + .chain(call_args.iter_mut().map(|v| &mut v.0)) .collect::>(); let args = arg_values.as_mut(); @@ -940,19 +954,31 @@ impl Engine { mods, state, lib, fn_name, hash, args, is_ref, true, pos, None, level, ) } - KEYWORD_FN_PTR_CURRY if obj.is::() => { - // Curry call + KEYWORD_FN_PTR_CURRY => { + if !obj.is::() { + return Err(self.make_type_mismatch_err::( + self.map_type_name(obj.type_name()), + pos, + )); + } + let fn_ptr = obj.read_lock::().unwrap(); + + // Curry call Ok(( - FnPtr::new_unchecked( - fn_ptr.get_fn_name().clone(), - fn_ptr - .curry() - .iter() - .cloned() - .chain(call_args.into_iter()) - .collect(), - ) + if call_args.is_empty() { + fn_ptr.clone() + } else { + FnPtr::new_unchecked( + fn_ptr.get_fn_name().clone(), + fn_ptr + .curry() + .iter() + .cloned() + .chain(call_args.iter_mut().map(|v| mem::take(v).0)) + .collect(), + ) + } .into(), false, )) @@ -981,7 +1007,7 @@ impl Engine { .iter() .cloned() .enumerate() - .for_each(|(i, v)| call_args.insert(i, v)); + .for_each(|(i, v)| call_args.insert(i, (v, Position::NONE))); // Recalculate the hash based on the new function name and new arguments hash = FnHash::from_script_and_native( calc_fn_hash(empty(), fn_name, call_args.len()), @@ -993,7 +1019,7 @@ impl Engine { // Attached object pointer in front of the arguments let mut arg_values = once(obj) - .chain(call_args.iter_mut()) + .chain(call_args.iter_mut().map(|v| &mut v.0)) .collect::>(); let args = arg_values.as_mut(); From 7805540b7b2f71e6d18ad40d30c54601c4ad9d12 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 11:55:49 +0800 Subject: [PATCH 02/19] Improve in statement. --- src/engine.rs | 49 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index ce6dcb13..7f169f48 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -900,6 +900,7 @@ impl Engine { } /// Search for a module within an imports stack. + #[inline] pub(crate) fn search_imports( &self, mods: &Imports, @@ -1609,6 +1610,7 @@ impl Engine { } // Evaluate an 'in' expression. + #[inline(always)] fn eval_in_expr( &self, scope: &mut Scope, @@ -1623,17 +1625,23 @@ impl Engine { self.inc_operations(state, rhs.position())?; let lhs_value = self.eval_expr(scope, mods, state, lib, this_ptr, lhs, level)?; - let rhs_value = self - .eval_expr(scope, mods, state, lib, this_ptr, rhs, level)? - .flatten(); - match rhs_value { + let mut rhs_target = if rhs.get_variable_access(false).is_some() { + let (rhs_ptr, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, rhs)?; + self.inc_operations(state, pos)?; + rhs_ptr + } else { + self.eval_expr(scope, mods, state, lib, this_ptr, rhs, level)? + .into() + }; + + match rhs_target.as_mut() { #[cfg(not(feature = "no_index"))] - Dynamic(Union::Array(mut rhs_value, _)) => { + Dynamic(Union::Array(rhs_value, _)) => { // Call the `==` operator to compare each value let hash = calc_fn_hash(empty(), OP_EQUALS, 2); for value in rhs_value.iter_mut() { - let args = &mut [&mut lhs_value.clone(), value]; + let args = &mut [&mut lhs_value.clone(), &mut value.clone()]; let pos = rhs.position(); if self @@ -1649,18 +1657,26 @@ impl Engine { Ok(false.into()) } #[cfg(not(feature = "no_object"))] - Dynamic(Union::Map(rhs_value, _)) => match lhs_value { + Dynamic(Union::Map(rhs_value, _)) => { // Only allows string or char - Dynamic(Union::Str(s, _)) => Ok(rhs_value.contains_key(&s).into()), - Dynamic(Union::Char(c, _)) => Ok(rhs_value.contains_key(&c.to_string()).into()), - _ => EvalAltResult::ErrorInExpr(lhs.position()).into(), - }, - Dynamic(Union::Str(rhs_value, _)) => match lhs_value { + if let Ok(c) = lhs_value.as_char() { + Ok(rhs_value.contains_key(&c.to_string()).into()) + } else if let Some(s) = lhs_value.read_lock::() { + Ok(rhs_value.contains_key(&*s).into()) + } else { + EvalAltResult::ErrorInExpr(lhs.position()).into() + } + } + Dynamic(Union::Str(rhs_value, _)) => { // Only allows string or char - Dynamic(Union::Str(s, _)) => Ok(rhs_value.contains(s.as_str()).into()), - Dynamic(Union::Char(c, _)) => Ok(rhs_value.contains(c).into()), - _ => EvalAltResult::ErrorInExpr(lhs.position()).into(), - }, + if let Ok(c) = lhs_value.as_char() { + Ok(rhs_value.contains(c).into()) + } else if let Some(s) = lhs_value.read_lock::() { + Ok(rhs_value.contains(s.as_str()).into()) + } else { + EvalAltResult::ErrorInExpr(lhs.position()).into() + } + } _ => EvalAltResult::ErrorInExpr(rhs.position()).into(), } } @@ -2487,6 +2503,7 @@ impl Engine { /// Check a result to ensure that the data size is within allowable limit. #[cfg(not(feature = "unchecked"))] + #[inline(always)] fn check_data_size(&self, result: RhaiResult, pos: Position) -> RhaiResult { // Simply return all errors if result.is_err() { From ff7844893de3565bdd58a498d183b724a154aec9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 13:16:05 +0800 Subject: [PATCH 03/19] Add contains for array. --- src/packages/array_basic.rs | 109 ++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 17 deletions(-) diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index dc66d207..8b39e09f 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -164,7 +164,7 @@ mod array_functions { result } } - #[rhai_fn(return_raw)] + #[rhai_fn(return_raw, pure)] pub fn map( ctx: NativeCallContext, array: &mut Array, @@ -197,7 +197,7 @@ mod array_functions { Ok(ar.into()) } - #[rhai_fn(return_raw)] + #[rhai_fn(return_raw, pure)] pub fn filter( ctx: NativeCallContext, array: &mut Array, @@ -233,8 +233,70 @@ mod array_functions { Ok(ar.into()) } - #[rhai_fn(return_raw)] + #[rhai_fn(return_raw, pure)] + pub fn contains( + ctx: NativeCallContext, + array: &mut Array, + mut value: Dynamic, + ) -> Result> { + for item in array.iter() { + if ctx + .call_fn_dynamic_raw(OP_EQUALS, true, &mut [&mut value, &mut item.clone()]) + .or_else(|err| match *err { + EvalAltResult::ErrorFunctionNotFound(ref fn_sig, _) + if fn_sig.starts_with(OP_EQUALS) => + { + if item.type_id() == value.type_id() { + // No default when comparing same type + Err(err) + } else { + Ok(Dynamic::FALSE) + } + } + _ => Err(err), + })? + .as_bool() + .unwrap_or(false) + { + return Ok(Dynamic::TRUE); + } + } + + Ok(Dynamic::FALSE) + } + #[rhai_fn(return_raw, pure)] pub fn index_of( + ctx: NativeCallContext, + array: &mut Array, + mut value: Dynamic, + ) -> Result> { + for (i, item) in array.iter().enumerate() { + if ctx + .call_fn_dynamic_raw(OP_EQUALS, true, &mut [&mut value, &mut item.clone()]) + .or_else(|err| match *err { + EvalAltResult::ErrorFunctionNotFound(ref fn_sig, _) + if fn_sig.starts_with(OP_EQUALS) => + { + if item.type_id() == value.type_id() { + // No default when comparing same type + Err(err) + } else { + Ok(Dynamic::FALSE) + } + } + _ => Err(err), + })? + .as_bool() + .unwrap_or(false) + { + return Ok((i as INT).into()); + } + } + + Ok((-1 as INT).into()) + } + #[rhai_fn(name = "index_of", return_raw, pure)] + pub fn index_of_filter( ctx: NativeCallContext, array: &mut Array, filter: FnPtr, @@ -267,7 +329,7 @@ mod array_functions { Ok((-1 as INT).into()) } - #[rhai_fn(return_raw)] + #[rhai_fn(return_raw, pure)] pub fn some( ctx: NativeCallContext, array: &mut Array, @@ -301,7 +363,7 @@ mod array_functions { Ok(false.into()) } - #[rhai_fn(return_raw)] + #[rhai_fn(return_raw, pure)] pub fn all( ctx: NativeCallContext, array: &mut Array, @@ -335,7 +397,7 @@ mod array_functions { Ok(true.into()) } - #[rhai_fn(return_raw)] + #[rhai_fn(return_raw, pure)] pub fn reduce( ctx: NativeCallContext, array: &mut Array, @@ -366,7 +428,7 @@ mod array_functions { Ok(result) } - #[rhai_fn(name = "reduce", return_raw)] + #[rhai_fn(name = "reduce", return_raw, pure)] pub fn reduce_with_initial( ctx: NativeCallContext, array: &mut Array, @@ -405,7 +467,7 @@ mod array_functions { Ok(result) } - #[rhai_fn(return_raw)] + #[rhai_fn(return_raw, pure)] pub fn reduce_rev( ctx: NativeCallContext, array: &mut Array, @@ -436,7 +498,7 @@ mod array_functions { Ok(result) } - #[rhai_fn(name = "reduce_rev", return_raw)] + #[rhai_fn(name = "reduce_rev", return_raw, pure)] pub fn reduce_rev_with_initial( ctx: NativeCallContext, array: &mut Array, @@ -634,7 +696,7 @@ mod array_functions { drained } - #[rhai_fn(name = "==", return_raw)] + #[rhai_fn(name = "==", return_raw, pure)] pub fn equals( ctx: NativeCallContext, array: &mut Array, @@ -648,18 +710,31 @@ mod array_functions { } for (a1, a2) in array.iter_mut().zip(array2.iter_mut()) { - let equals = ctx + if !ctx .call_fn_dynamic_raw(OP_EQUALS, true, &mut [a1, a2]) - .map(|v| v.as_bool().unwrap_or(false))?; - - if !equals { - return Ok(false.into()); + .or_else(|err| match *err { + EvalAltResult::ErrorFunctionNotFound(ref fn_sig, _) + if fn_sig.starts_with(OP_EQUALS) => + { + if a1.type_id() == a2.type_id() { + // No default when comparing same type + Err(err) + } else { + Ok(Dynamic::FALSE) + } + } + _ => Err(err), + })? + .as_bool() + .unwrap_or(false) + { + return Ok(Dynamic::FALSE); } } - Ok(true.into()) + Ok(Dynamic::TRUE) } - #[rhai_fn(name = "!=", return_raw)] + #[rhai_fn(name = "!=", return_raw, pure)] pub fn not_equals( ctx: NativeCallContext, array: &mut Array, From 975bb3d6bf07da097e21fb1f7c20114a2f29ee7e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 13:44:54 +0800 Subject: [PATCH 04/19] Map in operator to contains function call. --- src/ast.rs | 25 ++---- src/engine.rs | 84 ++----------------- src/fn_builtin.rs | 33 ++++++++ src/fn_call.rs | 1 + src/lib.rs | 2 +- src/optimize.rs | 29 +------ src/packages/array_basic.rs | 12 +-- src/packages/map_basic.rs | 4 +- src/packages/string_more.rs | 8 -- src/parser.rs | 159 ++++-------------------------------- tests/maps.rs | 2 +- 11 files changed, 76 insertions(+), 283 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 81b77671..99dc3b95 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1300,7 +1300,7 @@ pub enum Expr { Variable(Box<(Option, Option<(u64, NamespaceRef)>, Ident)>), /// Property access - (getter, hash, setter, hash, prop) Property(Box<(ImmutableString, u64, ImmutableString, u64, Ident)>), - /// { [statement][Stmt] } + /// { [statement][Stmt] ... } Stmt(Box>, Position), /// func `(` expr `,` ... `)` FnCall(Box, Position), @@ -1308,8 +1308,6 @@ pub enum Expr { Dot(Box, Position), /// expr `[` expr `]` Index(Box, Position), - /// lhs `in` rhs - In(Box, Position), /// lhs `&&` rhs And(Box, Position), /// lhs `||` rhs @@ -1397,7 +1395,7 @@ impl Expr { Self::Variable(x) => (x.2).pos, Self::FnCall(_, pos) => *pos, - Self::And(x, _) | Self::Or(x, _) | Self::In(x, _) => x.lhs.position(), + Self::And(x, _) | Self::Or(x, _) => x.lhs.position(), Self::Unit(pos) => *pos, @@ -1424,7 +1422,7 @@ impl Expr { Self::Property(x) => (x.4).pos = new_pos, Self::Stmt(_, pos) => *pos = new_pos, Self::FnCall(_, pos) => *pos = new_pos, - Self::And(_, pos) | Self::Or(_, pos) | Self::In(_, pos) => *pos = new_pos, + Self::And(_, pos) | Self::Or(_, pos) => *pos = new_pos, Self::Unit(pos) => *pos = new_pos, Self::Dot(_, pos) | Self::Index(_, pos) => *pos = new_pos, Self::Custom(_, pos) => *pos = new_pos, @@ -1441,7 +1439,7 @@ impl Expr { Self::Map(x, _) => x.iter().map(|(_, v)| v).all(Self::is_pure), - Self::Index(x, _) | Self::And(x, _) | Self::Or(x, _) | Self::In(x, _) => { + Self::Index(x, _) | Self::And(x, _) | Self::Or(x, _) => { x.lhs.is_pure() && x.rhs.is_pure() } @@ -1480,13 +1478,6 @@ impl Expr { // An map literal is constant if all items are constant Self::Map(x, _) => x.iter().map(|(_, expr)| expr).all(Self::is_constant), - // Check in expression - Self::In(x, _) => match (&x.lhs, &x.rhs) { - (Self::StringConstant(_, _), Self::StringConstant(_, _)) - | (Self::CharConstant(_, _), Self::StringConstant(_, _)) => true, - _ => false, - }, - _ => false, } } @@ -1507,7 +1498,6 @@ impl Expr { | Self::IntegerConstant(_, _) | Self::CharConstant(_, _) | Self::FnPointer(_, _) - | Self::In(_, _) | Self::And(_, _) | Self::Or(_, _) | Self::Unit(_) => false, @@ -1553,11 +1543,7 @@ impl Expr { Self::Stmt(x, _) => x.iter().for_each(|s| s.walk(path, on_node)), Self::Array(x, _) => x.iter().for_each(|e| e.walk(path, on_node)), Self::Map(x, _) => x.iter().for_each(|(_, e)| e.walk(path, on_node)), - Self::Index(x, _) - | Self::Dot(x, _) - | Expr::In(x, _) - | Expr::And(x, _) - | Expr::Or(x, _) => { + Self::Index(x, _) | Self::Dot(x, _) | Expr::And(x, _) | Expr::Or(x, _) => { x.lhs.walk(path, on_node); x.rhs.walk(path, on_node); } @@ -1582,6 +1568,7 @@ mod tests { assert_eq!(size_of::>(), 16); assert_eq!(size_of::(), 4); assert_eq!(size_of::(), 16); + assert_eq!(size_of::(), 32); assert_eq!(size_of::>(), 16); assert_eq!(size_of::(), 32); assert_eq!(size_of::>(), 32); diff --git a/src/engine.rs b/src/engine.rs index 7f169f48..017b2e27 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -202,9 +202,15 @@ pub const FN_IDX_GET: &str = "index$get$"; pub const FN_IDX_SET: &str = "index$set$"; #[cfg(not(feature = "no_function"))] pub const FN_ANONYMOUS: &str = "anon$"; -#[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] + +/// Standard equality comparison operator. pub const OP_EQUALS: &str = "=="; +/// Standard method function for containment testing. +/// +/// The `in` operator is implemented as a call to this method. +pub const OP_CONTAINS: &str = "contains"; + /// Method of chaining. #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] @@ -1609,78 +1615,6 @@ impl Engine { } } - // Evaluate an 'in' expression. - #[inline(always)] - fn eval_in_expr( - &self, - scope: &mut Scope, - mods: &mut Imports, - state: &mut State, - lib: &[&Module], - this_ptr: &mut Option<&mut Dynamic>, - lhs: &Expr, - rhs: &Expr, - level: usize, - ) -> RhaiResult { - self.inc_operations(state, rhs.position())?; - - let lhs_value = self.eval_expr(scope, mods, state, lib, this_ptr, lhs, level)?; - - let mut rhs_target = if rhs.get_variable_access(false).is_some() { - let (rhs_ptr, pos) = self.search_namespace(scope, mods, state, lib, this_ptr, rhs)?; - self.inc_operations(state, pos)?; - rhs_ptr - } else { - self.eval_expr(scope, mods, state, lib, this_ptr, rhs, level)? - .into() - }; - - match rhs_target.as_mut() { - #[cfg(not(feature = "no_index"))] - Dynamic(Union::Array(rhs_value, _)) => { - // Call the `==` operator to compare each value - let hash = calc_fn_hash(empty(), OP_EQUALS, 2); - for value in rhs_value.iter_mut() { - let args = &mut [&mut lhs_value.clone(), &mut value.clone()]; - let pos = rhs.position(); - - if self - .call_native_fn(mods, state, lib, OP_EQUALS, hash, args, false, false, pos)? - .0 - .as_bool() - .unwrap_or(false) - { - return Ok(true.into()); - } - } - - Ok(false.into()) - } - #[cfg(not(feature = "no_object"))] - Dynamic(Union::Map(rhs_value, _)) => { - // Only allows string or char - if let Ok(c) = lhs_value.as_char() { - Ok(rhs_value.contains_key(&c.to_string()).into()) - } else if let Some(s) = lhs_value.read_lock::() { - Ok(rhs_value.contains_key(&*s).into()) - } else { - EvalAltResult::ErrorInExpr(lhs.position()).into() - } - } - Dynamic(Union::Str(rhs_value, _)) => { - // Only allows string or char - if let Ok(c) = lhs_value.as_char() { - Ok(rhs_value.contains(c).into()) - } else if let Some(s) = lhs_value.read_lock::() { - Ok(rhs_value.contains(s.as_str()).into()) - } else { - EvalAltResult::ErrorInExpr(lhs.position()).into() - } - } - _ => EvalAltResult::ErrorInExpr(rhs.position()).into(), - } - } - /// Evaluate an expression. pub(crate) fn eval_expr( &self, @@ -1792,10 +1726,6 @@ impl Engine { ) } - Expr::In(x, _) => { - self.eval_in_expr(scope, mods, state, lib, this_ptr, &x.lhs, &x.rhs, level) - } - Expr::And(x, _) => { Ok((self .eval_expr(scope, mods, state, lib, this_ptr, &x.lhs, level)? diff --git a/src/fn_builtin.rs b/src/fn_builtin.rs index 8a1d96d8..c592bb0e 100644 --- a/src/fn_builtin.rs +++ b/src/fn_builtin.rs @@ -1,5 +1,6 @@ //! Built-in implementations for common operators. +use crate::engine::OP_CONTAINS; use crate::fn_native::{FnCallArgs, NativeCallContext}; use crate::stdlib::{any::TypeId, format, string::ToString}; use crate::{Dynamic, ImmutableString, RhaiResult, INT}; @@ -77,6 +78,13 @@ pub fn get_builtin_binary_op_fn( Ok((x $op y).into()) }) }; + ($xx:ident . $func:ident ( $yy:ty )) => { + return Some(|_, args| { + let x = &*args[0].read_lock::<$xx>().unwrap(); + let y = &*args[1].read_lock::<$yy>().unwrap(); + Ok(x.$func(y).into()) + }) + }; ($func:ident ( $op:tt )) => { return Some(|_, args| { let (x, y) = $func(args); @@ -259,6 +267,24 @@ pub fn get_builtin_binary_op_fn( ">=" => impl_op!(get_s1s2(>=)), "<" => impl_op!(get_s1s2(<)), "<=" => impl_op!(get_s1s2(<=)), + OP_CONTAINS => { + return Some(|_, args| { + let s = &*args[0].read_lock::().unwrap(); + let c = args[1].as_char().unwrap(); + Ok((s.contains(c)).into()) + }) + } + _ => return None, + } + } + + // map op string + #[cfg(not(feature = "no_object"))] + if types_pair == (TypeId::of::(), TypeId::of::()) { + use crate::Map; + + match op { + OP_CONTAINS => impl_op!(Map.contains_key(ImmutableString)), _ => return None, } } @@ -342,6 +368,13 @@ pub fn get_builtin_binary_op_fn( ">=" => impl_op!(ImmutableString >= ImmutableString), "<" => impl_op!(ImmutableString < ImmutableString), "<=" => impl_op!(ImmutableString <= ImmutableString), + OP_CONTAINS => { + return Some(|_, args| { + let s1 = &*args[0].read_lock::().unwrap(); + let s2 = &*args[1].read_lock::().unwrap(); + Ok((s1.contains(s2.as_str())).into()) + }) + } _ => return None, } } diff --git a/src/fn_call.rs b/src/fn_call.rs index d4fe037f..c3ed8d30 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -831,6 +831,7 @@ impl Engine { } /// Evaluate a text script in place - used primarily for 'eval'. + #[inline] fn eval_script_expr_in_place( &self, scope: &mut Scope, diff --git a/src/lib.rs b/src/lib.rs index aa95de62..d36ec5b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -124,7 +124,7 @@ pub type FLOAT = f32; pub use ast::{FnAccess, AST}; pub use dynamic::Dynamic; -pub use engine::{Engine, EvalContext}; +pub use engine::{Engine, EvalContext, OP_CONTAINS, OP_EQUALS}; pub use fn_native::{FnPtr, NativeCallContext}; pub use fn_register::{RegisterFn, RegisterResultFn}; pub use module::{FnNamespace, Module}; diff --git a/src/optimize.rs b/src/optimize.rs index 20456ad2..25f423ee 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -15,7 +15,6 @@ use crate::stdlib::{ vec, vec::Vec, }; -use crate::token::is_valid_identifier; use crate::utils::get_hasher; use crate::{ calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, Module, Position, Scope, @@ -598,32 +597,6 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { // #{ key:value, .. } #[cfg(not(feature = "no_object"))] Expr::Map(x, _) => x.iter_mut().for_each(|(_, expr)| optimize_expr(expr, state)), - // lhs in rhs - Expr::In(x, _) => match (&mut x.lhs, &mut x.rhs) { - // "xxx" in "xxxxx" - (Expr::StringConstant(a, pos), Expr::StringConstant(b, _)) => { - state.set_dirty(); - *expr = Expr::BoolConstant( b.contains(a.as_str()), *pos); - } - // 'x' in "xxxxx" - (Expr::CharConstant(a, pos), Expr::StringConstant(b, _)) => { - state.set_dirty(); - *expr = Expr::BoolConstant(b.contains(*a), *pos); - } - // "xxx" in #{...} - (Expr::StringConstant(a, pos), Expr::Map(b, _)) => { - state.set_dirty(); - *expr = Expr::BoolConstant(b.iter().find(|(x, _)| x.name == *a).is_some(), *pos); - } - // 'x' in #{...} - (Expr::CharConstant(a, pos), Expr::Map(b, _)) => { - state.set_dirty(); - let ch = a.to_string(); - *expr = Expr::BoolConstant(b.iter().find(|(x, _)| x.name == &ch).is_some(), *pos); - } - // lhs in rhs - (lhs, rhs) => { optimize_expr(lhs, state); optimize_expr(rhs, state); } - }, // lhs && rhs Expr::And(x, _) => match (&mut x.lhs, &mut x.rhs) { // true && rhs -> rhs @@ -684,7 +657,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { && state.optimization_level == OptimizationLevel::Simple // simple optimizations && x.args.len() == 2 // binary call && x.args.iter().all(Expr::is_constant) // all arguments are constants - && !is_valid_identifier(x.name.chars()) // cannot be scripted + //&& !is_valid_identifier(x.name.chars()) // cannot be scripted => { let mut arg_values: StaticVec<_> = x.args.iter().map(|e| e.get_constant_value().unwrap()).collect(); let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 8b39e09f..7e3012f9 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -237,11 +237,11 @@ mod array_functions { pub fn contains( ctx: NativeCallContext, array: &mut Array, - mut value: Dynamic, + value: Dynamic, ) -> Result> { - for item in array.iter() { + for item in array.iter_mut() { if ctx - .call_fn_dynamic_raw(OP_EQUALS, true, &mut [&mut value, &mut item.clone()]) + .call_fn_dynamic_raw(OP_EQUALS, true, &mut [item, &mut value.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(ref fn_sig, _) if fn_sig.starts_with(OP_EQUALS) => @@ -268,11 +268,11 @@ mod array_functions { pub fn index_of( ctx: NativeCallContext, array: &mut Array, - mut value: Dynamic, + value: Dynamic, ) -> Result> { - for (i, item) in array.iter().enumerate() { + for (i, item) in array.iter_mut().enumerate() { if ctx - .call_fn_dynamic_raw(OP_EQUALS, true, &mut [&mut value, &mut item.clone()]) + .call_fn_dynamic_raw(OP_EQUALS, true, &mut [item, &mut value.clone()]) .or_else(|err| match *err { EvalAltResult::ErrorFunctionNotFound(ref fn_sig, _) if fn_sig.starts_with(OP_EQUALS) => diff --git a/src/packages/map_basic.rs b/src/packages/map_basic.rs index e1c0558b..f54af86c 100644 --- a/src/packages/map_basic.rs +++ b/src/packages/map_basic.rs @@ -13,8 +13,8 @@ def_package!(crate:BasicMapPackage:"Basic object map utilities.", lib, { #[export_module] mod map_functions { - #[rhai_fn(pure)] - pub fn has(map: &mut Map, prop: ImmutableString) -> bool { + #[rhai_fn(name = "has", pure)] + pub fn contains(map: &mut Map, prop: ImmutableString) -> bool { map.contains_key(&prop) } #[rhai_fn(pure)] diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 783dc2c4..ce3d819e 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -74,14 +74,6 @@ mod string_functions { } } - #[rhai_fn(name = "contains")] - pub fn contains_char(string: &str, character: char) -> bool { - string.contains(character) - } - pub fn contains(string: &str, find_string: &str) -> bool { - string.contains(find_string) - } - #[rhai_fn(name = "index_of")] pub fn index_of_char_starting_from(string: &str, character: char, start: INT) -> INT { let start = if start < 0 { diff --git a/src/parser.rs b/src/parser.rs index 3361921b..3b1d05f7 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -5,7 +5,7 @@ use crate::ast::{ Stmt, }; use crate::dynamic::{AccessMode, Union}; -use crate::engine::KEYWORD_THIS; +use crate::engine::{KEYWORD_THIS, OP_CONTAINS}; use crate::module::NamespaceRef; use crate::optimize::optimize_into_ast; use crate::optimize::OptimizationLevel; @@ -480,7 +480,6 @@ fn parse_index_chain( Expr::CharConstant(_, _) | Expr::And(_, _) | Expr::Or(_, _) - | Expr::In(_, _) | Expr::BoolConstant(_, _) | Expr::Unit(_) => { return Err(PERR::MalformedIndexExpr( @@ -514,7 +513,6 @@ fn parse_index_chain( Expr::CharConstant(_, _) | Expr::And(_, _) | Expr::Or(_, _) - | Expr::In(_, _) | Expr::BoolConstant(_, _) | Expr::Unit(_) => { return Err(PERR::MalformedIndexExpr( @@ -548,8 +546,8 @@ fn parse_index_chain( ) .into_err(x.position())) } - // lhs[??? && ???], lhs[??? || ???], lhs[??? in ???] - x @ Expr::And(_, _) | x @ Expr::Or(_, _) | x @ Expr::In(_, _) => { + // lhs[??? && ???], lhs[??? || ???] + x @ Expr::And(_, _) | x @ Expr::Or(_, _) => { return Err(PERR::MalformedIndexExpr( "Array access expects integer index, not a boolean".into(), ) @@ -1602,139 +1600,6 @@ fn make_dot_expr( }) } -/// Make an 'in' expression. -fn make_in_expr(lhs: Expr, rhs: Expr, op_pos: Position) -> Result { - match (&lhs, &rhs) { - (_, x @ Expr::IntegerConstant(_, _)) - | (_, x @ Expr::And(_, _)) - | (_, x @ Expr::Or(_, _)) - | (_, x @ Expr::In(_, _)) - | (_, x @ Expr::BoolConstant(_, _)) - | (_, x @ Expr::Unit(_)) => { - return Err(PERR::MalformedInExpr( - "'in' expression expects a string, array or object map".into(), - ) - .into_err(x.position())) - } - - #[cfg(not(feature = "no_float"))] - (_, x @ Expr::FloatConstant(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression expects a string, array or object map".into(), - ) - .into_err(x.position())) - } - - // "xxx" in "xxxx", 'x' in "xxxx" - OK! - (Expr::StringConstant(_, _), Expr::StringConstant(_, _)) - | (Expr::CharConstant(_, _), Expr::StringConstant(_, _)) => (), - - // 123.456 in "xxxx" - #[cfg(not(feature = "no_float"))] - (x @ Expr::FloatConstant(_, _), Expr::StringConstant(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for a string expects a string, not a float".into(), - ) - .into_err(x.position())) - } - // 123 in "xxxx" - (x @ Expr::IntegerConstant(_, _), Expr::StringConstant(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for a string expects a string, not a number".into(), - ) - .into_err(x.position())) - } - // (??? && ???) in "xxxx", (??? || ???) in "xxxx", (??? in ???) in "xxxx", - // true in "xxxx", false in "xxxx" - (x @ Expr::And(_, _), Expr::StringConstant(_, _)) - | (x @ Expr::Or(_, _), Expr::StringConstant(_, _)) - | (x @ Expr::In(_, _), Expr::StringConstant(_, _)) - | (x @ Expr::BoolConstant(_, _), Expr::StringConstant(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for a string expects a string, not a boolean".into(), - ) - .into_err(x.position())) - } - // [???, ???, ???] in "xxxx" - (x @ Expr::Array(_, _), Expr::StringConstant(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for a string expects a string, not an array".into(), - ) - .into_err(x.position())) - } - // #{...} in "xxxx" - (x @ Expr::Map(_, _), Expr::StringConstant(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for a string expects a string, not an object map".into(), - ) - .into_err(x.position())) - } - // () in "xxxx" - (x @ Expr::Unit(_), Expr::StringConstant(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for a string expects a string, not ()".into(), - ) - .into_err(x.position())) - } - - // "xxx" in #{...}, 'x' in #{...} - OK! - (Expr::StringConstant(_, _), Expr::Map(_, _)) - | (Expr::CharConstant(_, _), Expr::Map(_, _)) => (), - - // 123.456 in #{...} - #[cfg(not(feature = "no_float"))] - (x @ Expr::FloatConstant(_, _), Expr::Map(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for an object map expects a string, not a float".into(), - ) - .into_err(x.position())) - } - // 123 in #{...} - (x @ Expr::IntegerConstant(_, _), Expr::Map(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for an object map expects a string, not a number".into(), - ) - .into_err(x.position())) - } - // (??? && ???) in #{...}, (??? || ???) in #{...}, (??? in ???) in #{...}, - // true in #{...}, false in #{...} - (x @ Expr::And(_, _), Expr::Map(_, _)) - | (x @ Expr::Or(_, _), Expr::Map(_, _)) - | (x @ Expr::In(_, _), Expr::Map(_, _)) - | (x @ Expr::BoolConstant(_, _), Expr::Map(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for an object map expects a string, not a boolean".into(), - ) - .into_err(x.position())) - } - // [???, ???, ???] in #{..} - (x @ Expr::Array(_, _), Expr::Map(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for an object map expects a string, not an array".into(), - ) - .into_err(x.position())) - } - // #{...} in #{..} - (x @ Expr::Map(_, _), Expr::Map(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for an object map expects a string, not an object map".into(), - ) - .into_err(x.position())) - } - // () in #{...} - (x @ Expr::Unit(_), Expr::Map(_, _)) => { - return Err(PERR::MalformedInExpr( - "'in' expression for an object map expects a string, not ()".into(), - ) - .into_err(x.position())) - } - - _ => (), - } - - Ok(Expr::In(Box::new(BinaryExpr { lhs, rhs }), op_pos)) -} - /// Parse a binary expression. fn parse_binary_op( input: &mut TokenStream, @@ -1880,9 +1745,21 @@ fn parse_binary_op( ) } Token::In => { - let rhs = args.pop().unwrap(); - let current_lhs = args.pop().unwrap(); - make_in_expr(current_lhs, rhs, pos)? + // Swap the arguments + let current_lhs = args.remove(0); + args.push(current_lhs); + + // Convert into a call to `contains` + let hash = calc_fn_hash(empty(), OP_CONTAINS, 2); + Expr::FnCall( + Box::new(FnCallExpr { + hash: FnHash::from_script(hash), + args, + name: OP_CONTAINS.into(), + ..op_base + }), + pos, + ) } Token::Custom(s) diff --git a/tests/maps.rs b/tests/maps.rs index 988c51a3..5892c29d 100644 --- a/tests/maps.rs +++ b/tests/maps.rs @@ -38,7 +38,7 @@ fn test_map_indexing() -> Result<(), Box> { engine.eval::<()>("let y = #{a: 1, b: 2, c: 3}; y.z")?; assert!(engine.eval::(r#"let y = #{a: 1, b: 2, c: 3}; "c" in y"#)?); - assert!(engine.eval::("let y = #{a: 1, b: 2, c: 3}; 'b' in y")?); + assert!(engine.eval::(r#"let y = #{a: 1, b: 2, c: 3}; "b" in y"#)?); assert!(!engine.eval::(r#"let y = #{a: 1, b: 2, c: 3}; "z" in y"#)?); assert_eq!( From b78c6ddf62ec62bbe4c9463e6c9f1bcac4d3cb63 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 14:00:21 +0800 Subject: [PATCH 05/19] Better debug display. --- src/ast.rs | 16 +++++++++++++++- src/module/mod.rs | 15 ++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 99dc3b95..e10ecd91 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1102,7 +1102,7 @@ pub struct OpAssignment { /// # Volatile Data Structure /// /// This type is volatile and may change. -#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Default)] +#[derive(Clone, Copy, Eq, PartialEq, Hash, Default)] pub struct FnHash { /// Pre-calculated hash for a script-defined function ([`None`] if native functions only). script: Option, @@ -1110,6 +1110,20 @@ pub struct FnHash { native: u64, } +impl fmt::Debug for FnHash { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(script) = self.script { + if script == self.native { + write!(f, "({}=={})", script, self.native) + } else { + write!(f, "({}, {})", script, self.native) + } + } else { + write!(f, "{}", self.native) + } + } +} + impl FnHash { /// Create a [`FnHash`] with only the native Rust hash. #[inline(always)] diff --git a/src/module/mod.rs b/src/module/mod.rs index c79e2396..0727c8fb 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -2022,13 +2022,18 @@ pub struct NamespaceRef { impl fmt::Debug for NamespaceRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.path, f)?; - if let Some(index) = self.index { - write!(f, " -> {}", index) - } else { - Ok(()) + write!(f, "{} -> ", index)?; } + + f.write_str( + &self + .path + .iter() + .map(|Ident { name, .. }| name.as_str()) + .collect::>() + .join("::"), + ) } } From bc2b9bfbfd70e60da8fcea1edabaf3d29374003b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 14:39:03 +0800 Subject: [PATCH 06/19] Fix builds. --- src/ast.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ast.rs b/src/ast.rs index e10ecd91..6ad9d148 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1582,7 +1582,6 @@ mod tests { assert_eq!(size_of::>(), 16); assert_eq!(size_of::(), 4); assert_eq!(size_of::(), 16); - assert_eq!(size_of::(), 32); assert_eq!(size_of::>(), 16); assert_eq!(size_of::(), 32); assert_eq!(size_of::>(), 32); From 9daa894e257044087e63a50767bb463339db5fa1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 18:11:43 +0800 Subject: [PATCH 07/19] Pack method call args more tightly. --- CHANGELOG.md | 1 + src/engine.rs | 34 +++++++++++++++++++++------------- src/fn_call.rs | 22 +++++++++++++--------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c0f4af9..c9a74be3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Breaking changes * `is_def_var` and `is_def_fn` are now reserved keywords. * `Engine::id` field is removed. * `num-traits` is now a required dependency. +* The `in` operator is now implemented on top of the `contains` function and is no longer restricted to a few specific types. Enhancements ------------ diff --git a/src/engine.rs b/src/engine.rs index 017b2e27..274710bd 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -230,7 +230,7 @@ pub enum ChainArgument { /// Dot-property access. Property(Position), /// Arguments to a dot-function call. - FnCallArgs(StaticVec<(Dynamic, Position)>), + FnCallArgs(StaticVec, StaticVec), /// Index value. IndexValue(Dynamic, Position), } @@ -246,7 +246,7 @@ impl ChainArgument { #[cfg(not(feature = "no_index"))] pub fn as_index_value(self) -> Dynamic { match self { - Self::Property(_) | Self::FnCallArgs(_) => { + Self::Property(_) | Self::FnCallArgs(_, _) => { panic!("expecting ChainArgument::IndexValue") } Self::IndexValue(value, _) => value, @@ -259,21 +259,21 @@ impl ChainArgument { /// Panics if not `ChainArgument::FnCallArgs`. #[inline(always)] #[cfg(not(feature = "no_object"))] - pub fn as_fn_call_args(self) -> StaticVec<(Dynamic, Position)> { + pub fn as_fn_call_args(self) -> (StaticVec, StaticVec) { match self { Self::Property(_) | Self::IndexValue(_, _) => { panic!("expecting ChainArgument::FnCallArgs") } - Self::FnCallArgs(value) => value, + Self::FnCallArgs(values, positions) => (values, positions), } } } #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] -impl From> for ChainArgument { +impl From<(StaticVec, StaticVec)> for ChainArgument { #[inline(always)] - fn from(value: StaticVec<(Dynamic, Position)>) -> Self { - Self::FnCallArgs(value) + fn from((values, positions): (StaticVec, StaticVec)) -> Self { + Self::FnCallArgs(values, positions) } } @@ -1436,16 +1436,19 @@ impl Engine { match expr { Expr::FnCall(x, _) if parent_chain_type == ChainType::Dot && x.namespace.is_none() => { + let mut arg_positions: StaticVec<_> = Default::default(); + let arg_values = x .args .iter() .map(|arg_expr| { + arg_positions.push(arg_expr.position()); self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(|v| (v.flatten(), arg_expr.position())) + .map(Dynamic::flatten) }) .collect::, _>>()?; - idx_values.push(arg_values.into()); + idx_values.push((arg_values, arg_positions).into()); } Expr::FnCall(_, _) if parent_chain_type == ChainType::Dot => { unreachable!("function call in dot chain should not be namespace-qualified") @@ -1468,14 +1471,19 @@ impl Engine { Expr::FnCall(x, _) if parent_chain_type == ChainType::Dot && x.namespace.is_none() => { - x.args + let mut arg_positions: StaticVec<_> = Default::default(); + + let arg_values = x + .args .iter() .map(|arg_expr| { + arg_positions.push(arg_expr.position()); self.eval_expr(scope, mods, state, lib, this_ptr, arg_expr, level) - .map(|v| (v.flatten(), arg_expr.position())) + .map(Dynamic::flatten) }) - .collect::, _>>()? - .into() + .collect::, _>>()?; + + (arg_values, arg_positions).into() } Expr::FnCall(_, _) if parent_chain_type == ChainType::Dot => { unreachable!("function call in dot chain should not be namespace-qualified") diff --git a/src/fn_call.rs b/src/fn_call.rs index c3ed8d30..f9ce5626 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -885,7 +885,7 @@ impl Engine { fn_name: &str, mut hash: FnHash, target: &mut crate::engine::Target, - call_args: &mut StaticVec<(Dynamic, Position)>, + (call_args, call_arg_positions): &mut (StaticVec, StaticVec), pos: Position, level: usize, ) -> Result<(Dynamic, bool), Box> { @@ -908,7 +908,7 @@ impl Engine { let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = curry .iter_mut() - .chain(call_args.iter_mut().map(|v| &mut v.0)) + .chain(call_args.iter_mut()) .collect::>(); let args = arg_values.as_mut(); @@ -919,10 +919,10 @@ impl Engine { } KEYWORD_FN_PTR_CALL => { if call_args.len() > 0 { - if !call_args[0].0.is::() { + if !call_args[0].is::() { return Err(self.make_type_mismatch_err::( self.map_type_name(obj.type_name()), - call_args[0].1, + call_arg_positions[0], )); } } else { @@ -933,7 +933,8 @@ impl Engine { } // FnPtr call on object - let fn_ptr = call_args.remove(0).0.cast::(); + let fn_ptr = call_args.remove(0).cast::(); + call_arg_positions.remove(0); // Redirect function name let fn_name = fn_ptr.fn_name(); let args_len = call_args.len() + fn_ptr.curry().len(); @@ -946,7 +947,7 @@ impl Engine { let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = once(obj) .chain(curry.iter_mut()) - .chain(call_args.iter_mut().map(|v| &mut v.0)) + .chain(call_args.iter_mut()) .collect::>(); let args = arg_values.as_mut(); @@ -976,7 +977,7 @@ impl Engine { .curry() .iter() .cloned() - .chain(call_args.iter_mut().map(|v| mem::take(v).0)) + .chain(call_args.iter_mut().map(|v| mem::take(v))) .collect(), ) } @@ -1008,7 +1009,10 @@ impl Engine { .iter() .cloned() .enumerate() - .for_each(|(i, v)| call_args.insert(i, (v, Position::NONE))); + .for_each(|(i, v)| { + call_args.insert(i, v); + call_arg_positions.insert(i, Position::NONE); + }); // Recalculate the hash based on the new function name and new arguments hash = FnHash::from_script_and_native( calc_fn_hash(empty(), fn_name, call_args.len()), @@ -1020,7 +1024,7 @@ impl Engine { // Attached object pointer in front of the arguments let mut arg_values = once(obj) - .chain(call_args.iter_mut().map(|v| &mut v.0)) + .chain(call_args.iter_mut()) .collect::>(); let args = arg_values.as_mut(); From 8853ebf0595ec418b2d14580efeee8007306070d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 19:13:26 +0800 Subject: [PATCH 08/19] Remove EvalAltResult::ErrorInExpr. --- CHANGELOG.md | 3 ++- src/result.rs | 8 -------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9a74be3..e0c37190 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,9 +27,10 @@ Breaking changes * `Module::update_fn_metadata` input parameter is changed. * Function keywords (e.g. `type_of`, `eval`, `Fn`) can no longer be overloaded. It is more trouble than worth. To disable these keywords, use `Engine::disable_symbol`. * `is_def_var` and `is_def_fn` are now reserved keywords. -* `Engine::id` field is removed. +* `Engine::id` field is removed because it is never used. * `num-traits` is now a required dependency. * The `in` operator is now implemented on top of the `contains` function and is no longer restricted to a few specific types. +* `EvalAltResult::ErrorInExpr` is removed because the `in` operator now calls `contains`. Enhancements ------------ diff --git a/src/result.rs b/src/result.rs index 49743d1d..0e008cad 100644 --- a/src/result.rs +++ b/src/result.rs @@ -58,8 +58,6 @@ pub enum EvalAltResult { /// Trying to index into a type that is not an array, an object map, or a string, and has no /// indexer function defined. Wrapped value is the type name. ErrorIndexingType(String, Position), - /// Invalid arguments for `in` operator. - ErrorInExpr(Position), /// The `for` statement encounters a type that is not an iterator. ErrorFor(Position), /// Data race detected when accessing a variable. Wrapped value is the variable name. @@ -122,7 +120,6 @@ impl EvalAltResult { Self::ErrorDataRace(_, _) => "Data race detected when accessing variable", Self::ErrorAssignmentToConstant(_, _) => "Cannot modify a constant", Self::ErrorMismatchOutputType(_, _, _) => "Output type is incorrect", - Self::ErrorInExpr(_) => "Malformed 'in' expression", Self::ErrorDotExpr(_, _) => "Malformed dot expression", Self::ErrorArithmetic(_, _) => "Arithmetic error", Self::ErrorTooManyOperations(_) => "Too many operations", @@ -182,7 +179,6 @@ impl fmt::Display for EvalAltResult { Self::ErrorUnboundThis(_) | Self::ErrorFor(_) - | Self::ErrorInExpr(_) | Self::ErrorDotExpr(_, _) | Self::ErrorTooManyOperations(_) | Self::ErrorTooManyModules(_) @@ -305,7 +301,6 @@ impl EvalAltResult { | Self::ErrorDataRace(_, _) | Self::ErrorAssignmentToConstant(_, _) | Self::ErrorMismatchOutputType(_, _, _) - | Self::ErrorInExpr(_) | Self::ErrorDotExpr(_, _) | Self::ErrorArithmetic(_, _) | Self::ErrorRuntime(_, _) => true, @@ -362,7 +357,6 @@ impl EvalAltResult { | Self::ErrorParsing(_, _) | Self::ErrorUnboundThis(_) | Self::ErrorFor(_) - | Self::ErrorInExpr(_) | Self::ErrorArithmetic(_, _) | Self::ErrorTooManyOperations(_) | Self::ErrorTooManyModules(_) @@ -430,7 +424,6 @@ impl EvalAltResult { | Self::ErrorDataRace(_, pos) | Self::ErrorAssignmentToConstant(_, pos) | Self::ErrorMismatchOutputType(_, _, pos) - | Self::ErrorInExpr(pos) | Self::ErrorDotExpr(_, pos) | Self::ErrorArithmetic(_, pos) | Self::ErrorTooManyOperations(pos) @@ -471,7 +464,6 @@ impl EvalAltResult { | Self::ErrorDataRace(_, pos) | Self::ErrorAssignmentToConstant(_, pos) | Self::ErrorMismatchOutputType(_, _, pos) - | Self::ErrorInExpr(pos) | Self::ErrorDotExpr(_, pos) | Self::ErrorArithmetic(_, pos) | Self::ErrorTooManyOperations(pos) From b11b8d6d397688591f8eb8cdb221751ac0706e83 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 23:30:48 +0800 Subject: [PATCH 09/19] Reduce redirections in Stmt. --- src/ast.rs | 68 ++++++++++++++---------------- src/engine.rs | 64 ++++++++++++++--------------- src/fn_call.rs | 9 ++-- src/optimize.rs | 100 ++++++++++++++------------------------------- src/parser.rs | 59 +++++++++++++++----------- tests/optimizer.rs | 3 +- 6 files changed, 132 insertions(+), 171 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 6ad9d148..a8ed9a4e 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -757,6 +757,8 @@ pub struct Ident { pub name: ImmutableString, /// Declaration position. pub pos: Position, + /// Is this identifier public? + pub public: bool, } impl fmt::Debug for Ident { @@ -815,7 +817,7 @@ pub enum Stmt { /// No-op. Noop(Position), /// `if` expr `{` stmt `}` `else` `{` stmt `}` - If(Expr, Box<(Stmt, Option)>, Position), + If(Expr, Box<(Stmt, Stmt)>, Position), /// `switch` expr `{` literal or _ `=>` stmt `,` ... `}` Switch( Expr, @@ -826,15 +828,15 @@ pub enum Stmt { Position, ), /// `while` expr `{` stmt `}` - While(Option, Box, Position), + While(Expr, Box, Position), /// `do` `{` stmt `}` `while`|`until` expr Do(Box, Expr, bool, Position), /// `for` id `in` expr `{` stmt `}` - For(Expr, Box<(String, Stmt)>, Position), + For(Expr, String, Box, Position), /// \[`export`\] `let` id `=` expr - Let(Box, Option, bool, Position), + Let(Expr, Ident, Position), /// \[`export`\] `const` id `=` expr - Const(Box, Option, bool, Position), + Const(Expr, Ident, Position), /// expr op`=` expr Assignment(Box<(Expr, Expr, Option)>, Position), /// `{` stmt`;` ... `}` @@ -848,10 +850,10 @@ pub enum Stmt { /// `break` Break(Position), /// `return`/`throw` - Return((ReturnType, Position), Option, Position), + Return(ReturnType, Option, Position), /// `import` expr `as` var #[cfg(not(feature = "no_module"))] - Import(Expr, Option>, Position), + Import(Expr, Option, Position), /// `export` var `as` var `,` ... #[cfg(not(feature = "no_module"))] Export(Vec<(Ident, Option)>, Position), @@ -888,10 +890,10 @@ impl Stmt { | Self::Switch(_, _, pos) | Self::While(_, _, pos) | Self::Do(_, _, _, pos) - | Self::For(_, _, pos) - | Self::Return((_, pos), _, _) - | Self::Let(_, _, _, pos) - | Self::Const(_, _, _, pos) + | Self::For(_, _, _, pos) + | Self::Return(_, _, pos) + | Self::Let(_, _, pos) + | Self::Const(_, _, pos) | Self::TryCatch(_, pos, _) => *pos, Self::Expr(x) => x.position(), @@ -917,10 +919,10 @@ impl Stmt { | Self::Switch(_, _, pos) | Self::While(_, _, pos) | Self::Do(_, _, _, pos) - | Self::For(_, _, pos) - | Self::Return((_, pos), _, _) - | Self::Let(_, _, _, pos) - | Self::Const(_, _, _, pos) + | Self::For(_, _, _, pos) + | Self::Return(_, _, pos) + | Self::Let(_, _, pos) + | Self::Const(_, _, pos) | Self::TryCatch(_, pos, _) => *pos = new_pos, Self::Expr(x) => { @@ -944,15 +946,15 @@ impl Stmt { Self::If(_, _, _) | Self::Switch(_, _, _) | Self::While(_, _, _) - | Self::For(_, _, _) + | Self::For(_, _, _, _) | Self::Block(_, _) | Self::TryCatch(_, _, _) => true, // A No-op requires a semicolon in order to know it is an empty statement! Self::Noop(_) => false, - Self::Let(_, _, _, _) - | Self::Const(_, _, _, _) + Self::Let(_, _, _) + | Self::Const(_, _, _) | Self::Assignment(_, _) | Self::Expr(_) | Self::Do(_, _, _, _) @@ -974,22 +976,17 @@ impl Stmt { match self { Self::Noop(_) => true, Self::Expr(expr) => expr.is_pure(), - Self::If(condition, x, _) => { - condition.is_pure() - && x.0.is_pure() - && x.1.as_ref().map(Stmt::is_pure).unwrap_or(true) - } + Self::If(condition, x, _) => condition.is_pure() && x.0.is_pure() && x.1.is_pure(), Self::Switch(expr, x, _) => { expr.is_pure() && x.0.values().all(Stmt::is_pure) && x.1.as_ref().map(Stmt::is_pure).unwrap_or(true) } - Self::While(Some(condition), block, _) | Self::Do(block, condition, _, _) => { + Self::While(condition, block, _) | Self::Do(block, condition, _, _) => { condition.is_pure() && block.is_pure() } - Self::While(None, block, _) => block.is_pure(), - Self::For(iterable, x, _) => iterable.is_pure() && x.1.is_pure(), - Self::Let(_, _, _, _) | Self::Const(_, _, _, _) | Self::Assignment(_, _) => false, + Self::For(iterable, _, block, _) => iterable.is_pure() && block.is_pure(), + Self::Let(_, _, _) | Self::Const(_, _, _) | Self::Assignment(_, _) => false, Self::Block(block, _) => block.iter().all(|stmt| stmt.is_pure()), Self::Continue(_) | Self::Break(_) | Self::Return(_, _, _) => false, Self::TryCatch(x, _, _) => x.0.is_pure() && x.2.is_pure(), @@ -1010,13 +1007,11 @@ impl Stmt { on_node(path); match self { - Self::Let(_, Some(e), _, _) | Self::Const(_, Some(e), _, _) => e.walk(path, on_node), + Self::Let(e, _, _) | Self::Const(e, _, _) => e.walk(path, on_node), Self::If(e, x, _) => { e.walk(path, on_node); x.0.walk(path, on_node); - if let Some(ref s) = x.1 { - s.walk(path, on_node); - } + x.1.walk(path, on_node); } Self::Switch(e, x, _) => { e.walk(path, on_node); @@ -1025,14 +1020,13 @@ impl Stmt { s.walk(path, on_node); } } - Self::While(Some(e), s, _) | Self::Do(s, e, _, _) => { + Self::While(e, s, _) | Self::Do(s, e, _, _) => { e.walk(path, on_node); s.walk(path, on_node); } - Self::While(None, s, _) => s.walk(path, on_node), - Self::For(e, x, _) => { + Self::For(e, _, s, _) => { e.walk(path, on_node); - x.1.walk(path, on_node); + s.walk(path, on_node); } Self::Assignment(x, _) => { x.0.walk(path, on_node); @@ -1583,8 +1577,8 @@ mod tests { assert_eq!(size_of::(), 4); assert_eq!(size_of::(), 16); assert_eq!(size_of::>(), 16); - assert_eq!(size_of::(), 32); - assert_eq!(size_of::>(), 32); + assert_eq!(size_of::(), 40); + assert_eq!(size_of::>(), 40); assert_eq!(size_of::(), 32); assert_eq!(size_of::(), 48); assert_eq!(size_of::(), 56); diff --git a/src/engine.rs b/src/engine.rs index 274710bd..62cf0ac7 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -946,7 +946,7 @@ impl Engine { match expr { Expr::Variable(v) => match v.as_ref() { // Qualified variable - (_, Some((hash_var, modules)), Ident { name, pos }) => { + (_, Some((hash_var, modules)), Ident { name, pos, .. }) => { let module = self.search_imports(mods, state, modules).ok_or_else(|| { EvalAltResult::ErrorModuleNotFound( modules[0].name.to_string(), @@ -985,7 +985,7 @@ impl Engine { this_ptr: &'s mut Option<&mut Dynamic>, expr: &Expr, ) -> Result<(Target<'s>, Position), Box> { - let (index, _, Ident { name, pos }) = match expr { + let (index, _, Ident { name, pos, .. }) = match expr { Expr::Variable(v) => v.as_ref(), _ => unreachable!("Expr::Variable expected, but gets {:?}", expr), }; @@ -1181,7 +1181,7 @@ impl Engine { } // {xxx:map}.id op= ??? Expr::Property(x) if target_val.is::() && new_val.is_some() => { - let Ident { name, pos } = &x.4; + let Ident { name, pos, .. } = &x.4; let index = name.clone().into(); let val = self.get_indexed_mut( mods, state, lib, target_val, index, *pos, true, is_ref, false, level, @@ -1194,7 +1194,7 @@ impl Engine { } // {xxx:map}.id Expr::Property(x) if target_val.is::() => { - let Ident { name, pos } = &x.4; + let Ident { name, pos, .. } = &x.4; let index = name.clone().into(); let val = self.get_indexed_mut( mods, state, lib, target_val, index, *pos, false, is_ref, false, level, @@ -1229,7 +1229,7 @@ impl Engine { Expr::Index(x, x_pos) | Expr::Dot(x, x_pos) if target_val.is::() => { let mut val = match &x.lhs { Expr::Property(p) => { - let Ident { name, pos } = &p.4; + let Ident { name, pos, .. } = &p.4; let index = name.clone().into(); self.get_indexed_mut( mods, state, lib, target_val, index, *pos, false, is_ref, true, @@ -1378,6 +1378,7 @@ impl Engine { let Ident { name: var_name, pos: var_pos, + .. } = &x.2; self.inc_operations(state, *var_pos)?; @@ -2037,8 +2038,8 @@ impl Engine { .and_then(|guard_val| { if guard_val { self.eval_stmt(scope, mods, state, lib, this_ptr, if_block, level) - } else if let Some(stmt) = else_block { - self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level) + } else if !else_block.is_noop() { + self.eval_stmt(scope, mods, state, lib, this_ptr, else_block, level) } else { Ok(Dynamic::UNIT) } @@ -2076,7 +2077,7 @@ impl Engine { // While loop Stmt::While(expr, body, _) => loop { - let condition = if let Some(expr) = expr { + let condition = if !expr.is_unit() { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .as_bool() .map_err(|err| self.make_type_mismatch_err::(err, expr.position()))? @@ -2125,8 +2126,8 @@ impl Engine { }, // For loop - Stmt::For(expr, x, _) => { - let (name, stmt) = x.as_ref(); + Stmt::For(expr, name, x, _) => { + let stmt = x.as_ref(); let iter_obj = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .flatten(); @@ -2287,7 +2288,7 @@ impl Engine { } // Return value - Stmt::Return((ReturnType::Return, pos), Some(expr), _) => { + Stmt::Return(ReturnType::Return, Some(expr), pos) => { let value = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .flatten(); @@ -2295,12 +2296,12 @@ impl Engine { } // Empty return - Stmt::Return((ReturnType::Return, pos), None, _) => { + Stmt::Return(ReturnType::Return, None, pos) => { EvalAltResult::Return(Default::default(), *pos).into() } // Throw value - Stmt::Return((ReturnType::Exception, pos), Some(expr), _) => { + Stmt::Return(ReturnType::Exception, Some(expr), pos) => { let value = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .flatten(); @@ -2308,38 +2309,34 @@ impl Engine { } // Empty throw - Stmt::Return((ReturnType::Exception, pos), None, _) => { + Stmt::Return(ReturnType::Exception, None, pos) => { EvalAltResult::ErrorRuntime(Dynamic::UNIT, *pos).into() } // Let/const statement - Stmt::Let(var_def, expr, export, _) | Stmt::Const(var_def, expr, export, _) => { + Stmt::Let(expr, Ident { name, public, .. }, _) + | Stmt::Const(expr, Ident { name, public, .. }, _) => { let entry_type = match stmt { - Stmt::Let(_, _, _, _) => AccessMode::ReadWrite, - Stmt::Const(_, _, _, _) => AccessMode::ReadOnly, + Stmt::Let(_, _, _) => AccessMode::ReadWrite, + Stmt::Const(_, _, _) => AccessMode::ReadOnly, _ => unreachable!("should be Stmt::Let or Stmt::Const, but gets {:?}", stmt), }; - let value = if let Some(expr) = expr { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? - .flatten() - } else { - Dynamic::UNIT - }; + let value = self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? + .flatten(); + let (var_name, _alias): (Cow<'_, str>, _) = if state.is_global() { ( - var_def.name.to_string().into(), - if *export { - Some(var_def.name.clone()) - } else { - None - }, + name.to_string().into(), + if *public { Some(name.clone()) } else { None }, ) - } else if *export { + } else if *public { unreachable!("exported variable not on global level"); } else { - (unsafe_cast_var_name_to_lifetime(&var_def.name).into(), None) + (unsafe_cast_var_name_to_lifetime(name).into(), None) }; + scope.push_dynamic_value(var_name, entry_type, value); #[cfg(not(feature = "no_module"))] @@ -2400,14 +2397,13 @@ impl Engine { // Export statement #[cfg(not(feature = "no_module"))] Stmt::Export(list, _) => { - for (Ident { name, pos: id_pos }, rename) in list.iter() { + for (Ident { name, pos, .. }, rename) in list.iter() { // Mark scope variables as public if let Some(index) = scope.get_index(name).map(|(i, _)| i) { let alias = rename.as_ref().map(|x| &x.name).unwrap_or_else(|| name); scope.add_entry_alias(index, alias.clone()); } else { - return EvalAltResult::ErrorVariableNotFound(name.to_string(), *id_pos) - .into(); + return EvalAltResult::ErrorVariableNotFound(name.to_string(), *pos).into(); } } Ok(Dynamic::UNIT) diff --git a/src/fn_call.rs b/src/fn_call.rs index f9ce5626..8a959115 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -903,7 +903,7 @@ impl Engine { let fn_name = fn_ptr.fn_name(); let args_len = call_args.len() + fn_ptr.curry().len(); // Recalculate hashes - let hash = FnHash::from_script(calc_fn_hash(empty(), fn_name, args_len)); + let new_hash = FnHash::from_script(calc_fn_hash(empty(), fn_name, args_len)); // Arguments are passed as-is, adding the curried arguments let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = curry @@ -914,7 +914,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - mods, state, lib, fn_name, hash, args, false, false, pos, None, level, + mods, state, lib, fn_name, new_hash, args, false, false, pos, None, level, ) } KEYWORD_FN_PTR_CALL => { @@ -939,7 +939,7 @@ impl Engine { let fn_name = fn_ptr.fn_name(); let args_len = call_args.len() + fn_ptr.curry().len(); // Recalculate hash - let hash = FnHash::from_script_and_native( + let new_hash = FnHash::from_script_and_native( calc_fn_hash(empty(), fn_name, args_len), calc_fn_hash(empty(), fn_name, args_len + 1), ); @@ -953,7 +953,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - mods, state, lib, fn_name, hash, args, is_ref, true, pos, None, level, + mods, state, lib, fn_name, new_hash, args, is_ref, true, pos, None, level, ) } KEYWORD_FN_PTR_CURRY => { @@ -1096,7 +1096,6 @@ impl Engine { FnHash::from_native(calc_fn_hash(empty(), name, args_len)) }; } - // Handle Fn() KEYWORD_FN_PTR if args_expr.len() == 1 => { // Fn - only in function call style diff --git a/src/optimize.rs b/src/optimize.rs index 25f423ee..571ebd01 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -1,6 +1,6 @@ //! Module implementing the [`AST`] optimizer. -use crate::ast::{Expr, Stmt}; +use crate::ast::{Expr, Ident, Stmt}; use crate::dynamic::AccessMode; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::fn_builtin::get_builtin_binary_op_fn; @@ -181,27 +181,17 @@ fn optimize_stmt_block( statements.iter_mut().for_each(|stmt| { match stmt { // Add constant literals into the state - Stmt::Const(var_def, Some(value_expr), _, _) => { + Stmt::Const(value_expr, Ident { name, .. }, _) => { optimize_expr(value_expr, state); if value_expr.is_constant() { - state.push_var(&var_def.name, AccessMode::ReadOnly, value_expr.clone()); + state.push_var(name, AccessMode::ReadOnly, value_expr.clone()); } } - Stmt::Const(var_def, None, _, _) => { - state.push_var(&var_def.name, AccessMode::ReadOnly, Expr::Unit(var_def.pos)); - } // Add variables into the state - Stmt::Let(var_def, expr, _, _) => { - if let Some(value_expr) = expr { - optimize_expr(value_expr, state); - } - - state.push_var( - &var_def.name, - AccessMode::ReadWrite, - Expr::Unit(var_def.pos), - ); + Stmt::Let(value_expr, Ident { name, pos, .. }, _) => { + optimize_expr(value_expr, state); + state.push_var(name, AccessMode::ReadWrite, Expr::Unit(*pos)); } // Optimize the statement _ => optimize_stmt(stmt, state, preserve_result), @@ -228,9 +218,7 @@ fn optimize_stmt_block( while let Some(expr) = statements.pop() { match expr { - Stmt::Let(_, expr, _, _) | Stmt::Const(_, expr, _, _) => { - removed = expr.as_ref().map(Expr::is_pure).unwrap_or(true) - } + Stmt::Let(expr, _, _) | Stmt::Const(expr, _, _) => removed = expr.is_pure(), #[cfg(not(feature = "no_module"))] Stmt::Import(expr, _, _) => removed = expr.is_pure(), _ => { @@ -286,9 +274,9 @@ fn optimize_stmt_block( Stmt::Noop(pos) } // Only one let statement - leave it alone - [x] if matches!(x, Stmt::Let(_, _, _, _)) => Stmt::Block(statements, pos), + [x] if matches!(x, Stmt::Let(_, _, _)) => Stmt::Block(statements, pos), // Only one const statement - leave it alone - [x] if matches!(x, Stmt::Const(_, _, _, _)) => Stmt::Block(statements, pos), + [x] if matches!(x, Stmt::Const(_, _, _)) => Stmt::Block(statements, pos), // Only one import statement - leave it alone #[cfg(not(feature = "no_module"))] [x] if matches!(x, Stmt::Import(_, _, _)) => Stmt::Block(statements, pos), @@ -316,17 +304,17 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { }, // if false { if_block } -> Noop - Stmt::If(Expr::BoolConstant(false, pos), x, _) if x.1.is_none() => { + Stmt::If(Expr::BoolConstant(false, pos), x, _) if x.1.is_noop() => { state.set_dirty(); *stmt = Stmt::Noop(*pos); } // if true { if_block } -> if_block - Stmt::If(Expr::BoolConstant(true, _), x, _) if x.1.is_none() => { + Stmt::If(Expr::BoolConstant(true, _), x, _) if x.1.is_noop() => { *stmt = mem::take(&mut x.0); optimize_stmt(stmt, state, true); } // if expr { Noop } - Stmt::If(condition, x, _) if x.1.is_none() && matches!(x.0, Stmt::Noop(_)) => { + Stmt::If(condition, x, _) if x.1.is_noop() && x.0.is_noop() => { state.set_dirty(); let pos = condition.position(); @@ -342,13 +330,13 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { }; } // if expr { if_block } - Stmt::If(condition, x, _) if x.1.is_none() => { + Stmt::If(condition, x, _) if x.1.is_noop() => { optimize_expr(condition, state); optimize_stmt(&mut x.0, state, true); } // if false { if_block } else { else_block } -> else_block - Stmt::If(Expr::BoolConstant(false, _), x, _) if x.1.is_some() => { - *stmt = mem::take(x.1.as_mut().unwrap()); + Stmt::If(Expr::BoolConstant(false, _), x, _) if !x.1.is_noop() => { + *stmt = mem::take(&mut x.1); optimize_stmt(stmt, state, true); } // if true { if_block } else { else_block } -> if_block @@ -360,13 +348,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { Stmt::If(condition, x, _) => { optimize_expr(condition, state); optimize_stmt(&mut x.0, state, true); - if let Some(else_block) = x.1.as_mut() { - optimize_stmt(else_block, state, true); - match else_block { - Stmt::Noop(_) => x.1 = None, // Noop -> no else block - _ => (), - } - } + optimize_stmt(&mut x.1, state, true); } // switch const { ... } @@ -406,24 +388,21 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { } // while false { block } -> Noop - Stmt::While(Some(Expr::BoolConstant(false, pos)), _, _) => { + Stmt::While(Expr::BoolConstant(false, pos), _, _) => { state.set_dirty(); *stmt = Stmt::Noop(*pos) } // while expr { block } Stmt::While(condition, block, _) => { optimize_stmt(block, state, false); - - if let Some(condition) = condition { - optimize_expr(condition, state); - } + optimize_expr(condition, state); match **block { // while expr { break; } -> { expr; } Stmt::Break(pos) => { // Only a single break statement - turn into running the guard expression once state.set_dirty(); - if let Some(condition) = condition { + if !condition.is_unit() { let mut statements = vec![Stmt::Expr(mem::take(condition))]; if preserve_result { statements.push(Stmt::Noop(pos)) @@ -449,14 +428,12 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { optimize_expr(condition, state); } // for id in expr { block } - Stmt::For(iterable, x, _) => { + Stmt::For(iterable, _, block, _) => { optimize_expr(iterable, state); - optimize_stmt(&mut x.1, state, false); + optimize_stmt(block, state, false); } // let id = expr; - Stmt::Let(_, Some(expr), _, _) => optimize_expr(expr, state), - // let id; - Stmt::Let(_, None, _, _) => (), + Stmt::Let(expr, _, _) => optimize_expr(expr, state), // import expr as var; #[cfg(not(feature = "no_module"))] Stmt::Import(expr, _, _) => optimize_expr(expr, state), @@ -789,40 +766,23 @@ fn optimize_top_level( statements.iter_mut().enumerate().for_each(|(i, stmt)| { match stmt { - Stmt::Const(var_def, expr, _, _) if expr.is_some() => { + Stmt::Const(value_expr, Ident { name, .. }, _) => { // Load constants - let value_expr = expr.as_mut().unwrap(); optimize_expr(value_expr, &mut state); if value_expr.is_constant() { - state.push_var(&var_def.name, AccessMode::ReadOnly, value_expr.clone()); - } - - // Keep it in the global scope - if value_expr.is_unit() { - state.set_dirty(); - *expr = None; + state.push_var(name, AccessMode::ReadOnly, value_expr.clone()); } } - Stmt::Const(var_def, None, _, _) => { - state.push_var(&var_def.name, AccessMode::ReadOnly, Expr::Unit(var_def.pos)); - } - Stmt::Let(var_def, expr, _, _) => { - if let Some(value_expr) = expr { - optimize_expr(value_expr, &mut state); - } - - state.push_var( - &var_def.name, - AccessMode::ReadWrite, - Expr::Unit(var_def.pos), - ); + Stmt::Let(value_expr, Ident { name, pos, .. }, _) => { + optimize_expr(value_expr, &mut state); + state.push_var(name, AccessMode::ReadWrite, Expr::Unit(*pos)); } _ => { // Keep all variable declarations at this level // and always keep the last return value let keep = match stmt { - Stmt::Let(_, _, _, _) | Stmt::Const(_, _, _, _) => true, + Stmt::Let(_, _, _) | Stmt::Const(_, _, _) => true, #[cfg(not(feature = "no_module"))] Stmt::Import(_, _, _) => true, _ => i == num_statements - 1, @@ -911,11 +871,11 @@ pub fn optimize_into_ast( // {} -> Noop fn_def.body = match body.pop().unwrap_or_else(|| Stmt::Noop(pos)) { // { return val; } -> val - Stmt::Return((crate::ast::ReturnType::Return, _), Some(expr), _) => { + Stmt::Return(crate::ast::ReturnType::Return, Some(expr), _) => { Stmt::Expr(expr) } // { return; } -> () - Stmt::Return((crate::ast::ReturnType::Return, pos), None, _) => { + Stmt::Return(crate::ast::ReturnType::Return, None, pos) => { Stmt::Expr(Expr::Unit(pos)) } // All others diff --git a/src/parser.rs b/src/parser.rs index 3b1d05f7..e0c58dc9 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -755,7 +755,8 @@ fn parse_map_literal( let expr = parse_expr(input, state, lib, settings.level_up())?; let name = state.get_interned_string(name); - map.push((Ident { name, pos }, expr)); + let public = false; + map.push((Ident { name, pos, public }, expr)); match input.peek().unwrap() { (Token::Comma, _) => { @@ -1033,6 +1034,7 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, + public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1047,6 +1049,7 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, + public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1056,6 +1059,7 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, + public: false, }; Expr::Variable(Box::new((index, None, var_name_def))) } @@ -1075,6 +1079,7 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, + public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1083,6 +1088,7 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, + public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1147,14 +1153,14 @@ fn parse_primary( .into_err(pos)); } - let (_, namespace, Ident { name, pos }) = *x; + let (_, namespace, Ident { name, pos, .. }) = *x; settings.pos = pos; let ns = namespace.map(|(_, ns)| ns); parse_fn_call(input, state, lib, name, true, ns, settings.level_up())? } // Function call (Expr::Variable(x), Token::LeftParen) => { - let (_, namespace, Ident { name, pos }) = *x; + let (_, namespace, Ident { name, pos, .. }) = *x; settings.pos = pos; let ns = namespace.map(|(_, ns)| ns); parse_fn_call(input, state, lib, name, false, ns, settings.level_up())? @@ -1176,6 +1182,7 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(id2), pos: pos2, + public: false, }; Expr::Variable(Box::new((index, namespace, var_name_def))) } @@ -1395,7 +1402,7 @@ fn make_assignment_stmt<'a>( } // var (indexed) = rhs Expr::Variable(x) => { - let (index, _, Ident { name, pos }) = x.as_ref(); + let (index, _, Ident { name, pos, .. }) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { AccessMode::ReadWrite => { Ok(Stmt::Assignment(Box::new((lhs, rhs, op_info)), op_pos)) @@ -1416,7 +1423,7 @@ fn make_assignment_stmt<'a>( } // var[???] (indexed) = rhs, var.??? (indexed) = rhs Expr::Variable(x) => { - let (index, _, Ident { name, pos }) = x.as_ref(); + let (index, _, Ident { name, pos, .. }) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { AccessMode::ReadWrite => { Ok(Stmt::Assignment(Box::new((lhs, rhs, op_info)), op_pos)) @@ -1846,7 +1853,8 @@ fn parse_custom_syntax( let name = state.get_interned_string(s); segments.push(name.clone()); tokens.push(state.get_interned_string(MARKER_IDENT)); - let var_name_def = Ident { name, pos }; + let public = false; + let var_name_def = Ident { name, pos, public }; keywords.push(Expr::Variable(Box::new((None, None, var_name_def)))); } (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { @@ -1995,15 +2003,15 @@ fn parse_if( // if guard { if_body } else ... let else_body = if match_token(input, Token::Else).0 { - Some(if let (Token::If, _) = input.peek().unwrap() { + if let (Token::If, _) = input.peek().unwrap() { // if guard { if_body } else if ... parse_if(input, state, lib, settings.level_up())? } else { // if guard { if_body } else { else-body } parse_block(input, state, lib, settings.level_up())? - }) + } } else { - None + Stmt::Noop(Position::NONE) }; Ok(Stmt::If( @@ -2028,9 +2036,9 @@ fn parse_while_loop( (Token::While, pos) => { ensure_not_statement_expr(input, "a boolean")?; let expr = parse_expr(input, state, lib, settings.level_up())?; - (Some(expr), pos) + (expr, pos) } - (Token::Loop, pos) => (None, pos), + (Token::Loop, pos) => (Expr::Unit(Position::NONE), pos), _ => unreachable!(), }; settings.pos = token_pos; @@ -2130,7 +2138,7 @@ fn parse_for( state.stack.truncate(prev_stack_len); - Ok(Stmt::For(expr, Box::new((name, body)), settings.pos)) + Ok(Stmt::For(expr, name, Box::new(body), settings.pos)) } /// Parse a variable definition statement. @@ -2162,23 +2170,24 @@ fn parse_let( let var_def = Ident { name: name.clone(), pos, + public: export, }; // let name = ... let expr = if match_token(input, Token::Equals).0 { // let name = expr - Some(parse_expr(input, state, lib, settings.level_up())?) + parse_expr(input, state, lib, settings.level_up())? } else { - None + Expr::Unit(Position::NONE) }; state.stack.push((name, var_type)); match var_type { // let name = expr - AccessMode::ReadWrite => Ok(Stmt::Let(Box::new(var_def), expr, export, settings.pos)), + AccessMode::ReadWrite => Ok(Stmt::Let(expr, var_def, settings.pos)), // const name = { expr:constant } - AccessMode::ReadOnly => Ok(Stmt::Const(Box::new(var_def), expr, export, settings.pos)), + AccessMode::ReadOnly => Ok(Stmt::Const(expr, var_def, settings.pos)), } } @@ -2219,10 +2228,11 @@ fn parse_import( Ok(Stmt::Import( expr, - Some(Box::new(Ident { + Some(Ident { name, pos: name_pos, - })), + public: false, + }), settings.pos, )) } @@ -2273,6 +2283,7 @@ fn parse_export( (Token::Identifier(s), pos) => Some(Ident { name: state.get_interned_string(s), pos, + public: false, }), (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { return Err(PERR::Reserved(s).into_err(pos)); @@ -2288,6 +2299,7 @@ fn parse_export( Ident { name: state.get_interned_string(id), pos: id_pos, + public: false, }, rename, )); @@ -2565,16 +2577,13 @@ fn parse_stmt( match input.peek().unwrap() { // `return`/`throw` at - (Token::EOF, pos) => Ok(Stmt::Return((return_type, token_pos), None, *pos)), + (Token::EOF, _) => Ok(Stmt::Return(return_type, None, token_pos)), // `return;` or `throw;` - (Token::SemiColon, _) => { - Ok(Stmt::Return((return_type, token_pos), None, settings.pos)) - } + (Token::SemiColon, _) => Ok(Stmt::Return(return_type, None, token_pos)), // `return` or `throw` with expression (_, _) => { let expr = parse_expr(input, state, lib, settings.level_up())?; - let pos = expr.position(); - Ok(Stmt::Return((return_type, token_pos), Some(expr), pos)) + Ok(Stmt::Return(return_type, Some(expr), token_pos)) } } } @@ -2629,6 +2638,7 @@ fn parse_try_catch( (Token::Identifier(s), pos) => Ident { name: state.get_interned_string(s), pos, + public: false, }, (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; @@ -2861,6 +2871,7 @@ fn parse_anon_fn( .map(|(name, &pos)| Ident { name: name.clone(), pos, + public: false, }) .collect() } diff --git a/tests/optimizer.rs b/tests/optimizer.rs index d062ec5f..c5b91b42 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -54,8 +54,9 @@ fn test_optimizer_parse() -> Result<(), Box> { engine.set_optimization_level(OptimizationLevel::Simple); let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; + println!("{:?}", ast); - assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, statements: [Block([Const(Ident("DECISION" @ 1:9), Some(BoolConstant(false, 1:20)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); + assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, statements: [Block([Const(BoolConstant(false, 1:20), Ident("DECISION" @ 1:9), 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)], functions: Module("#)); let ast = engine.compile("if 1 == 2 { 42 }")?; From 2ff2789326245a5b716f57c67de662f10bce2056 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 9 Mar 2021 23:48:40 +0800 Subject: [PATCH 10/19] Fix Stmt size. --- src/ast.rs | 16 ++++++++-------- src/engine.rs | 12 ++++++------ src/optimize.rs | 4 ++-- src/parser.rs | 18 +++++++++++++----- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index a8ed9a4e..c773d760 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -832,7 +832,7 @@ pub enum Stmt { /// `do` `{` stmt `}` `while`|`until` expr Do(Box, Expr, bool, Position), /// `for` id `in` expr `{` stmt `}` - For(Expr, String, Box, Position), + For(Expr, Box<(String, Stmt)>, Position), /// \[`export`\] `let` id `=` expr Let(Expr, Ident, Position), /// \[`export`\] `const` id `=` expr @@ -853,7 +853,7 @@ pub enum Stmt { Return(ReturnType, Option, Position), /// `import` expr `as` var #[cfg(not(feature = "no_module"))] - Import(Expr, Option, Position), + Import(Expr, Ident, Position), /// `export` var `as` var `,` ... #[cfg(not(feature = "no_module"))] Export(Vec<(Ident, Option)>, Position), @@ -890,7 +890,7 @@ impl Stmt { | Self::Switch(_, _, pos) | Self::While(_, _, pos) | Self::Do(_, _, _, pos) - | Self::For(_, _, _, pos) + | Self::For(_, _, pos) | Self::Return(_, _, pos) | Self::Let(_, _, pos) | Self::Const(_, _, pos) @@ -919,7 +919,7 @@ impl Stmt { | Self::Switch(_, _, pos) | Self::While(_, _, pos) | Self::Do(_, _, _, pos) - | Self::For(_, _, _, pos) + | Self::For(_, _, pos) | Self::Return(_, _, pos) | Self::Let(_, _, pos) | Self::Const(_, _, pos) @@ -946,7 +946,7 @@ impl Stmt { Self::If(_, _, _) | Self::Switch(_, _, _) | Self::While(_, _, _) - | Self::For(_, _, _, _) + | Self::For(_, _, _) | Self::Block(_, _) | Self::TryCatch(_, _, _) => true, @@ -985,7 +985,7 @@ impl Stmt { Self::While(condition, block, _) | Self::Do(block, condition, _, _) => { condition.is_pure() && block.is_pure() } - Self::For(iterable, _, block, _) => iterable.is_pure() && block.is_pure(), + Self::For(iterable, x, _) => iterable.is_pure() && x.1.is_pure(), Self::Let(_, _, _) | Self::Const(_, _, _) | Self::Assignment(_, _) => false, Self::Block(block, _) => block.iter().all(|stmt| stmt.is_pure()), Self::Continue(_) | Self::Break(_) | Self::Return(_, _, _) => false, @@ -1024,9 +1024,9 @@ impl Stmt { e.walk(path, on_node); s.walk(path, on_node); } - Self::For(e, _, s, _) => { + Self::For(e, x, _) => { e.walk(path, on_node); - s.walk(path, on_node); + x.1.walk(path, on_node); } Self::Assignment(x, _) => { x.0.walk(path, on_node); diff --git a/src/engine.rs b/src/engine.rs index 62cf0ac7..d061c54b 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -2126,8 +2126,8 @@ impl Engine { }, // For loop - Stmt::For(expr, name, x, _) => { - let stmt = x.as_ref(); + Stmt::For(expr, x, _) => { + let (name, stmt) = x.as_ref(); let iter_obj = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .flatten(); @@ -2348,7 +2348,7 @@ impl Engine { // Import statement #[cfg(not(feature = "no_module"))] - Stmt::Import(expr, alias, _pos) => { + Stmt::Import(expr, Ident { name, public, .. }, _pos) => { // Guard against too many modules #[cfg(not(feature = "unchecked"))] if state.modules >= self.max_modules() { @@ -2375,14 +2375,14 @@ impl Engine { }) .unwrap_or_else(|| self.module_resolver.resolve(self, &path, expr_pos))?; - if let Some(name_def) = alias { + if *public { if !module.is_indexed() { // Index the module (making a clone copy if necessary) if it is not indexed let mut module = crate::fn_native::shared_take_or_clone(module); module.build_index(); - mods.push(name_def.name.clone(), module); + mods.push(name.clone(), module); } else { - mods.push(name_def.name.clone(), module); + mods.push(name.clone(), module); } } diff --git a/src/optimize.rs b/src/optimize.rs index 571ebd01..ad3c3f72 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -428,9 +428,9 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { optimize_expr(condition, state); } // for id in expr { block } - Stmt::For(iterable, _, block, _) => { + Stmt::For(iterable, x, _) => { optimize_expr(iterable, state); - optimize_stmt(block, state, false); + optimize_stmt(&mut x.1, state, false); } // let id = expr; Stmt::Let(expr, _, _) => optimize_expr(expr, state), diff --git a/src/parser.rs b/src/parser.rs index e0c58dc9..b8088159 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2138,7 +2138,7 @@ fn parse_for( state.stack.truncate(prev_stack_len); - Ok(Stmt::For(expr, name, Box::new(body), settings.pos)) + Ok(Stmt::For(expr, Box::new((name, body)), settings.pos)) } /// Parse a variable definition statement. @@ -2210,7 +2210,15 @@ fn parse_import( // import expr as ... if !match_token(input, Token::As).0 { - return Ok(Stmt::Import(expr, None, settings.pos)); + return Ok(Stmt::Import( + expr, + Ident { + name: "".into(), + pos: Position::NONE, + public: false, + }, + settings.pos, + )); } // import expr as name ... @@ -2228,11 +2236,11 @@ fn parse_import( Ok(Stmt::Import( expr, - Some(Ident { + Ident { name, pos: name_pos, - public: false, - }), + public: true, + }, settings.pos, )) } From 352408fd36af21a624fe8901494eeb5fdef3a6b9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 10 Mar 2021 12:27:10 +0800 Subject: [PATCH 11/19] Flatten statement blocks. --- CHANGELOG.md | 3 + src/ast.rs | 105 ++++++++++++++----- src/engine.rs | 158 +++++++++++++++++----------- src/optimize.rs | 249 ++++++++++++++++++++++++++------------------- src/parser.rs | 67 ++++++------ tests/optimizer.rs | 3 +- 6 files changed, 356 insertions(+), 229 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0c37190..0481e729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Rhai Release Notes Version 0.19.14 =============== +This version runs faster due to optimizations done on AST node structures. + Bug fixes --------- @@ -35,6 +37,7 @@ Breaking changes Enhancements ------------ +* Layout of AST nodes is optimized to reduce redirections, so speed is improved. * Function calls are more optimized and should now run faster. * `range` function now supports negative step and decreasing streams (i.e. to < from). * More information is provided to the error variable captured by the `catch` statement in an _object map_. diff --git a/src/ast.rs b/src/ast.rs index c773d760..caaecf11 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -757,8 +757,6 @@ pub struct Ident { pub name: ImmutableString, /// Declaration position. pub pos: Position, - /// Is this identifier public? - pub public: bool, } impl fmt::Debug for Ident { @@ -806,6 +804,29 @@ impl<'a> From<&'a Expr> for ASTNode<'a> { } } +/// _(INTERNALS)_ A statements block. +/// Exported under the `internals` feature only. +/// +/// # Volatile Data Structure +/// +/// This type is volatile and may change. +#[derive(Debug, Clone, Hash)] +pub struct StmtBlock { + pub statements: StaticVec, + pub pos: Position, +} + +impl StmtBlock { + /// Is this statements block empty? + pub fn is_empty(&self) -> bool { + self.statements.is_empty() + } + /// Number of statements in this statements block. + pub fn len(&self) -> usize { + self.statements.len() + } +} + /// _(INTERNALS)_ A statement. /// Exported under the `internals` feature only. /// @@ -817,7 +838,7 @@ pub enum Stmt { /// No-op. Noop(Position), /// `if` expr `{` stmt `}` `else` `{` stmt `}` - If(Expr, Box<(Stmt, Stmt)>, Position), + If(Expr, Box<(StmtBlock, StmtBlock)>, Position), /// `switch` expr `{` literal or _ `=>` stmt `,` ... `}` Switch( Expr, @@ -828,21 +849,25 @@ pub enum Stmt { Position, ), /// `while` expr `{` stmt `}` - While(Expr, Box, Position), + While(Expr, Box, Position), /// `do` `{` stmt `}` `while`|`until` expr - Do(Box, Expr, bool, Position), + Do(Box, Expr, bool, Position), /// `for` id `in` expr `{` stmt `}` - For(Expr, Box<(String, Stmt)>, Position), + For(Expr, Box<(String, StmtBlock)>, Position), /// \[`export`\] `let` id `=` expr - Let(Expr, Ident, Position), + Let(Expr, Ident, bool, Position), /// \[`export`\] `const` id `=` expr - Const(Expr, Ident, Position), + Const(Expr, Ident, bool, Position), /// expr op`=` expr Assignment(Box<(Expr, Expr, Option)>, Position), /// `{` stmt`;` ... `}` Block(Vec, Position), /// `try` `{` stmt; ... `}` `catch` `(` var `)` `{` stmt; ... `}` - TryCatch(Box<(Stmt, Option, Stmt)>, Position, Position), + TryCatch( + Box<(StmtBlock, Option, StmtBlock)>, + Position, + Position, + ), /// [expression][Expr] Expr(Expr), /// `continue` @@ -853,7 +878,7 @@ pub enum Stmt { Return(ReturnType, Option, Position), /// `import` expr `as` var #[cfg(not(feature = "no_module"))] - Import(Expr, Ident, Position), + Import(Expr, Option, Position), /// `export` var `as` var `,` ... #[cfg(not(feature = "no_module"))] Export(Vec<(Ident, Option)>, Position), @@ -869,6 +894,22 @@ impl Default for Stmt { } } +impl From for StmtBlock { + fn from(stmt: Stmt) -> Self { + match stmt { + Stmt::Block(block, pos) => Self { + statements: block.into(), + pos, + }, + Stmt::Noop(pos) => Self { + statements: Default::default(), + pos, + }, + _ => panic!("cannot convert {:?} into a StmtBlock", stmt), + } + } +} + impl Stmt { /// Is this statement [`Noop`][Stmt::Noop]? #[inline(always)] @@ -892,8 +933,8 @@ impl Stmt { | Self::Do(_, _, _, pos) | Self::For(_, _, pos) | Self::Return(_, _, pos) - | Self::Let(_, _, pos) - | Self::Const(_, _, pos) + | Self::Let(_, _, _, pos) + | Self::Const(_, _, _, pos) | Self::TryCatch(_, pos, _) => *pos, Self::Expr(x) => x.position(), @@ -921,8 +962,8 @@ impl Stmt { | Self::Do(_, _, _, pos) | Self::For(_, _, pos) | Self::Return(_, _, pos) - | Self::Let(_, _, pos) - | Self::Const(_, _, pos) + | Self::Let(_, _, _, pos) + | Self::Const(_, _, _, pos) | Self::TryCatch(_, pos, _) => *pos = new_pos, Self::Expr(x) => { @@ -953,8 +994,8 @@ impl Stmt { // A No-op requires a semicolon in order to know it is an empty statement! Self::Noop(_) => false, - Self::Let(_, _, _) - | Self::Const(_, _, _) + Self::Let(_, _, _, _) + | Self::Const(_, _, _, _) | Self::Assignment(_, _) | Self::Expr(_) | Self::Do(_, _, _, _) @@ -976,20 +1017,28 @@ impl Stmt { match self { Self::Noop(_) => true, Self::Expr(expr) => expr.is_pure(), - Self::If(condition, x, _) => condition.is_pure() && x.0.is_pure() && x.1.is_pure(), + Self::If(condition, x, _) => { + condition.is_pure() + && x.0.statements.iter().all(Stmt::is_pure) + && x.1.statements.iter().all(Stmt::is_pure) + } Self::Switch(expr, x, _) => { expr.is_pure() && x.0.values().all(Stmt::is_pure) && x.1.as_ref().map(Stmt::is_pure).unwrap_or(true) } Self::While(condition, block, _) | Self::Do(block, condition, _, _) => { - condition.is_pure() && block.is_pure() + condition.is_pure() && block.statements.iter().all(Stmt::is_pure) } - Self::For(iterable, x, _) => iterable.is_pure() && x.1.is_pure(), - Self::Let(_, _, _) | Self::Const(_, _, _) | Self::Assignment(_, _) => false, + Self::For(iterable, x, _) => { + iterable.is_pure() && x.1.statements.iter().all(Stmt::is_pure) + } + Self::Let(_, _, _, _) | Self::Const(_, _, _, _) | Self::Assignment(_, _) => false, Self::Block(block, _) => block.iter().all(|stmt| stmt.is_pure()), Self::Continue(_) | Self::Break(_) | Self::Return(_, _, _) => false, - Self::TryCatch(x, _, _) => x.0.is_pure() && x.2.is_pure(), + Self::TryCatch(x, _, _) => { + x.0.statements.iter().all(Stmt::is_pure) && x.2.statements.iter().all(Stmt::is_pure) + } #[cfg(not(feature = "no_module"))] Self::Import(_, _, _) => false, @@ -1007,11 +1056,11 @@ impl Stmt { on_node(path); match self { - Self::Let(e, _, _) | Self::Const(e, _, _) => e.walk(path, on_node), + Self::Let(e, _, _, _) | Self::Const(e, _, _, _) => e.walk(path, on_node), Self::If(e, x, _) => { e.walk(path, on_node); - x.0.walk(path, on_node); - x.1.walk(path, on_node); + x.0.statements.iter().for_each(|s| s.walk(path, on_node)); + x.1.statements.iter().for_each(|s| s.walk(path, on_node)); } Self::Switch(e, x, _) => { e.walk(path, on_node); @@ -1022,11 +1071,11 @@ impl Stmt { } Self::While(e, s, _) | Self::Do(s, e, _, _) => { e.walk(path, on_node); - s.walk(path, on_node); + s.statements.iter().for_each(|s| s.walk(path, on_node)); } Self::For(e, x, _) => { e.walk(path, on_node); - x.1.walk(path, on_node); + x.1.statements.iter().for_each(|s| s.walk(path, on_node)); } Self::Assignment(x, _) => { x.0.walk(path, on_node); @@ -1034,8 +1083,8 @@ impl Stmt { } Self::Block(x, _) => x.iter().for_each(|s| s.walk(path, on_node)), Self::TryCatch(x, _, _) => { - x.0.walk(path, on_node); - x.2.walk(path, on_node); + x.0.statements.iter().for_each(|s| s.walk(path, on_node)); + x.2.statements.iter().for_each(|s| s.walk(path, on_node)); } Self::Expr(e) | Self::Return(_, Some(e), _) => e.walk(path, on_node), #[cfg(not(feature = "no_module"))] diff --git a/src/engine.rs b/src/engine.rs index d061c54b..297570ff 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,6 +1,6 @@ //! Main module defining the script evaluation [`Engine`]. -use crate::ast::{Expr, FnCallExpr, FnHash, Ident, OpAssignment, ReturnType, Stmt}; +use crate::ast::{Expr, FnCallExpr, FnHash, Ident, OpAssignment, ReturnType, Stmt, StmtBlock}; use crate::dynamic::{map_std_type_name, AccessMode, Union, Variant}; use crate::fn_native::{ CallableFunction, IteratorFn, OnDebugCallback, OnPrintCallback, OnProgressCallback, @@ -2031,15 +2031,28 @@ impl Engine { // If statement Stmt::If(expr, x, _) => { - let (if_block, else_block) = x.as_ref(); + let ( + StmtBlock { + statements: if_stmt, + .. + }, + StmtBlock { + statements: else_stmt, + .. + }, + ) = x.as_ref(); self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .as_bool() .map_err(|err| self.make_type_mismatch_err::(err, expr.position())) .and_then(|guard_val| { if guard_val { - self.eval_stmt(scope, mods, state, lib, this_ptr, if_block, level) - } else if !else_block.is_noop() { - self.eval_stmt(scope, mods, state, lib, this_ptr, else_block, level) + self.eval_stmt_block( + scope, mods, state, lib, this_ptr, if_stmt, true, level, + ) + } else if !else_stmt.is_empty() { + self.eval_stmt_block( + scope, mods, state, lib, this_ptr, else_stmt, true, level, + ) } else { Ok(Dynamic::UNIT) } @@ -2076,58 +2089,70 @@ impl Engine { } // While loop - Stmt::While(expr, body, _) => loop { - let condition = if !expr.is_unit() { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? - .as_bool() - .map_err(|err| self.make_type_mismatch_err::(err, expr.position()))? - } else { - true - }; + Stmt::While(expr, body, _) => { + let body = &body.statements; + loop { + let condition = if !expr.is_unit() { + self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? + .as_bool() + .map_err(|err| { + self.make_type_mismatch_err::(err, expr.position()) + })? + } else { + true + }; - if condition { - match self.eval_stmt(scope, mods, state, lib, this_ptr, body, level) { + if condition { + match self + .eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level) + { + Ok(_) => (), + Err(err) => match *err { + EvalAltResult::LoopBreak(false, _) => (), + EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), + _ => return Err(err), + }, + } + } else { + return Ok(Dynamic::UNIT); + } + } + } + + // Do loop + Stmt::Do(body, expr, is_while, _) => { + let body = &body.statements; + + loop { + match self.eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level) + { Ok(_) => (), Err(err) => match *err { - EvalAltResult::LoopBreak(false, _) => (), + EvalAltResult::LoopBreak(false, _) => continue, EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), _ => return Err(err), }, } - } else { - return Ok(Dynamic::UNIT); - } - }, - // Do loop - Stmt::Do(body, expr, is_while, _) => loop { - match self.eval_stmt(scope, mods, state, lib, this_ptr, body, level) { - Ok(_) => (), - Err(err) => match *err { - EvalAltResult::LoopBreak(false, _) => continue, - EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), - _ => return Err(err), - }, - } - - if self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? - .as_bool() - .map_err(|err| self.make_type_mismatch_err::(err, expr.position()))? - { - if !*is_while { - return Ok(Dynamic::UNIT); - } - } else { - if *is_while { - return Ok(Dynamic::UNIT); + if self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? + .as_bool() + .map_err(|err| self.make_type_mismatch_err::(err, expr.position()))? + { + if !*is_while { + return Ok(Dynamic::UNIT); + } + } else { + if *is_while { + return Ok(Dynamic::UNIT); + } } } - }, + } // For loop Stmt::For(expr, x, _) => { - let (name, stmt) = x.as_ref(); + let (name, StmtBlock { statements, pos }) = x.as_ref(); let iter_obj = self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .flatten(); @@ -2176,9 +2201,11 @@ impl Engine { *loop_var = value; } - self.inc_operations(state, stmt.position())?; + self.inc_operations(state, *pos)?; - match self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level) { + match self.eval_stmt_block( + scope, mods, state, lib, this_ptr, statements, true, level, + ) { Ok(_) => (), Err(err) => match *err { EvalAltResult::LoopBreak(false, _) => (), @@ -2204,10 +2231,20 @@ impl Engine { // Try/Catch statement Stmt::TryCatch(x, _, _) => { - let (try_body, err_var, catch_body) = x.as_ref(); + let ( + StmtBlock { + statements: try_body, + .. + }, + err_var, + StmtBlock { + statements: catch_body, + .. + }, + ) = x.as_ref(); let result = self - .eval_stmt(scope, mods, state, lib, this_ptr, try_body, level) + .eval_stmt_block(scope, mods, state, lib, this_ptr, try_body, true, level) .map(|_| Dynamic::UNIT); match result { @@ -2266,8 +2303,9 @@ impl Engine { scope.push(unsafe_cast_var_name_to_lifetime(&name), err_value); } - let result = - self.eval_stmt(scope, mods, state, lib, this_ptr, catch_body, level); + let result = self.eval_stmt_block( + scope, mods, state, lib, this_ptr, catch_body, true, level, + ); state.scope_level -= 1; scope.rewind(orig_scope_len); @@ -2314,11 +2352,11 @@ impl Engine { } // Let/const statement - Stmt::Let(expr, Ident { name, public, .. }, _) - | Stmt::Const(expr, Ident { name, public, .. }, _) => { + Stmt::Let(expr, Ident { name, .. }, export, _) + | Stmt::Const(expr, Ident { name, .. }, export, _) => { let entry_type = match stmt { - Stmt::Let(_, _, _) => AccessMode::ReadWrite, - Stmt::Const(_, _, _) => AccessMode::ReadOnly, + Stmt::Let(_, _, _, _) => AccessMode::ReadWrite, + Stmt::Const(_, _, _, _) => AccessMode::ReadOnly, _ => unreachable!("should be Stmt::Let or Stmt::Const, but gets {:?}", stmt), }; @@ -2329,9 +2367,9 @@ impl Engine { let (var_name, _alias): (Cow<'_, str>, _) = if state.is_global() { ( name.to_string().into(), - if *public { Some(name.clone()) } else { None }, + if *export { Some(name.clone()) } else { None }, ) - } else if *public { + } else if *export { unreachable!("exported variable not on global level"); } else { (unsafe_cast_var_name_to_lifetime(name).into(), None) @@ -2348,7 +2386,7 @@ impl Engine { // Import statement #[cfg(not(feature = "no_module"))] - Stmt::Import(expr, Ident { name, public, .. }, _pos) => { + Stmt::Import(expr, export, _pos) => { // Guard against too many modules #[cfg(not(feature = "unchecked"))] if state.modules >= self.max_modules() { @@ -2375,14 +2413,14 @@ impl Engine { }) .unwrap_or_else(|| self.module_resolver.resolve(self, &path, expr_pos))?; - if *public { + if let Some(name) = export.as_ref().map(|x| x.name.clone()) { if !module.is_indexed() { // Index the module (making a clone copy if necessary) if it is not indexed let mut module = crate::fn_native::shared_take_or_clone(module); module.build_index(); - mods.push(name.clone(), module); + mods.push(name, module); } else { - mods.push(name.clone(), module); + mods.push(name, module); } } diff --git a/src/optimize.rs b/src/optimize.rs index ad3c3f72..b70f72a1 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -171,8 +171,11 @@ fn optimize_stmt_block( pos: Position, state: &mut State, preserve_result: bool, - count_promote_as_dirty: bool, -) -> Stmt { +) -> Vec { + if statements.is_empty() { + return statements; + } + let orig_len = statements.len(); // Original number of statements in the block, for change detection let orig_constants_len = state.variables.len(); // Original number of constants in the state, for restore later let orig_propagate_constants = state.propagate_constants; @@ -181,7 +184,7 @@ fn optimize_stmt_block( statements.iter_mut().for_each(|stmt| { match stmt { // Add constant literals into the state - Stmt::Const(value_expr, Ident { name, .. }, _) => { + Stmt::Const(value_expr, Ident { name, .. }, _, _) => { optimize_expr(value_expr, state); if value_expr.is_constant() { @@ -189,7 +192,7 @@ fn optimize_stmt_block( } } // Add variables into the state - Stmt::Let(value_expr, Ident { name, pos, .. }, _) => { + Stmt::Let(value_expr, Ident { name, pos, .. }, _, _) => { optimize_expr(value_expr, state); state.push_var(name, AccessMode::ReadWrite, Expr::Unit(*pos)); } @@ -218,7 +221,7 @@ fn optimize_stmt_block( while let Some(expr) = statements.pop() { match expr { - Stmt::Let(expr, _, _) | Stmt::Const(expr, _, _) => removed = expr.is_pure(), + Stmt::Let(expr, _, _, _) | Stmt::Const(expr, _, _, _) => removed = expr.is_pure(), #[cfg(not(feature = "no_module"))] Stmt::Import(expr, _, _) => removed = expr.is_pure(), _ => { @@ -238,7 +241,7 @@ fn optimize_stmt_block( statements .iter_mut() .enumerate() - .for_each(|(i, stmt)| optimize_stmt(stmt, state, i == num_statements)); + .for_each(|(i, stmt)| optimize_stmt(stmt, state, i >= num_statements - 1)); } // Remove everything following the the first return/throw @@ -267,28 +270,7 @@ fn optimize_stmt_block( state.propagate_constants = orig_propagate_constants; - match &statements[..] { - // No statements in block - change to No-op - [] => { - state.set_dirty(); - Stmt::Noop(pos) - } - // Only one let statement - leave it alone - [x] if matches!(x, Stmt::Let(_, _, _)) => Stmt::Block(statements, pos), - // Only one const statement - leave it alone - [x] if matches!(x, Stmt::Const(_, _, _)) => Stmt::Block(statements, pos), - // Only one import statement - leave it alone - #[cfg(not(feature = "no_module"))] - [x] if matches!(x, Stmt::Import(_, _, _)) => Stmt::Block(statements, pos), - // Only one statement - promote - [_] => { - if count_promote_as_dirty { - state.set_dirty(); - } - statements.remove(0) - } - _ => Stmt::Block(statements, pos), - } + statements } /// Optimize a [statement][Stmt]. @@ -303,18 +285,8 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { } }, - // if false { if_block } -> Noop - Stmt::If(Expr::BoolConstant(false, pos), x, _) if x.1.is_noop() => { - state.set_dirty(); - *stmt = Stmt::Noop(*pos); - } - // if true { if_block } -> if_block - Stmt::If(Expr::BoolConstant(true, _), x, _) if x.1.is_noop() => { - *stmt = mem::take(&mut x.0); - optimize_stmt(stmt, state, true); - } - // if expr { Noop } - Stmt::If(condition, x, _) if x.1.is_noop() && x.0.is_noop() => { + // if expr {} + Stmt::If(condition, x, _) if x.0.is_empty() && x.1.is_empty() => { state.set_dirty(); let pos = condition.position(); @@ -323,32 +295,60 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { *stmt = if preserve_result { // -> { expr, Noop } - Stmt::Block(vec![Stmt::Expr(expr), mem::take(&mut x.0)], pos) + Stmt::Block(vec![Stmt::Expr(expr), Stmt::Noop(pos)], pos) } else { // -> expr Stmt::Expr(expr) }; } - // if expr { if_block } - Stmt::If(condition, x, _) if x.1.is_noop() => { - optimize_expr(condition, state); - optimize_stmt(&mut x.0, state, true); + // if false { if_block } -> Noop + Stmt::If(Expr::BoolConstant(false, pos), x, _) if x.1.is_empty() => { + state.set_dirty(); + *stmt = Stmt::Noop(*pos); } // if false { if_block } else { else_block } -> else_block - Stmt::If(Expr::BoolConstant(false, _), x, _) if !x.1.is_noop() => { - *stmt = mem::take(&mut x.1); - optimize_stmt(stmt, state, true); + Stmt::If(Expr::BoolConstant(false, _), x, _) => { + state.set_dirty(); + *stmt = match optimize_stmt_block( + mem::take(&mut x.1.statements).into_vec(), + x.1.pos, + state, + preserve_result, + ) { + statements if statements.is_empty() => Stmt::Noop(x.1.pos), + statements => Stmt::Block(statements, x.1.pos), + } } // if true { if_block } else { else_block } -> if_block Stmt::If(Expr::BoolConstant(true, _), x, _) => { - *stmt = mem::take(&mut x.0); - optimize_stmt(stmt, state, true); + state.set_dirty(); + *stmt = match optimize_stmt_block( + mem::take(&mut x.0.statements).into_vec(), + x.0.pos, + state, + preserve_result, + ) { + statements if statements.is_empty() => Stmt::Noop(x.0.pos), + statements => Stmt::Block(statements, x.0.pos), + } } // if expr { if_block } else { else_block } Stmt::If(condition, x, _) => { optimize_expr(condition, state); - optimize_stmt(&mut x.0, state, true); - optimize_stmt(&mut x.1, state, true); + x.0.statements = optimize_stmt_block( + mem::take(&mut x.0.statements).into_vec(), + x.0.pos, + state, + preserve_result, + ) + .into(); + x.1.statements = optimize_stmt_block( + mem::take(&mut x.1.statements).into_vec(), + x.1.pos, + state, + preserve_result, + ) + .into(); } // switch const { ... } @@ -376,9 +376,9 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { Stmt::Switch(expr, x, _) => { optimize_expr(expr, state); x.0.values_mut() - .for_each(|stmt| optimize_stmt(stmt, state, true)); + .for_each(|stmt| optimize_stmt(stmt, state, preserve_result)); if let Some(def_stmt) = x.1.as_mut() { - optimize_stmt(def_stmt, state, true); + optimize_stmt(def_stmt, state, preserve_result); match def_stmt { Stmt::Noop(_) | Stmt::Expr(Expr::Unit(_)) => x.1 = None, @@ -394,70 +394,122 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { } // while expr { block } Stmt::While(condition, block, _) => { - optimize_stmt(block, state, false); optimize_expr(condition, state); - match **block { - // while expr { break; } -> { expr; } - Stmt::Break(pos) => { - // Only a single break statement - turn into running the guard expression once - state.set_dirty(); - if !condition.is_unit() { - let mut statements = vec![Stmt::Expr(mem::take(condition))]; - if preserve_result { - statements.push(Stmt::Noop(pos)) - } - *stmt = Stmt::Block(statements, pos); - } else { - *stmt = Stmt::Noop(pos); - }; + block.statements = optimize_stmt_block( + mem::take(&mut block.statements).into_vec(), + block.pos, + state, + false, + ) + .into(); + + if block.len() == 1 { + match block.statements[0] { + // while expr { break; } -> { expr; } + Stmt::Break(pos) => { + // Only a single break statement - turn into running the guard expression once + state.set_dirty(); + if !condition.is_unit() { + let mut statements = vec![Stmt::Expr(mem::take(condition))]; + if preserve_result { + statements.push(Stmt::Noop(pos)) + } + *stmt = Stmt::Block(statements, pos); + } else { + *stmt = Stmt::Noop(pos); + }; + } + _ => (), } - _ => (), } } // do { block } while false | do { block } until true -> { block } Stmt::Do(block, Expr::BoolConstant(true, _), false, _) | Stmt::Do(block, Expr::BoolConstant(false, _), true, _) => { state.set_dirty(); - optimize_stmt(block.as_mut(), state, false); - *stmt = mem::take(block.as_mut()); + *stmt = Stmt::Block( + optimize_stmt_block( + mem::take(&mut block.statements).into_vec(), + block.pos, + state, + false, + ), + block.pos, + ); } // do { block } while|until expr Stmt::Do(block, condition, _, _) => { - optimize_stmt(block.as_mut(), state, false); optimize_expr(condition, state); + block.statements = optimize_stmt_block( + mem::take(&mut block.statements).into_vec(), + block.pos, + state, + false, + ) + .into(); } // for id in expr { block } Stmt::For(iterable, x, _) => { optimize_expr(iterable, state); - optimize_stmt(&mut x.1, state, false); + x.1.statements = optimize_stmt_block( + mem::take(&mut x.1.statements).into_vec(), + x.1.pos, + state, + false, + ) + .into(); } // let id = expr; - Stmt::Let(expr, _, _) => optimize_expr(expr, state), + Stmt::Let(expr, _, _, _) => optimize_expr(expr, state), // import expr as var; #[cfg(not(feature = "no_module"))] Stmt::Import(expr, _, _) => optimize_expr(expr, state), // { block } Stmt::Block(statements, pos) => { - *stmt = optimize_stmt_block(mem::take(statements), *pos, state, preserve_result, true); + *stmt = match optimize_stmt_block(mem::take(statements), *pos, state, preserve_result) { + statements if statements.is_empty() => { + state.set_dirty(); + Stmt::Noop(*pos) + } + // Only one statement - promote + mut statements if statements.len() == 1 => { + state.set_dirty(); + statements.pop().unwrap() + } + statements => Stmt::Block(statements, *pos), + }; } - // try { block } catch ( var ) { block } - Stmt::TryCatch(x, _, _) if x.0.is_pure() => { + // try { pure block } catch ( var ) { block } + Stmt::TryCatch(x, _, _) if x.0.statements.iter().all(Stmt::is_pure) => { // If try block is pure, there will never be any exceptions state.set_dirty(); - let pos = x.0.position(); - optimize_stmt(&mut x.0, state, preserve_result); - let mut statements = match mem::take(&mut x.0) { - Stmt::Block(statements, _) => statements, - stmt => vec![stmt], - }; - statements.push(Stmt::Noop(pos)); - *stmt = Stmt::Block(statements, pos); + *stmt = Stmt::Block( + optimize_stmt_block( + mem::take(&mut x.0.statements).into_vec(), + x.0.pos, + state, + false, + ), + x.0.pos, + ); } // try { block } catch ( var ) { block } Stmt::TryCatch(x, _, _) => { - optimize_stmt(&mut x.0, state, false); - optimize_stmt(&mut x.2, state, false); + x.0.statements = optimize_stmt_block( + mem::take(&mut x.0.statements).into_vec(), + x.0.pos, + state, + false, + ) + .into(); + x.2.statements = optimize_stmt_block( + mem::take(&mut x.2.statements).into_vec(), + x.2.pos, + state, + false, + ) + .into(); } // {} Stmt::Expr(Expr::Stmt(x, pos)) if x.is_empty() => { @@ -492,17 +544,10 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { // {} Expr::Stmt(x, pos) if x.is_empty() => { state.set_dirty(); *expr = Expr::Unit(*pos) } // { stmt; ... } - do not count promotion as dirty because it gets turned back into an array - Expr::Stmt(x, pos) => match optimize_stmt_block(mem::take(x).into_vec(), *pos, state, true, false) { - // {} - Stmt::Noop(_) => { state.set_dirty(); *expr = Expr::Unit(*pos); } - // { stmt, .. } - Stmt::Block(statements, _) => *x = Box::new(statements.into()), - // { expr } - Stmt::Expr(inner) => { state.set_dirty(); *expr = inner; } - // { stmt } - stmt => x.push(stmt), + Expr::Stmt(x, pos) => { + let statements = optimize_stmt_block(mem::take(x).into_vec(), *pos, state, true); + *expr = Expr::Stmt(Box::new(statements.into()), *pos); } - // lhs.rhs #[cfg(not(feature = "no_object"))] Expr::Dot(x, _) => match (&mut x.lhs, &mut x.rhs) { @@ -766,7 +811,7 @@ fn optimize_top_level( statements.iter_mut().enumerate().for_each(|(i, stmt)| { match stmt { - Stmt::Const(value_expr, Ident { name, .. }, _) => { + Stmt::Const(value_expr, Ident { name, .. }, _, _) => { // Load constants optimize_expr(value_expr, &mut state); @@ -774,7 +819,7 @@ fn optimize_top_level( state.push_var(name, AccessMode::ReadOnly, value_expr.clone()); } } - Stmt::Let(value_expr, Ident { name, pos, .. }, _) => { + Stmt::Let(value_expr, Ident { name, pos, .. }, _, _) => { optimize_expr(value_expr, &mut state); state.push_var(name, AccessMode::ReadWrite, Expr::Unit(*pos)); } @@ -782,10 +827,10 @@ fn optimize_top_level( // Keep all variable declarations at this level // and always keep the last return value let keep = match stmt { - Stmt::Let(_, _, _) | Stmt::Const(_, _, _) => true, + Stmt::Let(_, _, _, _) | Stmt::Const(_, _, _, _) => true, #[cfg(not(feature = "no_module"))] Stmt::Import(_, _, _) => true, - _ => i == num_statements - 1, + _ => i >= num_statements - 1, }; optimize_stmt(stmt, &mut state, keep); } diff --git a/src/parser.rs b/src/parser.rs index b8088159..56460f8b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2,7 +2,7 @@ use crate::ast::{ BinaryExpr, CustomExpr, Expr, FnCallExpr, FnHash, Ident, OpAssignment, ReturnType, ScriptFnDef, - Stmt, + Stmt, StmtBlock, }; use crate::dynamic::{AccessMode, Union}; use crate::engine::{KEYWORD_THIS, OP_CONTAINS}; @@ -755,8 +755,7 @@ fn parse_map_literal( let expr = parse_expr(input, state, lib, settings.level_up())?; let name = state.get_interned_string(name); - let public = false; - map.push((Ident { name, pos, public }, expr)); + map.push((Ident { name, pos }, expr)); match input.peek().unwrap() { (Token::Comma, _) => { @@ -1034,7 +1033,6 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, - public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1049,7 +1047,6 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, - public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1059,7 +1056,6 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, - public: false, }; Expr::Variable(Box::new((index, None, var_name_def))) } @@ -1079,7 +1075,6 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, - public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1088,7 +1083,6 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(s), pos: settings.pos, - public: false, }; Expr::Variable(Box::new((None, None, var_name_def))) } @@ -1182,7 +1176,6 @@ fn parse_primary( let var_name_def = Ident { name: state.get_interned_string(id2), pos: pos2, - public: false, }; Expr::Variable(Box::new((index, namespace, var_name_def))) } @@ -1853,8 +1846,7 @@ fn parse_custom_syntax( let name = state.get_interned_string(s); segments.push(name.clone()); tokens.push(state.get_interned_string(MARKER_IDENT)); - let public = false; - let var_name_def = Ident { name, pos, public }; + let var_name_def = Ident { name, pos }; keywords.push(Expr::Variable(Box::new((None, None, var_name_def)))); } (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { @@ -2014,9 +2006,19 @@ fn parse_if( Stmt::Noop(Position::NONE) }; + let else_body = match else_body { + Stmt::If(_, _, pos) => { + let mut statements: StaticVec<_> = Default::default(); + statements.push(else_body); + StmtBlock { statements, pos } + } + Stmt::Block(_, _) | Stmt::Noop(_) => else_body.into(), + _ => unreachable!("should either be if or a block, not {:?}", else_body), + }; + Ok(Stmt::If( guard, - Box::new((if_body, else_body)), + Box::new((if_body.into(), else_body)), settings.pos, )) } @@ -2045,9 +2047,9 @@ fn parse_while_loop( ensure_not_assignment(input)?; settings.is_breakable = true; - let body = Box::new(parse_block(input, state, lib, settings.level_up())?); + let body = parse_block(input, state, lib, settings.level_up())?; - Ok(Stmt::While(guard, body, settings.pos)) + Ok(Stmt::While(guard, Box::new(body.into()), settings.pos)) } /// Parse a do loop. @@ -2065,7 +2067,7 @@ fn parse_do( // do { body } [while|until] guard settings.is_breakable = true; - let body = Box::new(parse_block(input, state, lib, settings.level_up())?); + let body = parse_block(input, state, lib, settings.level_up())?; let is_while = match input.next().unwrap() { (Token::While, _) => true, @@ -2083,7 +2085,12 @@ fn parse_do( let guard = parse_expr(input, state, lib, settings.level_up())?; ensure_not_assignment(input)?; - Ok(Stmt::Do(body, guard, is_while, settings.pos)) + Ok(Stmt::Do( + Box::new(body.into()), + guard, + is_while, + settings.pos, + )) } /// Parse a for loop. @@ -2138,7 +2145,7 @@ fn parse_for( state.stack.truncate(prev_stack_len); - Ok(Stmt::For(expr, Box::new((name, body)), settings.pos)) + Ok(Stmt::For(expr, Box::new((name, body.into())), settings.pos)) } /// Parse a variable definition statement. @@ -2170,7 +2177,6 @@ fn parse_let( let var_def = Ident { name: name.clone(), pos, - public: export, }; // let name = ... @@ -2185,9 +2191,9 @@ fn parse_let( match var_type { // let name = expr - AccessMode::ReadWrite => Ok(Stmt::Let(expr, var_def, settings.pos)), + AccessMode::ReadWrite => Ok(Stmt::Let(expr, var_def, export, settings.pos)), // const name = { expr:constant } - AccessMode::ReadOnly => Ok(Stmt::Const(expr, var_def, settings.pos)), + AccessMode::ReadOnly => Ok(Stmt::Const(expr, var_def, export, settings.pos)), } } @@ -2210,15 +2216,7 @@ fn parse_import( // import expr as ... if !match_token(input, Token::As).0 { - return Ok(Stmt::Import( - expr, - Ident { - name: "".into(), - pos: Position::NONE, - public: false, - }, - settings.pos, - )); + return Ok(Stmt::Import(expr, None, settings.pos)); } // import expr as name ... @@ -2236,11 +2234,10 @@ fn parse_import( Ok(Stmt::Import( expr, - Ident { + Some(Ident { name, pos: name_pos, - public: true, - }, + }), settings.pos, )) } @@ -2291,7 +2288,6 @@ fn parse_export( (Token::Identifier(s), pos) => Some(Ident { name: state.get_interned_string(s), pos, - public: false, }), (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { return Err(PERR::Reserved(s).into_err(pos)); @@ -2307,7 +2303,6 @@ fn parse_export( Ident { name: state.get_interned_string(id), pos: id_pos, - public: false, }, rename, )); @@ -2646,7 +2641,6 @@ fn parse_try_catch( (Token::Identifier(s), pos) => Ident { name: state.get_interned_string(s), pos, - public: false, }, (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; @@ -2670,7 +2664,7 @@ fn parse_try_catch( let catch_body = parse_block(input, state, lib, settings.level_up())?; Ok(Stmt::TryCatch( - Box::new((body, var_def, catch_body)), + Box::new((body.into(), var_def, catch_body.into())), settings.pos, catch_pos, )) @@ -2879,7 +2873,6 @@ fn parse_anon_fn( .map(|(name, &pos)| Ident { name: name.clone(), pos, - public: false, }) .collect() } diff --git a/tests/optimizer.rs b/tests/optimizer.rs index c5b91b42..29c1c991 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -54,9 +54,8 @@ fn test_optimizer_parse() -> Result<(), Box> { engine.set_optimization_level(OptimizationLevel::Simple); let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - println!("{:?}", ast); - assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, statements: [Block([Const(BoolConstant(false, 1:20), Ident("DECISION" @ 1:9), 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)], functions: Module("#)); + assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, statements: [Block([Const(BoolConstant(false, 1:20), Ident("DECISION" @ 1:9), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)], functions: Module("#)); let ast = engine.compile("if 1 == 2 { 42 }")?; From cbad703b00cbe573f0a71e0acce34f6ae9eb713a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 10 Mar 2021 13:32:09 +0800 Subject: [PATCH 12/19] Flatten data structures and more aggressive inlining. --- src/ast.rs | 91 ++++++++++++++++++++++++++++++++-------------- src/engine_api.rs | 2 +- src/lib.rs | 2 +- src/module/mod.rs | 19 ++++++++++ src/parse_error.rs | 3 +- src/parser.rs | 4 +- src/utils.rs | 7 ++++ tests/optimizer.rs | 8 ++-- 8 files changed, 98 insertions(+), 38 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index caaecf11..64fe39e8 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -11,7 +11,6 @@ use crate::stdlib::{ num::NonZeroUsize, ops::{Add, AddAssign}, string::String, - vec, vec::Vec, }; use crate::token::Token; @@ -61,9 +60,9 @@ pub struct ScriptFnDef { pub params: StaticVec, /// Access to external variables. #[cfg(not(feature = "no_closure"))] - pub externals: Vec, + pub externals: StaticVec, /// Function doc-comments (if any). - pub comments: Vec, + pub comments: StaticVec, } impl fmt::Display for ScriptFnDef { @@ -149,7 +148,7 @@ pub struct AST { /// Source of the [`AST`]. source: Option, /// Global statements. - statements: Vec, + body: StmtBlock, /// Script-defined functions. functions: Shared, /// Embedded module resolver, if any. @@ -162,7 +161,7 @@ impl Default for AST { fn default() -> Self { Self { source: None, - statements: Vec::with_capacity(16), + body: Default::default(), functions: Default::default(), #[cfg(not(feature = "no_module"))] resolver: None, @@ -179,7 +178,10 @@ impl AST { ) -> Self { Self { source: None, - statements: statements.into_iter().collect(), + body: StmtBlock { + statements: statements.into_iter().collect(), + pos: Position::NONE, + }, functions: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, @@ -194,7 +196,10 @@ impl AST { ) -> Self { Self { source: Some(source.into()), - statements: statements.into_iter().collect(), + body: StmtBlock { + statements: statements.into_iter().collect(), + pos: Position::NONE, + }, functions: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, @@ -231,7 +236,7 @@ impl AST { #[cfg(not(feature = "internals"))] #[inline(always)] pub(crate) fn statements(&self) -> &[Stmt] { - &self.statements + &self.body.statements } /// _(INTERNALS)_ Get the statements. /// Exported under the `internals` feature only. @@ -244,8 +249,8 @@ impl AST { /// Get a mutable reference to the statements. #[cfg(not(feature = "no_optimize"))] #[inline(always)] - pub(crate) fn statements_mut(&mut self) -> &mut Vec { - &mut self.statements + pub(crate) fn statements_mut(&mut self) -> &mut StaticVec { + &mut self.body.statements } /// Get the internal shared [`Module`] containing all script-defined functions. #[cfg(not(feature = "internals"))] @@ -333,7 +338,7 @@ impl AST { functions.merge_filtered(&self.functions, &filter); Self { source: self.source.clone(), - statements: Default::default(), + body: Default::default(), functions: functions.into(), #[cfg(not(feature = "no_module"))] resolver: self.resolver.clone(), @@ -345,7 +350,7 @@ impl AST { pub fn clone_statements_only(&self) -> Self { Self { source: self.source.clone(), - statements: self.statements.clone(), + body: self.body.clone(), functions: Default::default(), #[cfg(not(feature = "no_module"))] resolver: self.resolver.clone(), @@ -515,20 +520,19 @@ impl AST { filter: impl Fn(FnNamespace, FnAccess, bool, &str, usize) -> bool, ) -> Self { let Self { - statements, - functions, - .. + body, functions, .. } = self; - let ast = match (statements.is_empty(), other.statements.is_empty()) { + let merged = match (body.is_empty(), other.body.is_empty()) { (false, false) => { - let mut statements = statements.clone(); - statements.extend(other.statements.iter().cloned()); - statements + let mut body = body.clone(); + body.statements + .extend(other.body.statements.iter().cloned()); + body } - (false, true) => statements.clone(), - (true, false) => other.statements.clone(), - (true, true) => vec![], + (false, true) => body.clone(), + (true, false) => other.body.clone(), + (true, true) => Default::default(), }; let source = other.source.clone().or_else(|| self.source.clone()); @@ -537,9 +541,9 @@ impl AST { functions.merge_filtered(&other.functions, &filter); if let Some(source) = source { - Self::new_with_source(ast, functions, source) + Self::new_with_source(merged.statements, functions, source) } else { - Self::new(ast, functions) + Self::new(merged.statements, functions) } } /// Combine one [`AST`] with another. The second [`AST`] is consumed. @@ -599,7 +603,10 @@ impl AST { other: Self, filter: impl Fn(FnNamespace, FnAccess, bool, &str, usize) -> bool, ) -> &mut Self { - self.statements.extend(other.statements.into_iter()); + self.body + .statements + .extend(other.body.statements.into_iter()); + if !other.functions.is_empty() { shared_make_mut(&mut self.functions).merge_filtered(&other.functions, &filter); } @@ -673,7 +680,7 @@ impl AST { /// Clear all statements in the [`AST`], leaving only function definitions. #[inline(always)] pub fn clear_statements(&mut self) { - self.statements = vec![]; + self.body = Default::default(); } /// Recursively walk the [`AST`], including function bodies (if any). #[cfg(not(feature = "internals"))] @@ -810,7 +817,7 @@ impl<'a> From<&'a Expr> for ASTNode<'a> { /// # Volatile Data Structure /// /// This type is volatile and may change. -#[derive(Debug, Clone, Hash)] +#[derive(Clone, Hash, Default)] pub struct StmtBlock { pub statements: StaticVec, pub pos: Position, @@ -818,15 +825,27 @@ pub struct StmtBlock { impl StmtBlock { /// Is this statements block empty? + #[inline(always)] pub fn is_empty(&self) -> bool { self.statements.is_empty() } /// Number of statements in this statements block. + #[inline(always)] pub fn len(&self) -> usize { self.statements.len() } } +impl fmt::Debug for StmtBlock { + #[inline(always)] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if !self.pos.is_none() { + write!(f, "{} @ ", self.pos)?; + } + fmt::Debug::fmt(&self.statements, f) + } +} + /// _(INTERNALS)_ A statement. /// Exported under the `internals` feature only. /// @@ -1242,6 +1261,7 @@ pub struct FloatWrapper(FLOAT); #[cfg(not(feature = "no_float"))] impl Hash for FloatWrapper { + #[inline(always)] fn hash(&self, state: &mut H) { self.0.to_ne_bytes().hash(state); } @@ -1249,6 +1269,7 @@ impl Hash for FloatWrapper { #[cfg(not(feature = "no_float"))] impl AsRef for FloatWrapper { + #[inline(always)] fn as_ref(&self) -> &FLOAT { &self.0 } @@ -1256,6 +1277,7 @@ impl AsRef for FloatWrapper { #[cfg(not(feature = "no_float"))] impl AsMut for FloatWrapper { + #[inline(always)] fn as_mut(&mut self) -> &mut FLOAT { &mut self.0 } @@ -1265,6 +1287,7 @@ impl AsMut for FloatWrapper { impl crate::stdlib::ops::Deref for FloatWrapper { type Target = FLOAT; + #[inline(always)] fn deref(&self) -> &Self::Target { &self.0 } @@ -1272,6 +1295,7 @@ impl crate::stdlib::ops::Deref for FloatWrapper { #[cfg(not(feature = "no_float"))] impl crate::stdlib::ops::DerefMut for FloatWrapper { + #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } @@ -1279,6 +1303,7 @@ impl crate::stdlib::ops::DerefMut for FloatWrapper { #[cfg(not(feature = "no_float"))] impl fmt::Debug for FloatWrapper { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(self, f) } @@ -1286,6 +1311,7 @@ impl fmt::Debug for FloatWrapper { #[cfg(not(feature = "no_float"))] impl fmt::Display for FloatWrapper { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { #[cfg(feature = "no_std")] use num_traits::Float; @@ -1301,6 +1327,7 @@ impl fmt::Display for FloatWrapper { #[cfg(not(feature = "no_float"))] impl From for FloatWrapper { + #[inline(always)] fn from(value: FLOAT) -> Self { Self::new(value) } @@ -1310,6 +1337,7 @@ impl From for FloatWrapper { impl FromStr for FloatWrapper { type Err = ::Err; + #[inline(always)] fn from_str(s: &str) -> Result { FLOAT::from_str(s).map(Into::::into) } @@ -1317,6 +1345,7 @@ impl FromStr for FloatWrapper { #[cfg(not(feature = "no_float"))] impl FloatWrapper { + #[inline(always)] pub const fn new(value: FLOAT) -> Self { Self(value) } @@ -1384,6 +1413,7 @@ impl Expr { /// Get the [`Dynamic`] value of a constant expression. /// /// Returns [`None`] if the expression is not constant. + #[inline] pub fn get_constant_value(&self) -> Option { Some(match self { Self::DynamicConstant(x, _) => x.as_ref().clone(), @@ -1434,6 +1464,7 @@ impl Expr { } } /// Get the [position][Position] of the expression. + #[inline] pub fn position(&self) -> Position { match self { #[cfg(not(feature = "no_float"))] @@ -1462,6 +1493,7 @@ impl Expr { } } /// Override the [position][Position] of the expression. + #[inline] pub fn set_position(&mut self, new_pos: Position) -> &mut Self { match self { #[cfg(not(feature = "no_float"))] @@ -1490,6 +1522,7 @@ impl Expr { /// Is the expression pure? /// /// A pure expression has no side effects. + #[inline] pub fn is_pure(&self) -> bool { match self { Self::Array(x, _) => x.iter().all(Self::is_pure), @@ -1516,6 +1549,7 @@ impl Expr { } } /// Is the expression a constant? + #[inline] pub fn is_constant(&self) -> bool { match self { #[cfg(not(feature = "no_float"))] @@ -1539,6 +1573,7 @@ impl Expr { } } /// Is a particular [token][Token] allowed as a postfix operator to this expression? + #[inline] pub fn is_valid_postfix(&self, token: &Token) -> bool { match token { #[cfg(not(feature = "no_object"))] @@ -1591,7 +1626,7 @@ impl Expr { } } /// Recursively walk this expression. - #[inline(always)] + #[inline] pub fn walk<'a>(&'a self, path: &mut Vec>, on_node: &mut impl FnMut(&[ASTNode])) { path.push(self.into()); on_node(path); diff --git a/src/engine_api.rs b/src/engine_api.rs index 9ac38a97..c2f0bb28 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1832,7 +1832,7 @@ impl Engine { let lib = Default::default(); let stmt = crate::stdlib::mem::take(ast.statements_mut()); - crate::optimize::optimize_into_ast(self, scope, stmt, lib, optimization_level) + crate::optimize::optimize_into_ast(self, scope, stmt.into_vec(), lib, optimization_level) } /// Generate a list of all registered functions. /// diff --git a/src/lib.rs b/src/lib.rs index d36ec5b9..503890d5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -192,7 +192,7 @@ pub use token::{get_next_token, parse_string_literal, InputStream, Token, Tokeni #[deprecated = "this type is volatile and may change"] pub use ast::{ ASTNode, BinaryExpr, CustomExpr, Expr, FloatWrapper, FnCallExpr, FnHash, Ident, OpAssignment, - ReturnType, ScriptFnDef, Stmt, + ReturnType, ScriptFnDef, Stmt, StmtBlock, }; #[cfg(feature = "internals")] diff --git a/src/module/mod.rs b/src/module/mod.rs index 0727c8fb..2a0c2edc 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -39,6 +39,7 @@ pub enum FnNamespace { } impl Default for FnNamespace { + #[inline(always)] fn default() -> Self { Self::Internal } @@ -150,6 +151,7 @@ pub struct Module { } impl Default for Module { + #[inline(always)] fn default() -> Self { Self { id: None, @@ -226,6 +228,7 @@ impl AsRef for Module { impl> Add for &Module { type Output = Module; + #[inline(always)] fn add(self, rhs: M) -> Self::Output { let mut module = self.clone(); module.merge(rhs.as_ref()); @@ -236,6 +239,7 @@ impl> Add for &Module { impl> Add for Module { type Output = Self; + #[inline(always)] fn add(mut self, rhs: M) -> Self::Output { self.merge(rhs.as_ref()); self @@ -243,6 +247,7 @@ impl> Add for Module { } impl> AddAssign for Module { + #[inline(always)] fn add_assign(&mut self, rhs: M) { self.combine(rhs.into()); } @@ -1953,16 +1958,19 @@ impl Module { } /// Does a type iterator exist in the entire module tree? + #[inline(always)] pub fn contains_qualified_iter(&self, id: TypeId) -> bool { self.all_type_iterators.contains_key(&id) } /// Does a type iterator exist in the module? + #[inline(always)] pub fn contains_iter(&self, id: TypeId) -> bool { self.type_iterators.contains_key(&id) } /// Set a type iterator into the [`Module`]. + #[inline(always)] pub fn set_iter(&mut self, typ: TypeId, func: IteratorFn) -> &mut Self { self.type_iterators.insert(typ, func); self.indexed = false; @@ -1971,6 +1979,7 @@ impl Module { } /// Set a type iterator into the [`Module`]. + #[inline(always)] pub fn set_iterable(&mut self) -> &mut Self where T: Variant + Clone + IntoIterator, @@ -1982,6 +1991,7 @@ impl Module { } /// Set an iterator type into the [`Module`] as a type iterator. + #[inline(always)] pub fn set_iterator(&mut self) -> &mut Self where T: Variant + Clone + Iterator, @@ -1993,11 +2003,13 @@ impl Module { } /// Get the specified type iterator. + #[inline(always)] pub(crate) fn get_qualified_iter(&self, id: TypeId) -> Option { self.all_type_iterators.get(&id).cloned() } /// Get the specified type iterator. + #[inline(always)] pub(crate) fn get_iter(&self, id: TypeId) -> Option { self.type_iterators.get(&id).cloned() } @@ -2021,6 +2033,7 @@ pub struct NamespaceRef { } impl fmt::Debug for NamespaceRef { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(index) = self.index { write!(f, "{} -> ", index)?; @@ -2040,18 +2053,21 @@ impl fmt::Debug for NamespaceRef { impl Deref for NamespaceRef { type Target = StaticVec; + #[inline(always)] fn deref(&self) -> &Self::Target { &self.path } } impl DerefMut for NamespaceRef { + #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { &mut self.path } } impl fmt::Display for NamespaceRef { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { for Ident { name, .. } in self.path.iter() { write!(f, "{}{}", name, Token::DoubleColon.syntax())?; @@ -2061,6 +2077,7 @@ impl fmt::Display for NamespaceRef { } impl From> for NamespaceRef { + #[inline(always)] fn from(path: StaticVec) -> Self { Self { index: None, path } } @@ -2068,11 +2085,13 @@ impl From> for NamespaceRef { impl NamespaceRef { /// Get the [`Scope`][crate::Scope] index offset. + #[inline(always)] pub(crate) fn index(&self) -> Option { self.index } /// Set the [`Scope`][crate::Scope] index offset. #[cfg(not(feature = "no_module"))] + #[inline(always)] pub(crate) fn set_index(&mut self, index: Option) { self.index = index } diff --git a/src/parse_error.rs b/src/parse_error.rs index a965af0b..b31608b6 100644 --- a/src/parse_error.rs +++ b/src/parse_error.rs @@ -38,6 +38,7 @@ pub enum LexError { impl Error for LexError {} impl fmt::Display for LexError { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::UnexpectedInput(s) => write!(f, "Unexpected '{}'", s), @@ -293,7 +294,7 @@ pub struct ParseError(pub Box, pub Position); impl Error for ParseError {} impl fmt::Display for ParseError { - #[inline] + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.0, f)?; diff --git a/src/parser.rs b/src/parser.rs index 56460f8b..1c19ab8c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2434,7 +2434,7 @@ fn parse_stmt( ) -> Result { use AccessMode::{ReadOnly, ReadWrite}; - let mut _comments: Vec = Default::default(); + let mut _comments: StaticVec = Default::default(); #[cfg(not(feature = "no_function"))] { @@ -2678,7 +2678,7 @@ fn parse_fn( lib: &mut FunctionsLib, access: FnAccess, mut settings: ParseSettings, - comments: Vec, + comments: StaticVec, ) -> Result { #[cfg(not(feature = "unchecked"))] settings.ensure_level_within_max_limit(state.max_expr_depth)?; diff --git a/src/utils.rs b/src/utils.rs index a51501e9..dd768752 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -116,16 +116,19 @@ pub(crate) fn combine_hashes(a: u64, b: u64) -> u64 { pub struct HashableHashMap(HashMap); impl From> for HashableHashMap { + #[inline(always)] fn from(value: HashMap) -> Self { Self(value) } } impl AsRef> for HashableHashMap { + #[inline(always)] fn as_ref(&self) -> &HashMap { &self.0 } } impl AsMut> for HashableHashMap { + #[inline(always)] fn as_mut(&mut self) -> &mut HashMap { &mut self.0 } @@ -133,21 +136,25 @@ impl AsMut> for HashableHashMap impl Deref for HashableHashMap { type Target = HashMap; + #[inline(always)] fn deref(&self) -> &Self::Target { &self.0 } } impl DerefMut for HashableHashMap { + #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } impl Debug for HashableHashMap { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.0.fmt(f) } } impl Hash for HashableHashMap { + #[inline(always)] fn hash(&self, state: &mut B) { let mut keys: Vec<_> = self.0.keys().collect(); keys.sort(); diff --git a/tests/optimizer.rs b/tests/optimizer.rs index 29c1c991..4c7cfb49 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -55,20 +55,18 @@ fn test_optimizer_parse() -> Result<(), Box> { let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, statements: [Block([Const(BoolConstant(false, 1:20), Ident("DECISION" @ 1:9), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)], functions: Module("#)); + assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, body: [Block([Const(BoolConstant(false, 1:20), Ident("DECISION" @ 1:9), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)], functions: Module("#)); let ast = engine.compile("if 1 == 2 { 42 }")?; - assert!( - format!("{:?}", ast).starts_with("AST { source: None, statements: [], functions: Module(") - ); + assert!(format!("{:?}", ast).starts_with("AST { source: None, body: [], functions: Module(")); engine.set_optimization_level(OptimizationLevel::Full); let ast = engine.compile("abs(-42)")?; assert!(format!("{:?}", ast) - .starts_with(r"AST { source: None, statements: [Expr(IntegerConstant(42, 1:1))]")); + .starts_with(r"AST { source: None, body: [Expr(IntegerConstant(42, 1:1))]")); Ok(()) } From 874b3fc843de28912ba009ed9e7ec072bb17807d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 10 Mar 2021 14:10:04 +0800 Subject: [PATCH 13/19] Fix metadata build. --- src/serde/metadata.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index 7f4edb25..05a709a9 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -151,7 +151,7 @@ impl From<&crate::module::FuncInfo> for FnMetadata { } #[cfg(not(feature = "no_function"))] { - info.func.get_fn_def().comments.clone() + info.func.get_fn_def().comments.to_vec() } } else { Default::default() From 728ed811735af1f9bbcc1f7356cc24aa6757a6d8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 10 Mar 2021 22:12:48 +0800 Subject: [PATCH 14/19] Optimize layout. --- src/ast.rs | 50 ++++++++++-------- src/engine.rs | 133 ++++++++++++++++++++++++++++-------------------- src/fn_call.rs | 39 +++++++++----- src/optimize.rs | 104 +++++++++++++++++++++++-------------- src/parser.rs | 59 ++++++++++----------- 5 files changed, 226 insertions(+), 159 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 64fe39e8..1e9f593c 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -46,7 +46,7 @@ pub enum FnAccess { #[derive(Debug, Clone)] pub struct ScriptFnDef { /// Function body. - pub body: Stmt, + pub body: StmtBlock, /// Encapsulated running environment, if any. pub lib: Option>, /// Encapsulated imported modules. @@ -692,7 +692,7 @@ impl AST { .chain({ #[cfg(not(feature = "no_function"))] { - self.iter_fn_def().map(|f| &f.body) + self.iter_fn_def().flat_map(|f| f.body.statements.iter()) } #[cfg(feature = "no_function")] { @@ -862,8 +862,8 @@ pub enum Stmt { Switch( Expr, Box<( - HashableHashMap, - Option, + HashableHashMap, + StmtBlock, )>, Position, ), @@ -914,6 +914,7 @@ impl Default for Stmt { } impl From for StmtBlock { + #[inline(always)] fn from(stmt: Stmt) -> Self { match stmt { Stmt::Block(block, pos) => Self { @@ -924,7 +925,11 @@ impl From for StmtBlock { statements: Default::default(), pos, }, - _ => panic!("cannot convert {:?} into a StmtBlock", stmt), + _ => { + let pos = stmt.position(); + let statements = vec![stmt].into(); + Self { statements, pos } + } } } } @@ -1043,8 +1048,11 @@ impl Stmt { } Self::Switch(expr, x, _) => { expr.is_pure() - && x.0.values().all(Stmt::is_pure) - && x.1.as_ref().map(Stmt::is_pure).unwrap_or(true) + && x.0 + .values() + .flat_map(|block| block.statements.iter()) + .all(Stmt::is_pure) + && x.1.statements.iter().all(Stmt::is_pure) } Self::While(condition, block, _) | Self::Do(block, condition, _, _) => { condition.is_pure() && block.statements.iter().all(Stmt::is_pure) @@ -1083,10 +1091,10 @@ impl Stmt { } Self::Switch(e, x, _) => { e.walk(path, on_node); - x.0.values().for_each(|s| s.walk(path, on_node)); - if let Some(ref s) = x.1 { - s.walk(path, on_node); - } + x.0.values() + .flat_map(|block| block.statements.iter()) + .for_each(|s| s.walk(path, on_node)); + x.1.statements.iter().for_each(|s| s.walk(path, on_node)); } Self::While(e, s, _) | Self::Do(s, e, _, _) => { e.walk(path, on_node); @@ -1384,10 +1392,10 @@ pub enum Expr { Unit(Position), /// Variable access - (optional index, optional (hash, modules), variable name) Variable(Box<(Option, Option<(u64, NamespaceRef)>, Ident)>), - /// Property access - (getter, hash, setter, hash, prop) - Property(Box<(ImmutableString, u64, ImmutableString, u64, Ident)>), + /// Property access - ((getter, hash), (setter, hash), prop) + Property(Box<((ImmutableString, u64), (ImmutableString, u64), Ident)>), /// { [statement][Stmt] ... } - Stmt(Box>, Position), + Stmt(Box), /// func `(` expr `,` ... `)` FnCall(Box, Position), /// lhs `.` rhs @@ -1478,8 +1486,8 @@ impl Expr { Self::FnPointer(_, pos) => *pos, Self::Array(_, pos) => *pos, Self::Map(_, pos) => *pos, - Self::Property(x) => (x.4).pos, - Self::Stmt(_, pos) => *pos, + Self::Property(x) => (x.2).pos, + Self::Stmt(x) => x.pos, Self::Variable(x) => (x.2).pos, Self::FnCall(_, pos) => *pos, @@ -1508,8 +1516,8 @@ impl Expr { Self::Array(_, pos) => *pos = new_pos, Self::Map(_, pos) => *pos = new_pos, Self::Variable(x) => (x.2).pos = new_pos, - Self::Property(x) => (x.4).pos = new_pos, - Self::Stmt(_, pos) => *pos = new_pos, + Self::Property(x) => (x.2).pos = new_pos, + Self::Stmt(x) => x.pos = new_pos, Self::FnCall(_, pos) => *pos = new_pos, Self::And(_, pos) | Self::Or(_, pos) => *pos = new_pos, Self::Unit(pos) => *pos = new_pos, @@ -1533,7 +1541,7 @@ impl Expr { x.lhs.is_pure() && x.rhs.is_pure() } - Self::Stmt(x, _) => x.iter().all(Stmt::is_pure), + Self::Stmt(x) => x.statements.iter().all(Stmt::is_pure), Self::Variable(_) => true, @@ -1596,7 +1604,7 @@ impl Expr { Self::StringConstant(_, _) | Self::FnCall(_, _) - | Self::Stmt(_, _) + | Self::Stmt(_) | Self::Dot(_, _) | Self::Index(_, _) | Self::Array(_, _) @@ -1632,7 +1640,7 @@ impl Expr { on_node(path); match self { - Self::Stmt(x, _) => x.iter().for_each(|s| s.walk(path, on_node)), + Self::Stmt(x) => x.statements.iter().for_each(|s| s.walk(path, on_node)), Self::Array(x, _) => x.iter().for_each(|e| e.walk(path, on_node)), Self::Map(x, _) => x.iter().for_each(|(_, e)| e.walk(path, on_node)), Self::Index(x, _) | Self::Dot(x, _) | Expr::And(x, _) | Expr::Or(x, _) => { diff --git a/src/engine.rs b/src/engine.rs index 297570ff..973cf56b 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1181,7 +1181,7 @@ impl Engine { } // {xxx:map}.id op= ??? Expr::Property(x) if target_val.is::() && new_val.is_some() => { - let Ident { name, pos, .. } = &x.4; + let Ident { name, pos, .. } = &x.2; let index = name.clone().into(); let val = self.get_indexed_mut( mods, state, lib, target_val, index, *pos, true, is_ref, false, level, @@ -1194,7 +1194,7 @@ impl Engine { } // {xxx:map}.id Expr::Property(x) if target_val.is::() => { - let Ident { name, pos, .. } = &x.4; + let Ident { name, pos, .. } = &x.2; let index = name.clone().into(); let val = self.get_indexed_mut( mods, state, lib, target_val, index, *pos, false, is_ref, false, level, @@ -1204,7 +1204,7 @@ impl Engine { } // xxx.id = ??? Expr::Property(x) if new_val.is_some() => { - let (_, _, setter, hash_set, Ident { pos, .. }) = x.as_ref(); + let (_, (setter, hash_set), Ident { pos, .. }) = x.as_ref(); let hash = FnHash::from_native(*hash_set); let mut new_val = new_val; let mut args = [target_val, &mut (new_val.as_mut().unwrap().0).0]; @@ -1216,7 +1216,7 @@ impl Engine { } // xxx.id Expr::Property(x) => { - let (getter, hash_get, _, _, Ident { pos, .. }) = x.as_ref(); + let ((getter, hash_get), _, Ident { pos, .. }) = x.as_ref(); let hash = FnHash::from_native(*hash_get); let mut args = [target_val]; self.exec_fn_call( @@ -1229,7 +1229,7 @@ impl Engine { Expr::Index(x, x_pos) | Expr::Dot(x, x_pos) if target_val.is::() => { let mut val = match &x.lhs { Expr::Property(p) => { - let Ident { name, pos, .. } = &p.4; + let Ident { name, pos, .. } = &p.2; let index = name.clone().into(); self.get_indexed_mut( mods, state, lib, target_val, index, *pos, false, is_ref, true, @@ -1264,7 +1264,7 @@ impl Engine { match &x.lhs { // xxx.prop[expr] | xxx.prop.expr Expr::Property(p) => { - let (getter, hash_get, setter, hash_set, Ident { pos, .. }) = + let ((getter, hash_get), (setter, hash_set), Ident { pos, .. }) = p.as_ref(); let hash_get = FnHash::from_native(*hash_get); let hash_set = FnHash::from_native(*hash_set); @@ -1456,7 +1456,7 @@ impl Engine { } Expr::Property(x) if parent_chain_type == ChainType::Dot => { - idx_values.push(ChainArgument::Property(x.4.pos)) + idx_values.push(ChainArgument::Property(x.2.pos)) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), @@ -1466,7 +1466,7 @@ impl Engine { // Evaluate in left-to-right order let lhs_val = match lhs { Expr::Property(x) if parent_chain_type == ChainType::Dot => { - ChainArgument::Property(x.4.pos) + ChainArgument::Property(x.2.pos) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), Expr::FnCall(x, _) @@ -1655,16 +1655,11 @@ impl Engine { .map(|(val, _)| val.take_or_clone()), // Statement block - Expr::Stmt(x, _) => self.eval_stmt_block( - scope, - mods, - state, - lib, - this_ptr, - x.as_ref().as_ref(), - true, - level, - ), + Expr::Stmt(x) if x.is_empty() => Ok(Dynamic::UNIT), + Expr::Stmt(x) => { + let statements = &x.statements; + self.eval_stmt_block(scope, mods, state, lib, this_ptr, statements, true, level) + } // lhs[idx_expr] #[cfg(not(feature = "no_index"))] @@ -1804,6 +1799,10 @@ impl Engine { restore_prev_state: bool, level: usize, ) -> RhaiResult { + if statements.is_empty() { + return Ok(Dynamic::UNIT); + } + let mut _extra_fn_resolution_cache = false; let prev_always_search = state.always_search; let prev_scope_len = scope.len(); @@ -2025,6 +2024,7 @@ impl Engine { } // Block scope + Stmt::Block(statements, _) if statements.is_empty() => Ok(Dynamic::UNIT), Stmt::Block(statements, _) => { self.eval_stmt_block(scope, mods, state, lib, this_ptr, statements, true, level) } @@ -2046,15 +2046,21 @@ impl Engine { .map_err(|err| self.make_type_mismatch_err::(err, expr.position())) .and_then(|guard_val| { if guard_val { - self.eval_stmt_block( - scope, mods, state, lib, this_ptr, if_stmt, true, level, - ) - } else if !else_stmt.is_empty() { - self.eval_stmt_block( - scope, mods, state, lib, this_ptr, else_stmt, true, level, - ) + if !if_stmt.is_empty() { + self.eval_stmt_block( + scope, mods, state, lib, this_ptr, if_stmt, true, level, + ) + } else { + Ok(Dynamic::UNIT) + } } else { - Ok(Dynamic::UNIT) + if !else_stmt.is_empty() { + self.eval_stmt_block( + scope, mods, state, lib, this_ptr, else_stmt, true, level, + ) + } else { + Ok(Dynamic::UNIT) + } } }) } @@ -2070,21 +2076,29 @@ impl Engine { value.hash(hasher); let hash = hasher.finish(); - table - .get(&hash) - .map(|stmt| self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level)) + table.get(&hash).map(|StmtBlock { statements, .. }| { + if !statements.is_empty() { + self.eval_stmt_block( + scope, mods, state, lib, this_ptr, statements, true, level, + ) + } else { + Ok(Dynamic::UNIT) + } + }) } else { // Non-hashable values never match any specific clause None } .unwrap_or_else(|| { // Default match clause - def_stmt.as_ref().map_or_else( - || Ok(Dynamic::UNIT), - |def_stmt| { - self.eval_stmt(scope, mods, state, lib, this_ptr, def_stmt, level) - }, - ) + let def_stmt = &def_stmt.statements; + if !def_stmt.is_empty() { + self.eval_stmt_block( + scope, mods, state, lib, this_ptr, def_stmt, true, level, + ) + } else { + Ok(Dynamic::UNIT) + } }) } @@ -2102,20 +2116,22 @@ impl Engine { true }; - if condition { - match self - .eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level) - { - Ok(_) => (), - Err(err) => match *err { - EvalAltResult::LoopBreak(false, _) => (), - EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), - _ => return Err(err), - }, - } - } else { + if !condition { return Ok(Dynamic::UNIT); } + if body.is_empty() { + continue; + } + + match self.eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level) + { + Ok(_) => (), + Err(err) => match *err { + EvalAltResult::LoopBreak(false, _) => (), + EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), + _ => return Err(err), + }, + } } } @@ -2124,14 +2140,17 @@ impl Engine { let body = &body.statements; loop { - match self.eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level) - { - Ok(_) => (), - Err(err) => match *err { - EvalAltResult::LoopBreak(false, _) => continue, - EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), - _ => return Err(err), - }, + if !body.is_empty() { + match self + .eval_stmt_block(scope, mods, state, lib, this_ptr, body, true, level) + { + Ok(_) => (), + Err(err) => match *err { + EvalAltResult::LoopBreak(false, _) => continue, + EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), + _ => return Err(err), + }, + } } if self @@ -2203,6 +2222,10 @@ impl Engine { self.inc_operations(state, *pos)?; + if statements.is_empty() { + continue; + } + match self.eval_stmt_block( scope, mods, state, lib, this_ptr, statements, true, level, ) { diff --git a/src/fn_call.rs b/src/fn_call.rs index 8a959115..8a008ee1 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -493,6 +493,10 @@ impl Engine { self.inc_operations(state, pos)?; + if fn_def.body.is_empty() { + return Ok(Dynamic::UNIT); + } + // Check for stack overflow #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "unchecked"))] @@ -539,10 +543,10 @@ impl Engine { } // Evaluate the function - let stmt = &fn_def.body; + let body = &fn_def.body.statements; let result = self - .eval_stmt(scope, mods, state, unified_lib, this_ptr, stmt, level) + .eval_stmt_block(scope, mods, state, unified_lib, this_ptr, body, true, level) .or_else(|err| match *err { // Convert return statement to return value EvalAltResult::Return(x, _) => Ok(x), @@ -722,6 +726,10 @@ impl Engine { let func = func.get_fn_def(); + if func.body.is_empty() { + return Ok((Dynamic::UNIT, false)); + } + let scope: &mut Scope = &mut Default::default(); // Move captured variables into scope @@ -1391,22 +1399,27 @@ impl Engine { match func { #[cfg(not(feature = "no_function"))] Some(f) if f.is_script() => { - let args = args.as_mut(); - let new_scope = &mut Default::default(); - let fn_def = f.get_fn_def().clone(); + let fn_def = f.get_fn_def(); - let mut source = module.id_raw().cloned(); - mem::swap(&mut state.source, &mut source); + if fn_def.body.is_empty() { + Ok(Dynamic::UNIT) + } else { + let args = args.as_mut(); + let new_scope = &mut Default::default(); - let level = level + 1; + let mut source = module.id_raw().cloned(); + mem::swap(&mut state.source, &mut source); - let result = self.call_script_fn( - new_scope, mods, state, lib, &mut None, &fn_def, args, pos, level, - ); + let level = level + 1; - state.source = source; + let result = self.call_script_fn( + new_scope, mods, state, lib, &mut None, fn_def, args, pos, level, + ); - result + state.source = source; + + result + } } Some(f) if f.is_plugin_fn() => f diff --git a/src/optimize.rs b/src/optimize.rs index b70f72a1..07dc1d48 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -1,6 +1,6 @@ //! Module implementing the [`AST`] optimizer. -use crate::ast::{Expr, Ident, Stmt}; +use crate::ast::{Expr, Ident, Stmt, StmtBlock}; use crate::dynamic::AccessMode; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::fn_builtin::get_builtin_binary_op_fn; @@ -362,29 +362,54 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { let table = &mut x.0; - if let Some(stmt) = table.get_mut(&hash) { - optimize_stmt(stmt, state, true); - *expr = Expr::Stmt(Box::new(vec![mem::take(stmt)].into()), *pos); - } else if let Some(def_stmt) = x.1.as_mut() { - optimize_stmt(def_stmt, state, true); - *expr = Expr::Stmt(Box::new(vec![mem::take(def_stmt)].into()), *pos); + let (statements, new_pos) = if let Some(block) = table.get_mut(&hash) { + ( + optimize_stmt_block( + mem::take(&mut block.statements).into_vec(), + block.pos, + state, + true, + ) + .into(), + block.pos, + ) } else { - *expr = Expr::Unit(*pos); - } + ( + optimize_stmt_block( + mem::take(&mut x.1.statements).into_vec(), + x.1.pos, + state, + true, + ) + .into(), + if x.1.pos.is_none() { *pos } else { x.1.pos }, + ) + }; + + *expr = Expr::Stmt(Box::new(StmtBlock { + statements, + pos: new_pos, + })); } // switch Stmt::Switch(expr, x, _) => { optimize_expr(expr, state); - x.0.values_mut() - .for_each(|stmt| optimize_stmt(stmt, state, preserve_result)); - if let Some(def_stmt) = x.1.as_mut() { - optimize_stmt(def_stmt, state, preserve_result); - - match def_stmt { - Stmt::Noop(_) | Stmt::Expr(Expr::Unit(_)) => x.1 = None, - _ => (), - } - } + x.0.values_mut().for_each(|block| { + block.statements = optimize_stmt_block( + mem::take(&mut block.statements).into_vec(), + block.pos, + state, + preserve_result, + ) + .into() + }); + x.1.statements = optimize_stmt_block( + mem::take(&mut x.1.statements).into_vec(), + x.1.pos, + state, + preserve_result, + ) + .into() } // while false { block } -> Noop @@ -512,14 +537,14 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { .into(); } // {} - Stmt::Expr(Expr::Stmt(x, pos)) if x.is_empty() => { + Stmt::Expr(Expr::Stmt(x)) if x.statements.is_empty() => { state.set_dirty(); - *stmt = Stmt::Noop(*pos); + *stmt = Stmt::Noop(x.pos); } // {...}; - Stmt::Expr(Expr::Stmt(x, pos)) => { + Stmt::Expr(Expr::Stmt(x)) => { state.set_dirty(); - *stmt = Stmt::Block(mem::take(x).into_vec(), *pos); + *stmt = Stmt::Block(mem::take(&mut x.statements).into_vec(), x.pos); } // expr; Stmt::Expr(expr) => optimize_expr(expr, state), @@ -542,18 +567,15 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { match expr { // {} - Expr::Stmt(x, pos) if x.is_empty() => { state.set_dirty(); *expr = Expr::Unit(*pos) } + Expr::Stmt(x) if x.statements.is_empty() => { state.set_dirty(); *expr = Expr::Unit(x.pos) } // { stmt; ... } - do not count promotion as dirty because it gets turned back into an array - Expr::Stmt(x, pos) => { - let statements = optimize_stmt_block(mem::take(x).into_vec(), *pos, state, true); - *expr = Expr::Stmt(Box::new(statements.into()), *pos); - } + Expr::Stmt(x) => x.statements = optimize_stmt_block(mem::take(&mut x.statements).into_vec(), x.pos, state, true).into(), // lhs.rhs #[cfg(not(feature = "no_object"))] Expr::Dot(x, _) => match (&mut x.lhs, &mut x.rhs) { // map.string (Expr::Map(m, pos), Expr::Property(p)) if m.iter().all(|(_, x)| x.is_pure()) => { - let prop = &p.4.name; + let prop = &p.2.name; // Map literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); @@ -902,30 +924,34 @@ pub fn optimize_into_ast( _functions .into_iter() .map(|mut fn_def| { - let pos = fn_def.body.position(); + let pos = fn_def.body.pos; // Optimize the function body let mut body = optimize_top_level( - vec![fn_def.body], + fn_def.body.statements.into_vec(), engine, &Scope::new(), &[&lib2], level, ); - // {} -> Noop - fn_def.body = match body.pop().unwrap_or_else(|| Stmt::Noop(pos)) { + match &mut body[..] { // { return val; } -> val - Stmt::Return(crate::ast::ReturnType::Return, Some(expr), _) => { - Stmt::Expr(expr) + [Stmt::Return(crate::ast::ReturnType::Return, Some(expr), _)] => { + body[0] = Stmt::Expr(mem::take(expr)) } // { return; } -> () - Stmt::Return(crate::ast::ReturnType::Return, None, pos) => { - Stmt::Expr(Expr::Unit(pos)) + [Stmt::Return(crate::ast::ReturnType::Return, None, _)] => { + body.clear(); } - // All others - stmt => stmt, + _ => (), + } + + fn_def.body = StmtBlock { + statements: body.into(), + pos, }; + fn_def }) .for_each(|fn_def| { diff --git a/src/parser.rs b/src/parser.rs index 1c19ab8c..db15c06a 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -243,7 +243,11 @@ impl Expr { let setter = state.get_interned_string(crate::engine::make_setter(&ident.name)); let hash_set = calc_fn_hash(empty(), &setter, 2); - Self::Property(Box::new((getter, hash_get, setter, hash_set, ident.into()))) + Self::Property(Box::new(( + (getter, hash_get), + (setter, hash_set), + ident.into(), + ))) } _ => self, } @@ -809,7 +813,7 @@ fn parse_switch( } } - let mut table = HashMap::new(); + let mut table = HashMap::::new(); let mut def_stmt = None; loop { @@ -869,10 +873,10 @@ fn parse_switch( let need_comma = !stmt.is_self_terminated(); def_stmt = if let Some(hash) = hash { - table.insert(hash, stmt); + table.insert(hash, stmt.into()); None } else { - Some(stmt) + Some(stmt.into()) }; match input.peek().unwrap() { @@ -903,7 +907,10 @@ fn parse_switch( Ok(Stmt::Switch( item, - Box::new((final_table.into(), def_stmt)), + Box::new(( + final_table.into(), + def_stmt.unwrap_or_else(|| Stmt::Noop(Position::NONE).into()), + )), settings.pos, )) } @@ -954,7 +961,7 @@ fn parse_primary( // { - block statement as expression Token::LeftBrace if settings.allow_stmt_expr => { match parse_block(input, state, lib, settings.level_up())? { - Stmt::Block(statements, pos) => Expr::Stmt(Box::new(statements.into()), pos), + block @ Stmt::Block(_, _) => Expr::Stmt(Box::new(block.into())), stmt => unreachable!("expecting Stmt::Block, but gets {:?}", stmt), } } @@ -962,15 +969,14 @@ fn parse_primary( Token::LeftParen => parse_paren_expr(input, state, lib, settings.level_up())?, // If statement is allowed to act as expressions - Token::If if settings.allow_if_expr => Expr::Stmt( - Box::new(vec![parse_if(input, state, lib, settings.level_up())?].into()), - settings.pos, - ), + Token::If if settings.allow_if_expr => Expr::Stmt(Box::new( + parse_if(input, state, lib, settings.level_up())?.into(), + )), // Switch statement is allowed to act as expressions - Token::Switch if settings.allow_switch_expr => Expr::Stmt( - Box::new(vec![parse_switch(input, state, lib, settings.level_up())?].into()), - settings.pos, - ), + Token::Switch if settings.allow_switch_expr => Expr::Stmt(Box::new( + parse_switch(input, state, lib, settings.level_up())?.into(), + )), + // | ... #[cfg(not(feature = "no_function"))] Token::Pipe | Token::Or if settings.allow_anonymous_fn => { @@ -1506,7 +1512,7 @@ fn make_dot_expr( let setter = state.get_interned_string(crate::engine::make_setter(&ident.name)); let hash_set = calc_fn_hash(empty(), &setter, 2); - let rhs = Expr::Property(Box::new((getter, hash_get, setter, hash_set, ident))); + let rhs = Expr::Property(Box::new(((getter, hash_get), (setter, hash_set), ident))); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } @@ -1861,8 +1867,8 @@ fn parse_custom_syntax( tokens.push(keyword); } MARKER_BLOCK => match parse_block(input, state, lib, settings)? { - Stmt::Block(statements, pos) => { - keywords.push(Expr::Stmt(Box::new(statements.into()), pos)); + block @ Stmt::Block(_, _) => { + keywords.push(Expr::Stmt(Box::new(block.into()))); let keyword = state.get_interned_string(MARKER_BLOCK); segments.push(keyword.clone()); tokens.push(keyword); @@ -2006,19 +2012,9 @@ fn parse_if( Stmt::Noop(Position::NONE) }; - let else_body = match else_body { - Stmt::If(_, _, pos) => { - let mut statements: StaticVec<_> = Default::default(); - statements.push(else_body); - StmtBlock { statements, pos } - } - Stmt::Block(_, _) | Stmt::Noop(_) => else_body.into(), - _ => unreachable!("should either be if or a block, not {:?}", else_body), - }; - Ok(Stmt::If( guard, - Box::new((if_body.into(), else_body)), + Box::new((if_body.into(), else_body.into())), settings.pos, )) } @@ -2741,7 +2737,8 @@ fn parse_fn( parse_block(input, state, lib, settings.level_up())? } (_, pos) => return Err(PERR::FnMissingBody(name).into_err(*pos)), - }; + } + .into(); let params: StaticVec<_> = params.into_iter().map(|(p, _)| p).collect(); @@ -2803,7 +2800,7 @@ fn make_curry_from_externals(fn_expr: Expr, externals: StaticVec, pos: Po let mut statements: StaticVec<_> = Default::default(); statements.extend(externals.into_iter().map(Stmt::Share)); statements.push(Stmt::Expr(expr)); - Expr::Stmt(Box::new(statements), pos) + Expr::Stmt(Box::new(StmtBlock { statements, pos })) } /// Parse an anonymous function definition. @@ -2905,7 +2902,7 @@ fn parse_anon_fn( params, #[cfg(not(feature = "no_closure"))] externals: Default::default(), - body, + body: body.into(), lib: None, #[cfg(not(feature = "no_module"))] mods: Default::default(), From 9b37d84a9b42396f14cd29e899820b1ea4b80680 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 10 Mar 2021 23:01:04 +0800 Subject: [PATCH 15/19] Enable/disable caching in FileModuleResolver. --- CHANGELOG.md | 1 + src/bin/rhai-repl.rs | 18 +++++++++++++++--- src/module/resolvers/file.rs | 30 +++++++++++++++++++++++++----- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0481e729..a0d83084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ Enhancements * Previously, `private` functions in an `AST` cannot be called with `call_fn` etc. This is inconvenient when trying to call a function inside a script which also serves as a loadable module exporting part (but not all) of the functions. Now, all functions (`private` or not) can be called in an `AST`. The `private` keyword is relegated to preventing a function from being exported. * `Dynamic::as_unit` just for completeness sake. * `bytes` method added for strings to get length quickly (if the string is ASCII-only). +* `FileModuleResolver` can now enable/disable caching. Version 0.19.13 diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index c5c91a18..44fe90f0 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -1,4 +1,6 @@ -use rhai::{Dynamic, Engine, EvalAltResult, Module, Scope, AST}; +use rhai::{ + module_resolvers::FileModuleResolver, Dynamic, Engine, EvalAltResult, Module, Scope, AST, +}; #[cfg(not(feature = "no_optimize"))] use rhai::OptimizationLevel; @@ -56,12 +58,19 @@ fn print_help() { } fn main() { - let mut engine = Engine::new(); - println!("Rhai REPL tool"); println!("=============="); print_help(); + // Initialize scripting engine + let mut engine = Engine::new(); + + // Set a file module resolver without caching + let mut resolver = FileModuleResolver::new(); + resolver.enable_cache(false); + + engine.set_module_resolver(resolver); + // Load init scripts #[cfg(not(feature = "no_module"))] @@ -130,6 +139,9 @@ fn main() { let mut ast_u: AST = Default::default(); let mut ast: AST = Default::default(); + // Make Engine immutable + let engine = engine; + // REPL loop 'main_loop: loop { diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index 5d71e0a2..e3a86634 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -41,6 +41,7 @@ use crate::{Engine, EvalAltResult, Module, ModuleResolver, Position, Shared}; pub struct FileModuleResolver { base_path: PathBuf, extension: String, + cache_enabled: bool, #[cfg(not(feature = "sync"))] cache: crate::stdlib::cell::RefCell>>, @@ -101,6 +102,7 @@ impl FileModuleResolver { Self { base_path: path.into(), extension: extension.into(), + cache_enabled: true, cache: Default::default(), } } @@ -152,9 +154,25 @@ impl FileModuleResolver { self } + /// Enable/disable the cache. + #[inline(always)] + pub fn enable_cache(&mut self, enable: bool) -> &mut Self { + self.cache_enabled = enable; + self + } + /// Is the cache enabled? + #[inline(always)] + pub fn is_cache_enabled(&self) -> bool { + self.cache_enabled + } + /// Is a particular path cached? #[inline(always)] pub fn is_cached(&self, path: &str) -> bool { + if !self.cache_enabled { + return false; + } + let file_path = self.get_file_path(path); #[cfg(not(feature = "sync"))] @@ -211,7 +229,7 @@ impl ModuleResolver for FileModuleResolver { let file_path = self.get_file_path(path); // See if it is cached - { + if self.is_cache_enabled() { #[cfg(not(feature = "sync"))] let c = self.cache.borrow(); #[cfg(feature = "sync")] @@ -242,10 +260,12 @@ impl ModuleResolver for FileModuleResolver { .into(); // Put it into the cache - #[cfg(not(feature = "sync"))] - self.cache.borrow_mut().insert(file_path, m.clone()); - #[cfg(feature = "sync")] - self.cache.write().unwrap().insert(file_path, m.clone()); + if self.is_cache_enabled() { + #[cfg(not(feature = "sync"))] + self.cache.borrow_mut().insert(file_path, m.clone()); + #[cfg(feature = "sync")] + self.cache.write().unwrap().insert(file_path, m.clone()); + } Ok(m) } From 99020f3ed1c1fae123c8b8003f3644a4fc932aa3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 10 Mar 2021 23:37:04 +0800 Subject: [PATCH 16/19] Fix builds. --- src/ast.rs | 5 +++-- src/bin/rhai-repl.rs | 49 +++++++++++++++++--------------------------- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 1e9f593c..a901ec82 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -11,6 +11,7 @@ use crate::stdlib::{ num::NonZeroUsize, ops::{Add, AddAssign}, string::String, + vec, vec::Vec, }; use crate::token::Token; @@ -244,7 +245,7 @@ impl AST { #[deprecated = "this method is volatile and may change"] #[inline(always)] pub fn statements(&self) -> &[Stmt] { - &self.statements + &self.body.statements } /// Get a mutable reference to the statements. #[cfg(not(feature = "no_optimize"))] @@ -711,7 +712,7 @@ impl AST { .chain({ #[cfg(not(feature = "no_function"))] { - self.iter_fn_def().map(|f| &f.body) + self.iter_fn_def().flat_map(|f| f.body.statements.iter()) } #[cfg(feature = "no_function")] { diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index 44fe90f0..fedd1dc0 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -1,9 +1,4 @@ -use rhai::{ - module_resolvers::FileModuleResolver, Dynamic, Engine, EvalAltResult, Module, Scope, AST, -}; - -#[cfg(not(feature = "no_optimize"))] -use rhai::OptimizationLevel; +use rhai::{Dynamic, Engine, EvalAltResult, Module, Scope, AST}; use std::{ env, @@ -65,35 +60,31 @@ fn main() { // Initialize scripting engine let mut engine = Engine::new(); - // Set a file module resolver without caching - let mut resolver = FileModuleResolver::new(); - resolver.enable_cache(false); - - engine.set_module_resolver(resolver); - - // Load init scripts - #[cfg(not(feature = "no_module"))] { + // Set a file module resolver without caching + let mut resolver = rhai::module_resolvers::FileModuleResolver::new(); + resolver.enable_cache(false); + engine.set_module_resolver(resolver); + + // Load init scripts let mut contents = String::new(); let mut has_init_scripts = false; for filename in env::args().skip(1) { - { - contents.clear(); + contents.clear(); - let mut f = match File::open(&filename) { - Err(err) => { - eprintln!("Error reading script file: {}\n{}", filename, err); - exit(1); - } - Ok(f) => f, - }; - - if let Err(err) = f.read_to_string(&mut contents) { - println!("Error reading script file: {}\n{}", filename, err); + let mut f = match File::open(&filename) { + Err(err) => { + eprintln!("Error reading script file: {}\n{}", filename, err); exit(1); } + Ok(f) => f, + }; + + if let Err(err) = f.read_to_string(&mut contents) { + println!("Error reading script file: {}\n{}", filename, err); + exit(1); } let module = match engine @@ -128,9 +119,8 @@ fn main() { } // Setup Engine - #[cfg(not(feature = "no_optimize"))] - engine.set_optimization_level(OptimizationLevel::None); + engine.set_optimization_level(rhai::OptimizationLevel::None); let mut scope = Scope::new(); @@ -143,7 +133,6 @@ fn main() { let engine = engine; // REPL loop - 'main_loop: loop { print!("rhai-repl> "); stdout().flush().expect("couldn't flush stdout"); @@ -245,7 +234,7 @@ fn main() { #[cfg(not(feature = "no_optimize"))] { - ast = engine.optimize_ast(&scope, r, OptimizationLevel::Simple); + ast = engine.optimize_ast(&scope, r, rhai::OptimizationLevel::Simple); } #[cfg(feature = "no_optimize")] From b2fd0222de1da4b59f8c23365ecb94bb0a892108 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 11 Mar 2021 18:29:22 +0800 Subject: [PATCH 17/19] Refine statement block optimization. --- src/ast.rs | 70 ++++++++++- src/optimize.rs | 329 ++++++++++++++++++++++-------------------------- 2 files changed, 221 insertions(+), 178 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index a901ec82..63004361 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1006,6 +1006,31 @@ impl Stmt { self } + /// Does this statement return a value? + pub fn returns_value(&self) -> bool { + match self { + Self::If(_, _, _) | Self::Switch(_, _, _) | Self::Block(_, _) | Self::Expr(_) => true, + + Self::Noop(_) + | Self::While(_, _, _) + | Self::Do(_, _, _, _) + | Self::For(_, _, _) + | Self::TryCatch(_, _, _) => false, + + Self::Let(_, _, _, _) + | Self::Const(_, _, _, _) + | Self::Assignment(_, _) + | Self::Continue(_) + | Self::Break(_) + | Self::Return(_, _, _) => false, + + #[cfg(not(feature = "no_module"))] + Self::Import(_, _, _) | Self::Export(_, _) => false, + + #[cfg(not(feature = "no_closure"))] + Self::Share(_) => unreachable!("Stmt::Share should not be parsed"), + } + } /// Is this statement self-terminated (i.e. no need for a semicolon terminator)? pub fn is_self_terminated(&self) -> bool { match self { @@ -1077,8 +1102,51 @@ impl Stmt { Self::Share(_) => false, } } + /// Is this statement _pure_ within the containing block? + /// + /// An internally pure statement only has side effects that disappear outside the block. + /// + /// Only variable declarations (i.e. `let` and `const`) and `import`/`export` statements + /// are internally pure. + pub fn is_internally_pure(&self) -> bool { + match self { + Self::Let(expr, _, _, _) | Self::Const(expr, _, _, _) => expr.is_pure(), + + #[cfg(not(feature = "no_module"))] + Self::Import(expr, _, _) => expr.is_pure(), + #[cfg(not(feature = "no_module"))] + Self::Export(_, _) => true, + + _ => self.is_pure(), + } + } + /// Does this statement break the current control flow through the containing block? + /// + /// Currently this is only true for `return`, `throw`, `break` and `continue`. + /// + /// All statements following this statement will essentially be dead code. + pub fn is_control_flow_break(&self) -> bool { + match self { + Self::Return(_, _, _) | Self::Break(_) | Self::Continue(_) => true, + + Self::Noop(_) + | Self::If(_, _, _) + | Self::Switch(_, _, _) + | Self::While(_, _, _) + | Self::Do(_, _, _, _) + | Self::For(_, _, _) + | Self::Let(_, _, _, _) + | Self::Const(_, _, _, _) + | Self::Assignment(_, _) + | Self::Block(_, _) + | Self::TryCatch(_, _, _) + | Self::Expr(_) + | Self::Import(_, _, _) + | Self::Export(_, _) + | Self::Share(_) => false, + } + } /// Recursively walk this statement. - #[inline(always)] pub fn walk<'a>(&'a self, path: &mut Vec>, on_node: &mut impl FnMut(&[ASTNode])) { path.push(self.into()); on_node(path); diff --git a/src/optimize.rs b/src/optimize.rs index 07dc1d48..06a06eaf 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -74,14 +74,18 @@ struct State<'a> { impl<'a> State<'a> { /// Create a new State. #[inline(always)] - pub fn new(engine: &'a Engine, lib: &'a [&'a Module], level: OptimizationLevel) -> Self { + pub fn new( + engine: &'a Engine, + lib: &'a [&'a Module], + optimization_level: OptimizationLevel, + ) -> Self { Self { changed: false, variables: vec![], propagate_constants: true, engine, lib, - optimization_level: level, + optimization_level, } } /// Reset the state from dirty to clean. @@ -94,6 +98,11 @@ impl<'a> State<'a> { pub fn set_dirty(&mut self) { self.changed = true; } + /// Set the [`AST`] state to be not dirty (i.e. unchanged). + #[inline(always)] + pub fn clear_dirty(&mut self) { + self.changed = false; + } /// Is the [`AST`] dirty (i.e. changed)? #[inline(always)] pub fn is_dirty(&self) -> bool { @@ -168,7 +177,6 @@ fn call_fn_with_constant_arguments( /// Optimize a block of [statements][Stmt]. fn optimize_stmt_block( mut statements: Vec, - pos: Position, state: &mut State, preserve_result: bool, ) -> Vec { @@ -176,100 +184,104 @@ fn optimize_stmt_block( return statements; } - let orig_len = statements.len(); // Original number of statements in the block, for change detection - let orig_constants_len = state.variables.len(); // Original number of constants in the state, for restore later - let orig_propagate_constants = state.propagate_constants; + let mut is_dirty = state.is_dirty(); - // Optimize each statement in the block - statements.iter_mut().for_each(|stmt| { - match stmt { - // Add constant literals into the state - Stmt::Const(value_expr, Ident { name, .. }, _, _) => { - optimize_expr(value_expr, state); + loop { + state.clear_dirty(); - if value_expr.is_constant() { - state.push_var(name, AccessMode::ReadOnly, value_expr.clone()); + let orig_constants_len = state.variables.len(); // Original number of constants in the state, for restore later + let orig_propagate_constants = state.propagate_constants; + + // Remove everything following control flow breaking statements + let mut dead_code = false; + + statements.retain(|stmt| { + if dead_code { + state.set_dirty(); + false + } else if stmt.is_control_flow_break() { + dead_code = true; + true + } else { + true + } + }); + + // Optimize each statement in the block + statements.iter_mut().for_each(|stmt| { + match stmt { + // Add constant literals into the state + Stmt::Const(value_expr, Ident { name, .. }, _, _) => { + optimize_expr(value_expr, state); + + if value_expr.is_constant() { + state.push_var(name, AccessMode::ReadOnly, value_expr.clone()); + } + } + // Add variables into the state + Stmt::Let(value_expr, Ident { name, pos, .. }, _, _) => { + optimize_expr(value_expr, state); + state.push_var(name, AccessMode::ReadWrite, Expr::Unit(*pos)); + } + // Optimize the statement + _ => optimize_stmt(stmt, state, preserve_result), + } + }); + + // Remove all pure statements that do not return values at the end of a block. + // We cannot remove anything for non-pure statements due to potential side-effects. + if preserve_result { + loop { + match &statements[..] { + [stmt] if !stmt.returns_value() && stmt.is_internally_pure() => { + state.set_dirty(); + statements.clear(); + } + [.., second_last_stmt, Stmt::Noop(_)] if second_last_stmt.returns_value() => {} + [.., second_last_stmt, last_stmt] + if !last_stmt.returns_value() && last_stmt.is_internally_pure() => + { + state.set_dirty(); + if second_last_stmt.returns_value() { + *statements.last_mut().unwrap() = Stmt::Noop(last_stmt.position()); + } else { + statements.pop().unwrap(); + } + } + _ => break, } } - // Add variables into the state - Stmt::Let(value_expr, Ident { name, pos, .. }, _, _) => { - optimize_expr(value_expr, state); - state.push_var(name, AccessMode::ReadWrite, Expr::Unit(*pos)); - } - // Optimize the statement - _ => optimize_stmt(stmt, state, preserve_result), - } - }); - - // Remove all raw expression statements that are pure except for the very last statement - let last_stmt = if preserve_result { - statements.pop() - } else { - None - }; - - statements.retain(|stmt| !stmt.is_pure()); - - if let Some(stmt) = last_stmt { - statements.push(stmt); - } - - // Remove all let/import statements at the end of a block - the new variables will go away anyway. - // But be careful only remove ones that have no initial values or have values that are pure expressions, - // otherwise there may be side effects. - let mut removed = false; - - while let Some(expr) = statements.pop() { - match expr { - Stmt::Let(expr, _, _, _) | Stmt::Const(expr, _, _, _) => removed = expr.is_pure(), - #[cfg(not(feature = "no_module"))] - Stmt::Import(expr, _, _) => removed = expr.is_pure(), - _ => { - statements.push(expr); - break; + } else { + loop { + match &statements[..] { + [stmt] if stmt.is_internally_pure() => { + state.set_dirty(); + statements.clear(); + } + [.., last_stmt] if last_stmt.is_internally_pure() => { + state.set_dirty(); + statements.pop().unwrap(); + } + _ => break, + } } } + + // Pop the stack and remove all the local constants + state.restore_var(orig_constants_len); + state.propagate_constants = orig_propagate_constants; + + if !state.is_dirty() { + break; + } + + is_dirty = true; } - if preserve_result { - if removed { - statements.push(Stmt::Noop(pos)) - } - - // Optimize all the statements again - let num_statements = statements.len(); - statements - .iter_mut() - .enumerate() - .for_each(|(i, stmt)| optimize_stmt(stmt, state, i >= num_statements - 1)); - } - - // Remove everything following the the first return/throw - let mut dead_code = false; - - statements.retain(|stmt| { - if dead_code { - return false; - } - - match stmt { - Stmt::Return(_, _, _) | Stmt::Break(_) => dead_code = true, - _ => (), - } - - true - }); - - // Change detection - if orig_len != statements.len() { + if is_dirty { state.set_dirty(); } - // Pop the stack and remove all the local constants - state.restore_var(orig_constants_len); - - state.propagate_constants = orig_propagate_constants; - statements } @@ -311,7 +323,6 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { state.set_dirty(); *stmt = match optimize_stmt_block( mem::take(&mut x.1.statements).into_vec(), - x.1.pos, state, preserve_result, ) { @@ -324,7 +335,6 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { state.set_dirty(); *stmt = match optimize_stmt_block( mem::take(&mut x.0.statements).into_vec(), - x.0.pos, state, preserve_result, ) { @@ -337,14 +347,12 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { optimize_expr(condition, state); x.0.statements = optimize_stmt_block( mem::take(&mut x.0.statements).into_vec(), - x.0.pos, state, preserve_result, ) .into(); x.1.statements = optimize_stmt_block( mem::take(&mut x.1.statements).into_vec(), - x.1.pos, state, preserve_result, ) @@ -364,24 +372,14 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { let (statements, new_pos) = if let Some(block) = table.get_mut(&hash) { ( - optimize_stmt_block( - mem::take(&mut block.statements).into_vec(), - block.pos, - state, - true, - ) - .into(), + optimize_stmt_block(mem::take(&mut block.statements).into_vec(), state, true) + .into(), block.pos, ) } else { ( - optimize_stmt_block( - mem::take(&mut x.1.statements).into_vec(), - x.1.pos, - state, - true, - ) - .into(), + optimize_stmt_block(mem::take(&mut x.1.statements).into_vec(), state, true) + .into(), if x.1.pos.is_none() { *pos } else { x.1.pos }, ) }; @@ -397,7 +395,6 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { x.0.values_mut().for_each(|block| { block.statements = optimize_stmt_block( mem::take(&mut block.statements).into_vec(), - block.pos, state, preserve_result, ) @@ -405,7 +402,6 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { }); x.1.statements = optimize_stmt_block( mem::take(&mut x.1.statements).into_vec(), - x.1.pos, state, preserve_result, ) @@ -421,13 +417,9 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { Stmt::While(condition, block, _) => { optimize_expr(condition, state); - block.statements = optimize_stmt_block( - mem::take(&mut block.statements).into_vec(), - block.pos, - state, - false, - ) - .into(); + block.statements = + optimize_stmt_block(mem::take(&mut block.statements).into_vec(), state, false) + .into(); if block.len() == 1 { match block.statements[0] { @@ -454,36 +446,22 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { | Stmt::Do(block, Expr::BoolConstant(false, _), true, _) => { state.set_dirty(); *stmt = Stmt::Block( - optimize_stmt_block( - mem::take(&mut block.statements).into_vec(), - block.pos, - state, - false, - ), + optimize_stmt_block(mem::take(&mut block.statements).into_vec(), state, false), block.pos, ); } // do { block } while|until expr Stmt::Do(block, condition, _, _) => { optimize_expr(condition, state); - block.statements = optimize_stmt_block( - mem::take(&mut block.statements).into_vec(), - block.pos, - state, - false, - ) - .into(); + block.statements = + optimize_stmt_block(mem::take(&mut block.statements).into_vec(), state, false) + .into(); } // for id in expr { block } Stmt::For(iterable, x, _) => { optimize_expr(iterable, state); - x.1.statements = optimize_stmt_block( - mem::take(&mut x.1.statements).into_vec(), - x.1.pos, - state, - false, - ) - .into(); + x.1.statements = + optimize_stmt_block(mem::take(&mut x.1.statements).into_vec(), state, false).into(); } // let id = expr; Stmt::Let(expr, _, _, _) => optimize_expr(expr, state), @@ -492,7 +470,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { Stmt::Import(expr, _, _) => optimize_expr(expr, state), // { block } Stmt::Block(statements, pos) => { - *stmt = match optimize_stmt_block(mem::take(statements), *pos, state, preserve_result) { + *stmt = match optimize_stmt_block(mem::take(statements), state, preserve_result) { statements if statements.is_empty() => { state.set_dirty(); Stmt::Noop(*pos) @@ -510,31 +488,16 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { // If try block is pure, there will never be any exceptions state.set_dirty(); *stmt = Stmt::Block( - optimize_stmt_block( - mem::take(&mut x.0.statements).into_vec(), - x.0.pos, - state, - false, - ), + optimize_stmt_block(mem::take(&mut x.0.statements).into_vec(), state, false), x.0.pos, ); } // try { block } catch ( var ) { block } Stmt::TryCatch(x, _, _) => { - x.0.statements = optimize_stmt_block( - mem::take(&mut x.0.statements).into_vec(), - x.0.pos, - state, - false, - ) - .into(); - x.2.statements = optimize_stmt_block( - mem::take(&mut x.2.statements).into_vec(), - x.2.pos, - state, - false, - ) - .into(); + x.0.statements = + optimize_stmt_block(mem::take(&mut x.0.statements).into_vec(), state, false).into(); + x.2.statements = + optimize_stmt_block(mem::take(&mut x.2.statements).into_vec(), state, false).into(); } // {} Stmt::Expr(Expr::Stmt(x)) if x.statements.is_empty() => { @@ -569,7 +532,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { // {} Expr::Stmt(x) if x.statements.is_empty() => { state.set_dirty(); *expr = Expr::Unit(x.pos) } // { stmt; ... } - do not count promotion as dirty because it gets turned back into an array - Expr::Stmt(x) => x.statements = optimize_stmt_block(mem::take(&mut x.statements).into_vec(), x.pos, state, true).into(), + Expr::Stmt(x) => x.statements = optimize_stmt_block(mem::take(&mut x.statements).into_vec(), state, true).into(), // lhs.rhs #[cfg(not(feature = "no_object"))] Expr::Dot(x, _) => match (&mut x.lhs, &mut x.rhs) { @@ -800,16 +763,16 @@ fn optimize_top_level( engine: &Engine, scope: &Scope, lib: &[&Module], - level: OptimizationLevel, + optimization_level: OptimizationLevel, ) -> Vec { // If optimization level is None then skip optimizing - if level == OptimizationLevel::None { + if optimization_level == OptimizationLevel::None { statements.shrink_to_fit(); return statements; } // Set up the state - let mut state = State::new(engine, lib, level); + let mut state = State::new(engine, lib, optimization_level); // Add constants and variables from the scope scope.iter().for_each(|(name, constant, value)| { @@ -887,12 +850,12 @@ pub fn optimize_into_ast( scope: &Scope, mut statements: Vec, _functions: Vec, - level: OptimizationLevel, + optimization_level: OptimizationLevel, ) -> AST { let level = if cfg!(feature = "no_optimize") { OptimizationLevel::None } else { - level + optimization_level }; #[cfg(not(feature = "no_function"))] @@ -921,30 +884,42 @@ pub fn optimize_into_ast( lib2.set_script_fn(fn_def); }); + let lib2 = &[&lib2]; + _functions .into_iter() .map(|mut fn_def| { let pos = fn_def.body.pos; - // Optimize the function body - let mut body = optimize_top_level( - fn_def.body.statements.into_vec(), - engine, - &Scope::new(), - &[&lib2], - level, - ); + let mut body = fn_def.body.statements.into_vec(); - match &mut body[..] { - // { return val; } -> val - [Stmt::Return(crate::ast::ReturnType::Return, Some(expr), _)] => { - body[0] = Stmt::Expr(mem::take(expr)) + loop { + // Optimize the function body + let state = &mut State::new(engine, lib2, level); + + body = optimize_stmt_block(body, state, true); + + match &mut body[..] { + // { return; } -> {} + [Stmt::Return(crate::ast::ReturnType::Return, None, _)] => { + body.clear(); + } + // { ...; return; } -> { ... } + [.., last_stmt, Stmt::Return(crate::ast::ReturnType::Return, None, _)] + if !last_stmt.returns_value() => + { + body.pop().unwrap(); + } + // { ...; return val; } -> { ...; val } + [.., Stmt::Return(crate::ast::ReturnType::Return, expr, pos)] => { + *body.last_mut().unwrap() = if let Some(expr) = expr { + Stmt::Expr(mem::take(expr)) + } else { + Stmt::Noop(*pos) + }; + } + _ => break, } - // { return; } -> () - [Stmt::Return(crate::ast::ReturnType::Return, None, _)] => { - body.clear(); - } - _ => (), } fn_def.body = StmtBlock { From 7b8a4c46e783fe5a6b79ea1d94afbf2589743946 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 11 Mar 2021 21:55:55 +0800 Subject: [PATCH 18/19] Add ability to terminate AST walk. --- CHANGELOG.md | 5 +- src/ast.rs | 230 +++++++++++++++++++++++++++++++++------------- src/engine_api.rs | 3 +- 3 files changed, 173 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0d83084..d67f2585 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ Rhai Release Notes Version 0.19.14 =============== -This version runs faster due to optimizations done on AST node structures. +This version runs faster due to optimizations done on AST node structures. It also fixes a number of +panic bugs related to passing shared values as function call arguments. Bug fixes --------- @@ -33,6 +34,7 @@ Breaking changes * `num-traits` is now a required dependency. * The `in` operator is now implemented on top of the `contains` function and is no longer restricted to a few specific types. * `EvalAltResult::ErrorInExpr` is removed because the `in` operator now calls `contains`. +* The methods `AST::walk`, `Expr::walk`, `Stmt::walk` and `ASTNode::walk` and the callbacks they take now return `bool` to optionally terminate the recursive walk. Enhancements ------------ @@ -45,6 +47,7 @@ Enhancements * `Dynamic::as_unit` just for completeness sake. * `bytes` method added for strings to get length quickly (if the string is ASCII-only). * `FileModuleResolver` can now enable/disable caching. +* Recursively walking an `AST` can now be terminated in the middle. Version 0.19.13 diff --git a/src/ast.rs b/src/ast.rs index 63004361..b3718ee1 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -684,25 +684,29 @@ impl AST { self.body = Default::default(); } /// Recursively walk the [`AST`], including function bodies (if any). + /// Return `false` from the callback to terminate the walk. #[cfg(not(feature = "internals"))] #[cfg(not(feature = "no_module"))] #[inline(always)] - pub(crate) fn walk(&self, on_node: &mut impl FnMut(&[ASTNode])) { - self.statements() - .iter() - .chain({ - #[cfg(not(feature = "no_function"))] - { - self.iter_fn_def().flat_map(|f| f.body.statements.iter()) - } - #[cfg(feature = "no_function")] - { - crate::stdlib::iter::empty() - } - }) - .for_each(|stmt| stmt.walk(&mut Default::default(), on_node)); + pub(crate) fn walk(&self, on_node: &mut impl FnMut(&[ASTNode]) -> bool) -> bool { + let path = &mut Default::default(); + + 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.statements.iter()) { + if !stmt.walk(path, on_node) { + return false; + } + } + + true } /// _(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(always)] @@ -1108,6 +1112,7 @@ impl Stmt { /// /// Only variable declarations (i.e. `let` and `const`) and `import`/`export` statements /// are internally pure. + #[inline(always)] pub fn is_internally_pure(&self) -> bool { match self { Self::Let(expr, _, _, _) | Self::Const(expr, _, _, _) => expr.is_pure(), @@ -1125,70 +1130,126 @@ impl Stmt { /// Currently this is only true for `return`, `throw`, `break` and `continue`. /// /// All statements following this statement will essentially be dead code. + #[inline(always)] pub fn is_control_flow_break(&self) -> bool { match self { Self::Return(_, _, _) | Self::Break(_) | Self::Continue(_) => true, - - Self::Noop(_) - | Self::If(_, _, _) - | Self::Switch(_, _, _) - | Self::While(_, _, _) - | Self::Do(_, _, _, _) - | Self::For(_, _, _) - | Self::Let(_, _, _, _) - | Self::Const(_, _, _, _) - | Self::Assignment(_, _) - | Self::Block(_, _) - | Self::TryCatch(_, _, _) - | Self::Expr(_) - | Self::Import(_, _, _) - | Self::Export(_, _) - | Self::Share(_) => false, + _ => false, } } /// Recursively walk this statement. - pub fn walk<'a>(&'a self, path: &mut Vec>, on_node: &mut impl FnMut(&[ASTNode])) { + /// Return `false` from the callback to terminate the walk. + pub fn walk<'a>( + &'a self, + path: &mut Vec>, + on_node: &mut impl FnMut(&[ASTNode]) -> bool, + ) -> bool { path.push(self.into()); - on_node(path); + + if !on_node(path) { + return false; + } match self { - Self::Let(e, _, _, _) | Self::Const(e, _, _, _) => e.walk(path, on_node), + Self::Let(e, _, _, _) | Self::Const(e, _, _, _) => { + if !e.walk(path, on_node) { + return false; + } + } Self::If(e, x, _) => { - e.walk(path, on_node); - x.0.statements.iter().for_each(|s| s.walk(path, on_node)); - x.1.statements.iter().for_each(|s| s.walk(path, on_node)); + if !e.walk(path, on_node) { + return false; + } + for s in &x.0.statements { + if !s.walk(path, on_node) { + return false; + } + } + for s in &x.1.statements { + if !s.walk(path, on_node) { + return false; + } + } } Self::Switch(e, x, _) => { - e.walk(path, on_node); - x.0.values() - .flat_map(|block| block.statements.iter()) - .for_each(|s| s.walk(path, on_node)); - x.1.statements.iter().for_each(|s| s.walk(path, on_node)); + if !e.walk(path, on_node) { + return false; + } + for s in x.0.values().flat_map(|block| block.statements.iter()) { + if !s.walk(path, on_node) { + return false; + } + } + for s in &x.1.statements { + if !s.walk(path, on_node) { + return false; + } + } } Self::While(e, s, _) | Self::Do(s, e, _, _) => { - e.walk(path, on_node); - s.statements.iter().for_each(|s| s.walk(path, on_node)); + if !e.walk(path, on_node) { + return false; + } + for s in &s.statements { + if !s.walk(path, on_node) { + return false; + } + } } Self::For(e, x, _) => { - e.walk(path, on_node); - x.1.statements.iter().for_each(|s| s.walk(path, on_node)); + if !e.walk(path, on_node) { + return false; + } + for s in &x.1.statements { + if !s.walk(path, on_node) { + return false; + } + } } Self::Assignment(x, _) => { - x.0.walk(path, on_node); - x.1.walk(path, on_node); + if !x.0.walk(path, on_node) { + return false; + } + if !x.1.walk(path, on_node) { + return false; + } + } + Self::Block(x, _) => { + for s in x { + if !s.walk(path, on_node) { + return false; + } + } } - Self::Block(x, _) => x.iter().for_each(|s| s.walk(path, on_node)), Self::TryCatch(x, _, _) => { - x.0.statements.iter().for_each(|s| s.walk(path, on_node)); - x.2.statements.iter().for_each(|s| s.walk(path, on_node)); + for s in &x.0.statements { + if !s.walk(path, on_node) { + return false; + } + } + for s in &x.2.statements { + if !s.walk(path, on_node) { + return false; + } + } + } + Self::Expr(e) | Self::Return(_, Some(e), _) => { + if !e.walk(path, on_node) { + return false; + } } - Self::Expr(e) | Self::Return(_, Some(e), _) => e.walk(path, on_node), #[cfg(not(feature = "no_module"))] - Self::Import(e, _, _) => e.walk(path, on_node), + Self::Import(e, _, _) => { + if !e.walk(path, on_node) { + return false; + } + } _ => (), } path.pop().unwrap(); + + true } } @@ -1703,25 +1764,68 @@ impl Expr { } } /// Recursively walk this expression. - #[inline] - pub fn walk<'a>(&'a self, path: &mut Vec>, on_node: &mut impl FnMut(&[ASTNode])) { + /// Return `false` from the callback to terminate the walk. + pub fn walk<'a>( + &'a self, + path: &mut Vec>, + on_node: &mut impl FnMut(&[ASTNode]) -> bool, + ) -> bool { path.push(self.into()); - on_node(path); + + if !on_node(path) { + return false; + } match self { - Self::Stmt(x) => x.statements.iter().for_each(|s| s.walk(path, on_node)), - Self::Array(x, _) => x.iter().for_each(|e| e.walk(path, on_node)), - Self::Map(x, _) => x.iter().for_each(|(_, e)| e.walk(path, on_node)), - Self::Index(x, _) | Self::Dot(x, _) | Expr::And(x, _) | Expr::Or(x, _) => { - x.lhs.walk(path, on_node); - x.rhs.walk(path, on_node); + Self::Stmt(x) => { + for s in &x.statements { + if !s.walk(path, on_node) { + return false; + } + } + } + Self::Array(x, _) => { + for e in x.as_ref() { + if !e.walk(path, on_node) { + return false; + } + } + } + Self::Map(x, _) => { + for (_, e) in x.as_ref() { + if !e.walk(path, on_node) { + return false; + } + } + } + Self::Index(x, _) | Self::Dot(x, _) | Expr::And(x, _) | Expr::Or(x, _) => { + if !x.lhs.walk(path, on_node) { + return false; + } + if !x.rhs.walk(path, on_node) { + return false; + } + } + Self::FnCall(x, _) => { + for e in &x.args { + if !e.walk(path, on_node) { + return false; + } + } + } + Self::Custom(x, _) => { + for e in &x.keywords { + if !e.walk(path, on_node) { + return false; + } + } } - Self::FnCall(x, _) => x.args.iter().for_each(|e| e.walk(path, on_node)), - Self::Custom(x, _) => x.keywords.iter().for_each(|e| e.walk(path, on_node)), _ => (), } path.pop().unwrap(); + + true } } diff --git a/src/engine_api.rs b/src/engine_api.rs index c2f0bb28..7218582d 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -926,8 +926,9 @@ impl Engine { if !resolver.contains_path(s) && !imports.contains(s) => { imports.insert(s.clone()); + true } - _ => (), + _ => true, }); } From c2a34bd5188e5d589fc60792f5b52da18d9a690a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 11 Mar 2021 22:27:35 +0800 Subject: [PATCH 19/19] Fix internals build. --- src/ast.rs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index b3718ee1..9ce4c8bf 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -710,20 +710,22 @@ impl AST { /// Exported under the `internals` feature only. #[cfg(feature = "internals")] #[inline(always)] - pub fn walk(&self, on_node: &mut impl FnMut(&[ASTNode])) { - self.statements() - .iter() - .chain({ - #[cfg(not(feature = "no_function"))] - { - self.iter_fn_def().flat_map(|f| f.body.statements.iter()) - } - #[cfg(feature = "no_function")] - { - crate::stdlib::iter::empty() - } - }) - .for_each(|stmt| stmt.walk(&mut Default::default(), on_node)); + pub fn walk(&self, on_node: &mut impl FnMut(&[ASTNode]) -> bool) -> bool { + let path = &mut Default::default(); + + 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.statements.iter()) { + if !stmt.walk(path, on_node) { + return false; + } + } + + true } }