From eca8212f389d788cadd6c287135cfd14fafabb8a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 29 Dec 2020 10:41:20 +0800 Subject: [PATCH 1/3] More code refinements. --- RELEASES.md | 1 + src/ast.rs | 10 +++ src/dynamic.rs | 7 ++ src/engine.rs | 127 ++++++++++++++++++---------------- src/engine_api.rs | 18 ++--- src/fn_call.rs | 5 +- src/fn_native.rs | 22 +++++- src/parser.rs | 161 ++++++++++++++++++++++++------------------- src/scope.rs | 1 + src/token.rs | 39 ++++++----- src/utils.rs | 1 + tests/assignments.rs | 82 ++++++++++++++++++++++ 12 files changed, 317 insertions(+), 157 deletions(-) create mode 100644 tests/assignments.rs diff --git a/RELEASES.md b/RELEASES.md index f91a0b16..9530b5a7 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -15,6 +15,7 @@ Bug fixes * Fix bug when accessing properties in closures. * Fix bug when accessing a deep index with a function call. +* Fix bug that sometimes allow assigning to an invalid l-value. Breaking changes ---------------- diff --git a/src/ast.rs b/src/ast.rs index 0707c87b..e6694324 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -88,6 +88,7 @@ pub struct ScriptFnDef { } impl fmt::Display for ScriptFnDef { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, @@ -130,6 +131,7 @@ pub struct ScriptFnMetadata<'a> { } impl fmt::Display for ScriptFnMetadata<'_> { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, @@ -146,6 +148,7 @@ impl fmt::Display for ScriptFnMetadata<'_> { } impl<'a> Into> for &'a ScriptFnDef { + #[inline(always)] fn into(self) -> ScriptFnMetadata<'a> { ScriptFnMetadata { comments: self.comments.iter().map(|s| s.as_str()).collect(), @@ -172,6 +175,7 @@ pub struct AST { } impl Default for AST { + #[inline(always)] fn default() -> Self { Self { source: None, @@ -208,14 +212,17 @@ impl AST { } } /// Get the source. + #[inline(always)] pub fn source(&self) -> Option<&str> { self.source.as_ref().map(|s| s.as_str()) } /// Clone the source. + #[inline(always)] pub(crate) fn clone_source(&self) -> Option { self.source.clone() } /// Set the source. + #[inline(always)] pub fn set_source>(&mut self, source: Option) { self.source = source.map(|s| s.into()) } @@ -655,6 +662,7 @@ pub struct Ident { } impl fmt::Debug for Ident { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Ident({:?} @ {:?})", self.name, self.pos) } @@ -736,6 +744,7 @@ impl Default for Stmt { impl Stmt { /// Is this statement [`Noop`][Stmt::Noop]? + #[inline(always)] pub fn is_noop(&self) -> bool { match self { Self::Noop(_) => true, @@ -1048,6 +1057,7 @@ impl Expr { }) } /// Is the expression a simple variable access? + #[inline(always)] pub(crate) fn get_variable_access(&self, non_qualified: bool) -> Option<&str> { match self { Self::Variable(x) if !non_qualified || x.1.is_none() => Some((x.2).name.as_str()), diff --git a/src/dynamic.rs b/src/dynamic.rs index 75b3c7a4..08e473ae 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -88,21 +88,27 @@ pub trait Variant: Any + Send + Sync + private::Sealed { } impl Variant for T { + #[inline(always)] fn as_any(&self) -> &dyn Any { self } + #[inline(always)] fn as_mut_any(&mut self) -> &mut dyn Any { self } + #[inline(always)] fn as_box_any(self: Box) -> Box { self } + #[inline(always)] fn type_name(&self) -> &'static str { type_name::() } + #[inline(always)] fn into_dynamic(self) -> Dynamic { Dynamic::from(self) } + #[inline(always)] fn clone_into_dynamic(&self) -> Dynamic { Dynamic::from(self.clone()) } @@ -127,6 +133,7 @@ pub enum AccessMode { impl AccessMode { /// Is the access type [`ReadOnly`]? + #[inline(always)] pub fn is_read_only(self) -> bool { match self { Self::ReadWrite => false, diff --git a/src/engine.rs b/src/engine.rs index 5d05c85a..dae39f5f 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -59,18 +59,22 @@ pub struct Imports(StaticVec<(ImmutableString, Shared)>); impl Imports { /// Get the length of this stack of imported [modules][Module]. + #[inline(always)] pub fn len(&self) -> usize { self.0.len() } /// Is this stack of imported [modules][Module] empty? + #[inline(always)] pub fn is_empty(&self) -> bool { self.0.is_empty() } /// Get the imported [modules][Module] at a particular index. + #[inline(always)] pub fn get(&self, index: usize) -> Option> { self.0.get(index).map(|(_, m)| m).cloned() } /// Get the index of an imported [modules][Module] by name. + #[inline(always)] pub fn find(&self, name: &str) -> Option { self.0 .iter() @@ -80,15 +84,18 @@ impl Imports { .map(|(index, _)| index) } /// Push an imported [modules][Module] onto the stack. + #[inline(always)] pub fn push(&mut self, name: impl Into, module: impl Into>) { self.0.push((name.into(), module.into())); } /// Truncate the stack of imported [modules][Module] to a particular length. + #[inline(always)] pub fn truncate(&mut self, size: usize) { self.0.truncate(size); } /// Get an iterator to this stack of imported [modules][Module] in reverse order. #[allow(dead_code)] + #[inline(always)] pub fn iter<'a>(&'a self) -> impl Iterator + 'a { self.0 .iter() @@ -97,25 +104,30 @@ impl Imports { } /// Get an iterator to this stack of imported [modules][Module] in reverse order. #[allow(dead_code)] + #[inline(always)] pub(crate) fn iter_raw<'a>( &'a self, ) -> impl Iterator)> + 'a { self.0.iter().rev().map(|(n, m)| (n, m)) } /// Get a consuming iterator to this stack of imported [modules][Module] in reverse order. + #[inline(always)] pub fn into_iter(self) -> impl Iterator)> { self.0.into_iter().rev() } /// Add a stream of imported [modules][Module]. + #[inline(always)] pub fn extend(&mut self, stream: impl Iterator)>) { self.0.extend(stream) } /// Does the specified function hash key exist in this stack of imported [modules][Module]? #[allow(dead_code)] + #[inline(always)] pub fn contains_fn(&self, hash: NonZeroU64) -> bool { self.0.iter().any(|(_, m)| m.contains_qualified_fn(hash)) } /// Get specified function via its hash key. + #[inline(always)] pub fn get_fn(&self, hash: NonZeroU64) -> Option<&CallableFunction> { self.0 .iter() @@ -124,10 +136,12 @@ impl Imports { } /// Does the specified [`TypeId`][std::any::TypeId] iterator exist in this stack of imported [modules][Module]? #[allow(dead_code)] + #[inline(always)] pub fn contains_iter(&self, id: TypeId) -> bool { self.0.iter().any(|(_, m)| m.contains_qualified_iter(id)) } /// Get the specified [`TypeId`][std::any::TypeId] iterator. + #[inline(always)] pub fn get_iter(&self, id: TypeId) -> Option { self.0 .iter() @@ -137,6 +151,7 @@ impl Imports { } impl<'a, T: IntoIterator)>> From for Imports { + #[inline(always)] fn from(value: T) -> Self { Self( value @@ -147,6 +162,7 @@ impl<'a, T: IntoIterator)>> From } } impl FromIterator<(ImmutableString, Shared)> for Imports { + #[inline(always)] fn from_iter)>>(iter: T) -> Self { Self(iter.into_iter().collect()) } @@ -222,6 +238,7 @@ impl IndexChainValue { /// # Panics /// /// Panics if not `IndexChainValue::Value`. + #[inline(always)] #[cfg(not(feature = "no_index"))] pub fn as_value(self) -> Dynamic { match self { @@ -234,6 +251,7 @@ impl IndexChainValue { /// # Panics /// /// Panics if not `IndexChainValue::FnCallArgs`. + #[inline(always)] #[cfg(not(feature = "no_object"))] pub fn as_fn_call_args(self) -> StaticVec { match self { @@ -245,6 +263,7 @@ impl IndexChainValue { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] impl From> for IndexChainValue { + #[inline(always)] fn from(value: StaticVec) -> Self { Self::FnCallArgs(value) } @@ -252,6 +271,7 @@ impl From> for IndexChainValue { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] impl From for IndexChainValue { + #[inline(always)] fn from(value: Dynamic) -> Self { Self::Value(value) } @@ -1088,7 +1108,7 @@ impl Engine { ChainType::Dot => { match rhs { // xxx.fn_name(arg_expr_list) - Expr::FnCall(x, pos) if x.namespace.is_none() => { + Expr::FnCall(x, pos) if x.namespace.is_none() && new_val.is_none() => { let FnCallExpr { name, hash_script: hash, @@ -1102,6 +1122,10 @@ impl Engine { level, ) } + // xxx.fn_name(...) = ??? + Expr::FnCall(_, _) if new_val.is_some() => { + unreachable!("method call cannot be assigned to") + } // xxx.module::fn_name(...) - syntax error Expr::FnCall(_, _) => { unreachable!("function call in dot chain should not be namespace-qualified") @@ -1578,7 +1602,7 @@ impl Engine { let args = &mut [&mut lhs_value.clone(), value]; // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s. - let hash = + let hash_fn = calc_native_fn_hash(empty(), OP_EQUALS, args.iter().map(|a| a.type_id())) .unwrap(); @@ -1586,7 +1610,8 @@ impl Engine { if self .call_native_fn( - mods, state, lib, OP_EQUALS, hash, args, false, false, pos, def_value, + mods, state, lib, OP_EQUALS, hash_fn, args, false, false, pos, + def_value, )? .0 .as_bool() @@ -1748,17 +1773,14 @@ impl Engine { Expr::StringConstant(x, _) => Ok(x.clone().into()), Expr::CharConstant(x, _) => Ok((*x).into()), Expr::FnPointer(x, _) => Ok(FnPtr::new_unchecked(x.clone(), Default::default()).into()), - Expr::Variable(x) if (x.2).name == KEYWORD_THIS => { - if let Some(val) = this_ptr { - Ok(val.clone()) - } else { - EvalAltResult::ErrorUnboundThis((x.2).pos).into() - } - } - Expr::Variable(_) => { - let (val, _) = self.search_namespace(scope, mods, state, lib, this_ptr, expr)?; - Ok(val.take_or_clone()) - } + + Expr::Variable(x) if (x.2).name == KEYWORD_THIS => this_ptr + .as_deref() + .cloned() + .ok_or_else(|| EvalAltResult::ErrorUnboundThis((x.2).pos).into()), + Expr::Variable(_) => self + .search_namespace(scope, mods, state, lib, this_ptr, expr) + .map(|(val, _)| val.take_or_clone()), // Statement block Expr::Stmt(x, _) => { @@ -1822,13 +1844,13 @@ impl Engine { let FnCallExpr { name, namespace, - hash_script: hash, + hash_script, args, def_value, .. } = x.as_ref(); let namespace = namespace.as_ref(); - let hash = hash.unwrap(); + let hash = hash_script.unwrap(); let def_value = def_value.as_ref(); self.make_qualified_function_call( scope, mods, state, lib, this_ptr, namespace, name, args, def_value, hash, @@ -2111,7 +2133,6 @@ impl Engine { )?; Ok(Dynamic::UNIT) } - // Non-lvalue expression (should be caught during parsing) _ => unreachable!("cannot assign to expression: {:?}", lhs_expr), } } @@ -2236,8 +2257,8 @@ impl Engine { for iter_value in func(iter_obj) { let loop_var = scope.get_mut_by_index(index); - let value = iter_value.flatten(); + if cfg!(not(feature = "no_closure")) && loop_var.is_shared() { *loop_var.write_lock().unwrap() = value; } else { @@ -2272,7 +2293,7 @@ impl Engine { // Try/Catch statement Stmt::TryCatch(x, _, _) => { - let (try_body, var_def, catch_body) = x.as_ref(); + let (try_body, err_var, catch_body) = x.as_ref(); let result = self .eval_stmt(scope, mods, state, lib, this_ptr, try_body, level) @@ -2280,51 +2301,41 @@ impl Engine { match result { Ok(_) => result, - Err(err) => match *err { - mut err @ EvalAltResult::ErrorRuntime(_, _) | mut err - if err.is_catchable() => - { - let value = if let EvalAltResult::ErrorRuntime(ref x, _) = err { - x.clone() - } else { + Err(err) if !err.is_catchable() => Err(err), + Err(mut err) => { + let value = match *err { + EvalAltResult::ErrorRuntime(ref x, _) => x.clone(), + _ => { err.set_position(Position::NONE); err.to_string().into() - }; - - let orig_scope_len = scope.len(); - state.scope_level += 1; - - if let Some(Ident { name, .. }) = var_def { - let var_name: Cow<'_, str> = if state.is_global() { - name.to_string().into() - } else { - unsafe_cast_var_name_to_lifetime(&name).into() - }; - scope.push(var_name, value); } + }; - let mut result = self - .eval_stmt(scope, mods, state, lib, this_ptr, catch_body, level) - .map(|_| ().into()); + let orig_scope_len = scope.len(); + state.scope_level += 1; - if let Some(result_err) = result.as_ref().err() { - if let EvalAltResult::ErrorRuntime( - Dynamic(Union::Unit(_, _)), - pos, - ) = result_err.as_ref() - { - err.set_position(*pos); - result = Err(Box::new(err)); - } - } - - state.scope_level -= 1; - scope.rewind(orig_scope_len); - - result + if let Some(Ident { name, .. }) = err_var { + scope.push(unsafe_cast_var_name_to_lifetime(&name), value); } - _ => Err(err), - }, + + let result = + self.eval_stmt(scope, mods, state, lib, this_ptr, catch_body, level); + + state.scope_level -= 1; + scope.rewind(orig_scope_len); + + match result { + Ok(_) => Ok(Dynamic::UNIT), + Err(result_err) => match *result_err { + // Re-throw exception + EvalAltResult::ErrorRuntime(Dynamic(Union::Unit(_, _)), pos) => { + err.set_position(pos); + Err(err) + } + _ => Err(result_err), + }, + } + } } } diff --git a/src/engine_api.rs b/src/engine_api.rs index 026e0df8..fff309b4 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -971,7 +971,7 @@ impl Engine { optimization_level: OptimizationLevel, ) -> Result { let hash = calc_hash_for_scripts(scripts); - let stream = self.lex(scripts, None); + let stream = self.lex(scripts); self.parse(hash, &mut stream.peekable(), scope, optimization_level) } /// Read the contents of a file into a string. @@ -1129,18 +1129,20 @@ impl Engine { }; let hash = calc_hash_for_scripts(&scripts); - let stream = self.lex( + + let stream = self.lex_with_map( &scripts, if has_null { - Some(Box::new(|token| match token { + |token| match token { // If `null` is present, make sure `null` is treated as a variable Token::Reserved(s) if s == "null" => Token::Identifier(s), _ => token, - })) + } } else { - None + |t| t }, ); + let ast = self.parse_global_expr( hash, &mut stream.peekable(), @@ -1226,7 +1228,7 @@ impl Engine { ) -> Result { let scripts = [script]; let hash = calc_hash_for_scripts(&scripts); - let stream = self.lex(&scripts, None); + let stream = self.lex(&scripts); let mut peekable = stream.peekable(); self.parse_global_expr(hash, &mut peekable, scope, self.optimization_level) @@ -1384,7 +1386,7 @@ impl Engine { ) -> Result> { let scripts = [script]; let hash = calc_hash_for_scripts(&scripts); - let stream = self.lex(&scripts, None); + let stream = self.lex(&scripts); // No need to optimize a lone expression let ast = @@ -1517,7 +1519,7 @@ impl Engine { ) -> Result<(), Box> { let scripts = [script]; let hash = calc_hash_for_scripts(&scripts); - let stream = self.lex(&scripts, None); + let stream = self.lex(&scripts); let ast = self.parse(hash, &mut stream.peekable(), scope, self.optimization_level)?; self.consume_ast_with_scope(scope, &ast) } diff --git a/src/fn_call.rs b/src/fn_call.rs index ae678448..c0329828 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -77,6 +77,7 @@ impl<'a> ArgBackup<'a> { /// This method blindly casts a reference to another lifetime, which saves allocation and string cloning. /// /// If `restore_first_arg` is called before the end of the scope, the shorter lifetime will not leak. + #[inline(always)] fn change_first_arg_to_copy(&mut self, normalize: bool, args: &mut FnCallArgs<'a>) { // Only do it for method calls with arguments. if !normalize || args.is_empty() { @@ -106,6 +107,7 @@ impl<'a> ArgBackup<'a> { /// /// If `change_first_arg_to_copy` has been called, this function **MUST** be called _BEFORE_ exiting /// the current scope. Otherwise it is undefined behavior as the shorter lifetime will leak. + #[inline(always)] fn restore_first_arg(&mut self, args: &mut FnCallArgs<'a>) { if let Some(this_pointer) = self.orig_mut.take() { args[0] = this_pointer; @@ -114,6 +116,7 @@ impl<'a> ArgBackup<'a> { } impl Drop for ArgBackup<'_> { + #[inline(always)] fn drop(&mut self) { // Panic if the shorter lifetime leaks. assert!( @@ -433,7 +436,7 @@ impl Engine { } // Has a system function an override? - #[inline] + #[inline(always)] pub(crate) fn has_override_by_name_and_arguments( &self, mods: Option<&Imports>, diff --git a/src/fn_native.rs b/src/fn_native.rs index ddfebb20..a1d32a37 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -65,6 +65,7 @@ impl<'e, 's, 'a, 'm, 'pm: 'm, M: AsRef<[&'pm Module]> + ?Sized> From<(&'e Engine, &'s Option, &'a Imports, &'m M)> for NativeCallContext<'e, 's, 'a, 'm, 'pm> { + #[inline(always)] fn from(value: (&'e Engine, &'s Option, &'a Imports, &'m M)) -> Self { Self { engine: value.0, @@ -78,6 +79,7 @@ impl<'e, 's, 'a, 'm, 'pm: 'm, M: AsRef<[&'pm Module]> + ?Sized> impl<'e, 'm, 'pm: 'm, M: AsRef<[&'pm Module]> + ?Sized> From<(&'e Engine, &'m M)> for NativeCallContext<'e, '_, '_, 'm, 'pm> { + #[inline(always)] fn from(value: (&'e Engine, &'m M)) -> Self { Self { engine: value.0, @@ -152,6 +154,7 @@ impl<'e, 's, 'a, 'm, 'pm> NativeCallContext<'e, 's, 'a, 'm, 'pm> { /// /// If `is_method` is [`true`], the first argument is assumed to be passed /// by reference and is not consumed. + #[inline(always)] pub fn call_fn_dynamic_raw( &self, fn_name: &str, @@ -225,6 +228,7 @@ pub struct FnPtr(ImmutableString, StaticVec); impl FnPtr { /// Create a new function pointer. + #[inline(always)] pub fn new(name: impl Into) -> Result> { name.into().try_into() } @@ -289,6 +293,7 @@ impl FnPtr { /// This is to avoid unnecessarily cloning the arguments. /// Do not use the arguments after this call. If they are needed afterwards, /// clone them _before_ calling this function. + #[inline(always)] pub fn call_dynamic( &self, ctx: NativeCallContext, @@ -306,13 +311,13 @@ impl FnPtr { let mut args = args_data.iter_mut().collect::>(); - let has_this = this_ptr.is_some(); + let is_method = this_ptr.is_some(); if let Some(obj) = this_ptr { args.insert(0, obj); } - ctx.call_fn_dynamic_raw(self.fn_name(), has_this, true, args.as_mut(), None) + ctx.call_fn_dynamic_raw(self.fn_name(), is_method, true, args.as_mut(), None) } } @@ -424,6 +429,7 @@ pub enum CallableFunction { } impl fmt::Debug for CallableFunction { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Pure(_) => write!(f, "NativePureFunction"), @@ -438,6 +444,7 @@ impl fmt::Debug for CallableFunction { } impl fmt::Display for CallableFunction { + #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Pure(_) => write!(f, "NativePureFunction"), @@ -453,6 +460,7 @@ impl fmt::Display for CallableFunction { impl CallableFunction { /// Is this a pure native Rust function? + #[inline(always)] pub fn is_pure(&self) -> bool { match self { Self::Pure(_) => true, @@ -465,6 +473,7 @@ impl CallableFunction { } } /// Is this a native Rust method function? + #[inline(always)] pub fn is_method(&self) -> bool { match self { Self::Method(_) => true, @@ -477,6 +486,7 @@ impl CallableFunction { } } /// Is this an iterator function? + #[inline(always)] pub fn is_iter(&self) -> bool { match self { Self::Iterator(_) => true, @@ -487,6 +497,7 @@ impl CallableFunction { } } /// Is this a Rhai-scripted function? + #[inline(always)] pub fn is_script(&self) -> bool { match self { #[cfg(not(feature = "no_function"))] @@ -496,6 +507,7 @@ impl CallableFunction { } } /// Is this a plugin function? + #[inline(always)] pub fn is_plugin_fn(&self) -> bool { match self { Self::Plugin(_) => true, @@ -506,6 +518,7 @@ impl CallableFunction { } } /// Is this a native Rust function? + #[inline(always)] pub fn is_native(&self) -> bool { match self { Self::Pure(_) | Self::Method(_) => true, @@ -517,6 +530,7 @@ impl CallableFunction { } } /// Get the access mode. + #[inline(always)] pub fn access(&self) -> FnAccess { match self { Self::Plugin(_) => FnAccess::Public, @@ -532,6 +546,7 @@ impl CallableFunction { /// /// Panics if the [`CallableFunction`] is not [`Pure`][CallableFunction::Pure] or /// [`Method`][CallableFunction::Method]. + #[inline(always)] pub fn get_native_fn(&self) -> &FnAny { match self { Self::Pure(f) | Self::Method(f) => f.as_ref(), @@ -547,6 +562,7 @@ impl CallableFunction { /// /// Panics if the [`CallableFunction`] is not [`Script`][CallableFunction::Script]. #[cfg(not(feature = "no_function"))] + #[inline(always)] pub fn get_fn_def(&self) -> &ScriptFnDef { match self { Self::Pure(_) | Self::Method(_) | Self::Iterator(_) | Self::Plugin(_) => { @@ -560,6 +576,7 @@ impl CallableFunction { /// # Panics /// /// Panics if the [`CallableFunction`] is not [`Iterator`][CallableFunction::Iterator]. + #[inline(always)] pub fn get_iter_fn(&self) -> IteratorFn { match self { Self::Iterator(f) => *f, @@ -576,6 +593,7 @@ impl CallableFunction { /// # Panics /// /// Panics if the [`CallableFunction`] is not [`Plugin`][CallableFunction::Plugin]. + #[inline(always)] pub fn get_plugin_fn<'s>(&'s self) -> &FnPlugin { match self { Self::Plugin(f) => f.as_ref(), diff --git a/src/parser.rs b/src/parser.rs index d9f37d74..b1c39d36 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -882,7 +882,7 @@ fn parse_switch( } }; - let stmt = parse_stmt(input, state, lib, settings.level_up()).map(Option::unwrap)?; + let stmt = parse_stmt(input, state, lib, settings.level_up())?; let need_comma = !stmt.is_self_terminated(); @@ -1014,10 +1014,11 @@ fn parse_primary( state.access_var(closure, *pos); }); - // Qualifiers (none) + function name + number of arguments. - let hash = calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(); - - lib.insert(hash, func); + lib.insert( + // Qualifiers (none) + function name + number of arguments. + calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(), + func, + ); expr } @@ -1372,7 +1373,28 @@ fn make_assignment_stmt<'a>( rhs: Expr, op_pos: Position, ) -> Result { + fn check_lvalue(expr: &Expr, parent_is_dot: bool) -> Position { + match expr { + Expr::Index(x, _) | Expr::Dot(x, _) if parent_is_dot => match x.lhs { + Expr::Property(_) => check_lvalue(&x.rhs, matches!(expr, Expr::Dot(_, _))), + ref e => e.position(), + }, + Expr::Index(x, _) | Expr::Dot(x, _) => match x.lhs { + Expr::Property(_) => unreachable!("unexpected Expr::Property in indexing"), + _ => check_lvalue(&x.rhs, matches!(expr, Expr::Dot(_, _))), + }, + Expr::Property(_) if parent_is_dot => Position::NONE, + Expr::Property(_) => unreachable!("unexpected Expr::Property in indexing"), + e if parent_is_dot => e.position(), + _ => Position::NONE, + } + } + match &lhs { + // const_expr = rhs + expr if expr.is_constant() => { + Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) + } // var (non-indexed) = rhs Expr::Variable(x) if x.0.is_none() => Ok(Stmt::Assignment( Box::new((lhs, fn_name.into(), rhs)), @@ -1392,33 +1414,36 @@ fn make_assignment_stmt<'a>( } } } - // xxx[???] = rhs, xxx.??? = rhs - Expr::Index(x, _) | Expr::Dot(x, _) => match &x.lhs { - // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs - Expr::Variable(x) if x.0.is_none() => Ok(Stmt::Assignment( - Box::new((lhs, fn_name.into(), rhs)), - op_pos, - )), - // var[???] (indexed) = rhs, var.??? (indexed) = rhs - Expr::Variable(x) => { - let (index, _, Ident { name, pos }) = x.as_ref(); - match state.stack[(state.stack.len() - index.unwrap().get())].1 { - AccessMode::ReadWrite => Ok(Stmt::Assignment( + // xxx[???]... = rhs, xxx.prop... = rhs + Expr::Index(x, _) | Expr::Dot(x, _) => { + match check_lvalue(&x.rhs, matches!(lhs, Expr::Dot(_, _))) { + Position::NONE => match &x.lhs { + // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs + Expr::Variable(x) if x.0.is_none() => Ok(Stmt::Assignment( Box::new((lhs, fn_name.into(), rhs)), op_pos, )), - // Constant values cannot be assigned to - AccessMode::ReadOnly => { - Err(PERR::AssignmentToConstant(name.to_string()).into_err(*pos)) + // var[???] (indexed) = rhs, var.??? (indexed) = rhs + Expr::Variable(x) => { + 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, fn_name.into(), rhs)), + op_pos, + )), + // Constant values cannot be assigned to + AccessMode::ReadOnly => { + Err(PERR::AssignmentToConstant(name.to_string()).into_err(*pos)) + } + } } - } + // expr[???] = rhs, expr.??? = rhs + expr => { + Err(PERR::AssignmentToInvalidLHS("".to_string()).into_err(expr.position())) + } + }, + pos => Err(PERR::AssignmentToInvalidLHS("".to_string()).into_err(pos)), } - // expr[???] = rhs, expr.??? = rhs - _ => Err(PERR::AssignmentToInvalidLHS("".to_string()).into_err(x.lhs.position())), - }, - // const_expr = rhs - expr if expr.is_constant() => { - Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) } // ??? && ??? = rhs, ??? || ??? = rhs Expr::And(_, _) | Expr::Or(_, _) => Err(LexError::ImproperSymbol( @@ -2435,7 +2460,11 @@ fn parse_block( // Parse statements inside the block settings.is_global = false; - let stmt = parse_stmt(input, state, lib, settings.level_up()).map(Option::unwrap)?; + let stmt = parse_stmt(input, state, lib, settings.level_up())?; + + if stmt.is_noop() { + continue; + } // See if it needs a terminating semicolon let need_semicolon = !stmt.is_self_terminated(); @@ -2502,7 +2531,7 @@ fn parse_stmt( state: &mut ParseState, lib: &mut FunctionsLib, mut settings: ParseSettings, -) -> Result, ParseError> { +) -> Result { use AccessMode::{ReadOnly, ReadWrite}; let mut comments: Vec = Default::default(); @@ -2538,7 +2567,7 @@ fn parse_stmt( } let (token, token_pos) = match input.peek().unwrap() { - (Token::EOF, pos) => return Ok(Some(Stmt::Noop(*pos))), + (Token::EOF, pos) => return Ok(Stmt::Noop(*pos)), x => x, }; settings.pos = *token_pos; @@ -2548,10 +2577,10 @@ fn parse_stmt( match token { // ; - empty statement - Token::SemiColon => Ok(Some(Stmt::Noop(settings.pos))), + Token::SemiColon => Ok(Stmt::Noop(settings.pos)), // { - statements block - Token::LeftBrace => Ok(Some(parse_block(input, state, lib, settings.level_up())?)), + Token::LeftBrace => Ok(parse_block(input, state, lib, settings.level_up())?), // fn ... #[cfg(not(feature = "no_function"))] @@ -2591,12 +2620,13 @@ fn parse_stmt( let func = parse_fn(input, &mut new_state, lib, access, settings, comments)?; - // Qualifiers (none) + function name + number of arguments. - let hash = calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(); + lib.insert( + // Qualifiers (none) + function name + number of arguments. + calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(), + func, + ); - lib.insert(hash, func); - - Ok(None) + Ok(Stmt::Noop(settings.pos)) } (_, pos) => Err(PERR::MissingToken( @@ -2607,21 +2637,19 @@ fn parse_stmt( } } - Token::If => parse_if(input, state, lib, settings.level_up()).map(Some), - Token::Switch => parse_switch(input, state, lib, settings.level_up()).map(Some), - Token::While | Token::Loop => { - parse_while_loop(input, state, lib, settings.level_up()).map(Some) - } - Token::Do => parse_do(input, state, lib, settings.level_up()).map(Some), - Token::For => parse_for(input, state, lib, settings.level_up()).map(Some), + Token::If => parse_if(input, state, lib, settings.level_up()), + Token::Switch => parse_switch(input, state, lib, settings.level_up()), + Token::While | Token::Loop => parse_while_loop(input, state, lib, settings.level_up()), + Token::Do => parse_do(input, state, lib, settings.level_up()), + Token::For => parse_for(input, state, lib, settings.level_up()), Token::Continue if settings.is_breakable => { let pos = eat_token(input, Token::Continue); - Ok(Some(Stmt::Continue(pos))) + Ok(Stmt::Continue(pos)) } Token::Break if settings.is_breakable => { let pos = eat_token(input, Token::Break); - Ok(Some(Stmt::Break(pos))) + Ok(Stmt::Break(pos)) } Token::Continue | Token::Break => Err(PERR::LoopBreak.into_err(settings.pos)), @@ -2645,43 +2673,35 @@ fn parse_stmt( match input.peek().unwrap() { // `return`/`throw` at - (Token::EOF, pos) => Ok(Some(Stmt::Return((return_type, token_pos), None, *pos))), + (Token::EOF, pos) => Ok(Stmt::Return((return_type, token_pos), None, *pos)), // `return;` or `throw;` - (Token::SemiColon, _) => Ok(Some(Stmt::Return( - (return_type, token_pos), - None, - settings.pos, - ))), + (Token::SemiColon, _) => { + Ok(Stmt::Return((return_type, token_pos), None, settings.pos)) + } // `return` or `throw` with expression (_, _) => { let expr = parse_expr(input, state, lib, settings.level_up())?; let pos = expr.position(); - Ok(Some(Stmt::Return( - (return_type, token_pos), - Some(expr), - pos, - ))) + Ok(Stmt::Return((return_type, token_pos), Some(expr), pos)) } } } - Token::Try => parse_try_catch(input, state, lib, settings.level_up()).map(Some), + Token::Try => parse_try_catch(input, state, lib, settings.level_up()), - Token::Let => parse_let(input, state, lib, ReadWrite, false, settings.level_up()).map(Some), - Token::Const => { - parse_let(input, state, lib, ReadOnly, false, settings.level_up()).map(Some) - } + Token::Let => parse_let(input, state, lib, ReadWrite, false, settings.level_up()), + Token::Const => parse_let(input, state, lib, ReadOnly, false, settings.level_up()), #[cfg(not(feature = "no_module"))] - Token::Import => parse_import(input, state, lib, settings.level_up()).map(Some), + Token::Import => parse_import(input, state, lib, settings.level_up()), #[cfg(not(feature = "no_module"))] Token::Export if !settings.is_global => Err(PERR::WrongExport.into_err(settings.pos)), #[cfg(not(feature = "no_module"))] - Token::Export => parse_export(input, state, lib, settings.level_up()).map(Some), + Token::Export => parse_export(input, state, lib, settings.level_up()), - _ => parse_expr_stmt(input, state, lib, settings.level_up()).map(Some), + _ => parse_expr_stmt(input, state, lib, settings.level_up()), } } @@ -2951,7 +2971,7 @@ fn parse_anon_fn( // Parse function body settings.is_breakable = false; - let body = parse_stmt(input, state, lib, settings.level_up()).map(Option::unwrap)?; + let body = parse_stmt(input, state, lib, settings.level_up())?; // External variables may need to be processed in a consistent order, // so extract them into a list. @@ -3095,10 +3115,11 @@ impl Engine { pos: Position::NONE, }; - let stmt = match parse_stmt(input, &mut state, &mut functions, settings)? { - Some(s) => s, - None => continue, - }; + let stmt = parse_stmt(input, &mut state, &mut functions, settings)?; + + if stmt.is_noop() { + continue; + } let need_semicolon = !stmt.is_self_terminated(); diff --git a/src/scope.rs b/src/scope.rs index 3498dce3..4386bae7 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -55,6 +55,7 @@ pub struct Scope<'a> { } impl Default for Scope<'_> { + #[inline(always)] fn default() -> Self { Self { values: Vec::with_capacity(16), diff --git a/src/token.rs b/src/token.rs index c10c1ba4..752c1da4 100644 --- a/src/token.rs +++ b/src/token.rs @@ -6,7 +6,6 @@ use crate::engine::{ }; use crate::stdlib::{ borrow::Cow, - boxed::Box, char, fmt, format, iter::Peekable, str::{Chars, FromStr}, @@ -1126,14 +1125,14 @@ fn get_next_token_inner( 'x' | 'X' => is_hex_char, 'o' | 'O' => is_octal_char, 'b' | 'B' => is_binary_char, - _ => unreachable!("expecting 'x', 'o' or 'B', but gets {}", ch), + _ => unreachable!("expecting 'x', 'o' or 'b', but gets {}", ch), }; radix_base = Some(match ch { 'x' | 'X' => 16, 'o' | 'O' => 8, 'b' | 'B' => 2, - _ => unreachable!("expecting 'x', 'o' or 'B', but gets {}", ch), + _ => unreachable!("expecting 'x', 'o' or 'b', but gets {}", ch), }); while let Some(next_char_in_escape_seq) = stream.peek_next() { @@ -1184,9 +1183,14 @@ fn get_next_token_inner( } // letter or underscore ... + #[cfg(not(feature = "unicode-xid-ident"))] ('A'..='Z', _) | ('a'..='z', _) | ('_', _) => { return get_identifier(stream, pos, start_pos, c); } + #[cfg(feature = "unicode-xid-ident")] + (ch, _) if unicode_xid::UnicodeXID::is_xid_start(ch) => { + return get_identifier(stream, pos, start_pos, c); + } // " - string literal ('"', _) => { @@ -1495,10 +1499,7 @@ fn get_next_token_inner( ('$', _) => return Some((Token::Reserved("$".into()), start_pos)), (ch, _) if ch.is_whitespace() => (), - #[cfg(feature = "unicode-xid-ident")] - (ch, _) if unicode_xid::UnicodeXID::is_xid_start(ch) => { - return get_identifier(stream, pos, start_pos, c); - } + (ch, _) => { return Some(( Token::LexError(LERR::UnexpectedInput(ch.to_string())), @@ -1683,8 +1684,8 @@ pub struct TokenIterator<'a, 'e> { pos: Position, /// Input character stream. stream: MultiInputsStream<'a>, - /// A processor function (if any) that maps a token to another. - map: Option Token>>, + /// A processor function that maps a token to another. + map: fn(Token) -> Token, } impl<'a> Iterator for TokenIterator<'a, '_> { @@ -1760,24 +1761,26 @@ impl<'a> Iterator for TokenIterator<'a, '_> { match token { None => None, - Some((token, pos)) => { - if let Some(ref map) = self.map { - Some((map(token), pos)) - } else { - Some((token, pos)) - } - } + Some((token, pos)) => Some(((self.map)(token), pos)), } } } impl Engine { /// Tokenize an input text stream. - #[inline] + #[inline(always)] pub fn lex<'a, 'e>( &'e self, input: impl IntoIterator, - map: Option Token>>, + ) -> TokenIterator<'a, 'e> { + self.lex_with_map(input, |f| f) + } + /// Tokenize an input text stream with a mapping function. + #[inline] + pub fn lex_with_map<'a, 'e>( + &'e self, + input: impl IntoIterator, + map: fn(Token) -> Token, ) -> TokenIterator<'a, 'e> { TokenIterator { engine: self, diff --git a/src/utils.rs b/src/utils.rs index 52dfda99..9043bb4a 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -54,6 +54,7 @@ impl BuildHasher for StraightHasherBuilder { } /// Create an instance of the default hasher. +#[inline(always)] pub fn get_hasher() -> impl Hasher { #[cfg(feature = "no_std")] let s: ahash::AHasher = Default::default(); diff --git a/tests/assignments.rs b/tests/assignments.rs new file mode 100644 index 00000000..d3936223 --- /dev/null +++ b/tests/assignments.rs @@ -0,0 +1,82 @@ +use rhai::{Engine, EvalAltResult, ParseErrorType, INT}; + +#[test] +fn test_assignments() -> Result<(), Box> { + let engine = Engine::new(); + + assert_eq!(engine.eval::("let x = 42; x = 123; x")?, 123); + assert_eq!(engine.eval::("let x = 42; x += 123; x")?, 165); + + #[cfg(not(feature = "no_index"))] + assert_eq!(engine.eval::("let x = [42]; x[0] += 123; x[0]")?, 165); + + #[cfg(not(feature = "no_object"))] + assert_eq!(engine.eval::("let x = #{a:42}; x.a += 123; x.a")?, 165); + + Ok(()) +} + +#[test] +fn test_assignments_bad_lhs() -> Result<(), Box> { + let engine = Engine::new(); + + assert_eq!( + *engine.compile(r"(x+y) = 42;").expect_err("should error").0, + ParseErrorType::AssignmentToInvalidLHS("".to_string()) + ); + assert_eq!( + *engine.compile(r"foo(x) = 42;").expect_err("should error").0, + ParseErrorType::AssignmentToInvalidLHS("".to_string()) + ); + assert_eq!( + *engine.compile(r"true = 42;").expect_err("should error").0, + ParseErrorType::AssignmentToConstant("".to_string()) + ); + assert_eq!( + *engine.compile(r"123 = 42;").expect_err("should error").0, + ParseErrorType::AssignmentToConstant("".to_string()) + ); + + #[cfg(not(feature = "no_object"))] + { + assert_eq!( + *engine + .compile(r"x.foo() = 42;") + .expect_err("should error") + .0, + ParseErrorType::AssignmentToInvalidLHS("".to_string()) + ); + assert_eq!( + *engine + .compile(r"x.foo().x.y = 42;") + .expect_err("should error") + .0, + ParseErrorType::AssignmentToInvalidLHS("".to_string()) + ); + assert_eq!( + *engine + .compile(r"x.y.z.foo() = 42;") + .expect_err("should error") + .0, + ParseErrorType::AssignmentToInvalidLHS("".to_string()) + ); + #[cfg(not(feature = "no_index"))] + assert_eq!( + *engine + .compile(r"x.foo()[0] = 42;") + .expect_err("should error") + .0, + ParseErrorType::AssignmentToInvalidLHS("".to_string()) + ); + #[cfg(not(feature = "no_index"))] + assert_eq!( + *engine + .compile(r"x[y].z.foo() = 42;") + .expect_err("should error") + .0, + ParseErrorType::AssignmentToInvalidLHS("".to_string()) + ); + } + + Ok(()) +} From 41c6f985f5376b29224237141f451058eb5ebe6a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 29 Dec 2020 11:37:15 +0800 Subject: [PATCH 2/3] Fix bug with tokenizing identifiers. --- src/token.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/token.rs b/src/token.rs index 752c1da4..54e1f1cc 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1115,7 +1115,7 @@ fn get_next_token_inner( } } // 0x????, 0o????, 0b???? - ch @ 'x' | ch @ 'X' | ch @ 'o' | ch @ 'O' | ch @ 'b' | ch @ 'B' + ch @ 'x' | ch @ 'o' | ch @ 'b' | ch @ 'X' | ch @ 'O' | ch @ 'B' if c == '0' => { result.push(next_char); @@ -1184,11 +1184,11 @@ fn get_next_token_inner( // letter or underscore ... #[cfg(not(feature = "unicode-xid-ident"))] - ('A'..='Z', _) | ('a'..='z', _) | ('_', _) => { + ('a'..='z', _) | ('_', _) | ('A'..='Z', _) => { return get_identifier(stream, pos, start_pos, c); } #[cfg(feature = "unicode-xid-ident")] - (ch, _) if unicode_xid::UnicodeXID::is_xid_start(ch) => { + (ch, _) if unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_' => { return get_identifier(stream, pos, start_pos, c); } From 13f5cec29169dcd7a57ff6e433ec1bc1c307619c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 29 Dec 2020 12:29:45 +0800 Subject: [PATCH 3/3] Fix call stack limits. --- RELEASES.md | 1 + src/engine.rs | 8 ++++-- src/engine_api.rs | 7 +++--- src/engine_settings.rs | 2 ++ src/fn_call.rs | 29 ++++++++++----------- src/module/mod.rs | 2 +- src/parser.rs | 57 ++++++++++++++++++++++-------------------- tests/stack.rs | 19 ++++++++------ 8 files changed, 70 insertions(+), 55 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 9530b5a7..ba43f472 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -16,6 +16,7 @@ Bug fixes * Fix bug when accessing properties in closures. * Fix bug when accessing a deep index with a function call. * Fix bug that sometimes allow assigning to an invalid l-value. +* Fix off-by-one error with `Engine::set_max_call_levels`. Breaking changes ---------------- diff --git a/src/engine.rs b/src/engine.rs index dae39f5f..bf36d028 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -170,6 +170,7 @@ impl FromIterator<(ImmutableString, Shared)> for Imports { #[cfg(not(feature = "unchecked"))] #[cfg(debug_assertions)] +#[cfg(not(feature = "no_function"))] pub const MAX_CALL_STACK_DEPTH: usize = 8; #[cfg(not(feature = "unchecked"))] #[cfg(debug_assertions)] @@ -181,6 +182,7 @@ pub const MAX_FUNCTION_EXPR_DEPTH: usize = 16; #[cfg(not(feature = "unchecked"))] #[cfg(not(debug_assertions))] +#[cfg(not(feature = "no_function"))] pub const MAX_CALL_STACK_DEPTH: usize = 128; #[cfg(not(feature = "unchecked"))] #[cfg(not(debug_assertions))] @@ -527,8 +529,8 @@ impl State { #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct Limits { /// Maximum levels of call-stack to prevent infinite recursion. - /// - /// Defaults to 16 for debug builds and 128 for non-debug builds. + /// Not available under `no_function`. + #[cfg(not(feature = "no_function"))] pub max_call_stack_depth: usize, /// Maximum depth of statements/expressions at global level (0 = unlimited). pub max_expr_depth: usize, @@ -809,6 +811,7 @@ impl Engine { #[cfg(not(feature = "unchecked"))] limits: Limits { + #[cfg(not(feature = "no_function"))] max_call_stack_depth: MAX_CALL_STACK_DEPTH, max_expr_depth: MAX_EXPR_DEPTH, #[cfg(not(feature = "no_function"))] @@ -864,6 +867,7 @@ impl Engine { #[cfg(not(feature = "unchecked"))] limits: Limits { + #[cfg(not(feature = "no_function"))] max_call_stack_depth: MAX_CALL_STACK_DEPTH, max_expr_depth: MAX_EXPR_DEPTH, #[cfg(not(feature = "no_function"))] diff --git a/src/engine_api.rs b/src/engine_api.rs index fff309b4..ff07e9e8 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1453,7 +1453,7 @@ impl Engine { ) -> Result> { let mods = &mut (&self.global_sub_modules).into(); - let result = self.eval_ast_with_scope_raw(scope, mods, ast)?; + let result = self.eval_ast_with_scope_raw(scope, mods, ast, 0)?; let typ = self.map_type_name(result.type_name()); @@ -1473,12 +1473,13 @@ impl Engine { scope: &mut Scope, mods: &mut Imports, ast: &'a AST, + level: usize, ) -> Result> { let state = &mut State { source: ast.clone_source(), ..Default::default() }; - self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()]) + self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()], level) } /// Evaluate a file, but throw away the result and only return error (if any). /// Useful for when you don't need the result, but still need to keep track of possible errors. @@ -1542,7 +1543,7 @@ impl Engine { source: ast.clone_source(), ..Default::default() }; - self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()])?; + self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()], 0)?; Ok(()) } /// Call a script function defined in an [`AST`] with multiple arguments. diff --git a/src/engine_settings.rs b/src/engine_settings.rs index 1a291ccc..cb60a6d0 100644 --- a/src/engine_settings.rs +++ b/src/engine_settings.rs @@ -38,6 +38,7 @@ impl Engine { /// Set the maximum levels of function calls allowed for a script in order to avoid /// infinite recursion and stack overflows. #[cfg(not(feature = "unchecked"))] + #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn set_max_call_levels(&mut self, levels: usize) -> &mut Self { self.limits.max_call_stack_depth = levels; @@ -45,6 +46,7 @@ impl Engine { } /// The maximum levels of function calls allowed for a script. #[cfg(not(feature = "unchecked"))] + #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn max_call_levels(&self) -> usize { self.limits.max_call_stack_depth diff --git a/src/fn_call.rs b/src/fn_call.rs index c0329828..38bdeff6 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -405,11 +405,11 @@ impl Engine { mods.extend(fn_def.mods.iter_raw().map(|(n, m)| (n.clone(), m.clone()))); } - // Evaluate the function at one higher level of call depth + // Evaluate the function let stmt = &fn_def.body; let result = self - .eval_stmt(scope, mods, state, unified_lib, this_ptr, stmt, level + 1) + .eval_stmt(scope, mods, state, unified_lib, this_ptr, stmt, level) .or_else(|err| match *err { // Convert return statement to return value EvalAltResult::Return(x, _) => Ok(x), @@ -586,6 +586,8 @@ impl Engine { mem::swap(&mut state.source, &mut source); + let level = _level + 1; + let result = self.call_script_fn( scope, mods, @@ -595,7 +597,7 @@ impl Engine { func, rest, pos, - _level, + level, ); // Restore the original source @@ -610,8 +612,10 @@ impl Engine { mem::swap(&mut state.source, &mut source); + let level = _level + 1; + let result = self.call_script_fn( - scope, mods, state, lib, &mut None, func, args, pos, _level, + scope, mods, state, lib, &mut None, func, args, pos, level, ); // Restore the original source @@ -667,11 +671,12 @@ impl Engine { state: &mut State, statements: impl IntoIterator, lib: &[&Module], + level: usize, ) -> Result> { statements .into_iter() .try_fold(().into(), |_, stmt| { - self.eval_stmt(scope, mods, state, lib, &mut None, stmt, 0) + self.eval_stmt(scope, mods, state, lib, &mut None, stmt, level) }) .or_else(|err| match *err { EvalAltResult::Return(out, _) => Ok(out), @@ -691,7 +696,7 @@ impl Engine { lib: &[&Module], script: &str, pos: Position, - _level: usize, + level: usize, ) -> Result> { self.inc_operations(state, pos)?; @@ -700,13 +705,6 @@ impl Engine { return Ok(Dynamic::UNIT); } - // Check for stack overflow - #[cfg(not(feature = "no_function"))] - #[cfg(not(feature = "unchecked"))] - if _level > self.max_call_levels() { - return Err(Box::new(EvalAltResult::ErrorStackOverflow(pos))); - } - // Compile the script text // No optimizations because we only run it once let ast = self.compile_with_scope_and_optimization_level( @@ -727,7 +725,8 @@ impl Engine { ..Default::default() }; - let result = self.eval_statements_raw(scope, mods, &mut new_state, ast.statements(), lib); + let result = + self.eval_statements_raw(scope, mods, &mut new_state, ast.statements(), lib, level); state.operations = new_state.operations; result @@ -1208,6 +1207,8 @@ impl Engine { let mut source = module.id_raw().clone(); mem::swap(&mut state.source, &mut source); + let level = level + 1; + let result = self.call_script_fn( new_scope, mods, state, lib, &mut None, &fn_def, args, pos, level, ); diff --git a/src/module/mod.rs b/src/module/mod.rs index d1c023e5..c90775a2 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1720,7 +1720,7 @@ impl Module { let orig_mods_len = mods.len(); // Run the script - engine.eval_ast_with_scope_raw(&mut scope, &mut mods, &ast)?; + engine.eval_ast_with_scope_raw(&mut scope, &mut mods, &ast, 0)?; // Create new module let mut module = Module::new(); diff --git a/src/parser.rs b/src/parser.rs index b1c39d36..ca546ac9 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -19,7 +19,7 @@ use crate::stdlib::{ vec::Vec, }; use crate::syntax::{CustomSyntax, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; -use crate::token::{is_doc_comment, is_keyword_function, is_valid_identifier, Token, TokenStream}; +use crate::token::{is_keyword_function, is_valid_identifier, Token, TokenStream}; use crate::utils::{get_hasher, StraightHasherBuilder}; use crate::{ calc_script_fn_hash, Dynamic, Engine, ImmutableString, LexError, ParseError, ParseErrorType, @@ -2534,35 +2534,38 @@ fn parse_stmt( ) -> Result { use AccessMode::{ReadOnly, ReadWrite}; - let mut comments: Vec = Default::default(); - let mut comments_pos = Position::NONE; + let mut _comments: Vec = Default::default(); - // Handle doc-comments. #[cfg(not(feature = "no_function"))] - while let (Token::Comment(ref comment), comment_pos) = input.peek().unwrap() { - if comments_pos.is_none() { - comments_pos = *comment_pos; - } + { + let mut comments_pos = Position::NONE; - if !is_doc_comment(comment) { - unreachable!("expecting doc-comment, but gets {:?}", comment); - } - - if !settings.is_global { - return Err(PERR::WrongDocComment.into_err(comments_pos)); - } - - match input.next().unwrap().0 { - Token::Comment(comment) => { - comments.push(comment); - - match input.peek().unwrap() { - (Token::Fn, _) | (Token::Private, _) => break, - (Token::Comment(_), _) => (), - _ => return Err(PERR::WrongDocComment.into_err(comments_pos)), - } + // Handle doc-comments. + while let (Token::Comment(ref comment), pos) = input.peek().unwrap() { + if comments_pos.is_none() { + comments_pos = *pos; + } + + if !crate::token::is_doc_comment(comment) { + unreachable!("expecting doc-comment, but gets {:?}", comment); + } + + if !settings.is_global { + return Err(PERR::WrongDocComment.into_err(comments_pos)); + } + + match input.next().unwrap().0 { + Token::Comment(comment) => { + _comments.push(comment); + + match input.peek().unwrap() { + (Token::Fn, _) | (Token::Private, _) => break, + (Token::Comment(_), _) => (), + _ => return Err(PERR::WrongDocComment.into_err(comments_pos)), + } + } + t => unreachable!("expecting Token::Comment, but gets {:?}", t), } - t => unreachable!("expecting Token::Comment, but gets {:?}", t), } } @@ -2618,7 +2621,7 @@ fn parse_stmt( pos: pos, }; - let func = parse_fn(input, &mut new_state, lib, access, settings, comments)?; + let func = parse_fn(input, &mut new_state, lib, access, settings, _comments)?; lib.insert( // Qualifiers (none) + function name + number of arguments. diff --git a/tests/stack.rs b/tests/stack.rs index 3319a55c..03f1c083 100644 --- a/tests/stack.rs +++ b/tests/stack.rs @@ -10,21 +10,24 @@ fn test_stack_overflow_fn_calls() -> Result<(), Box> { engine.eval::( r" fn foo(n) { if n <= 1 { 0 } else { n + foo(n-1) } } - foo(7) - ", + foo(6) + ", )?, - 27 + 20 ); + let max = engine.max_call_levels(); + #[cfg(not(feature = "unchecked"))] assert!(matches!( *engine - .eval::<()>( + .eval::<()>(&format!( r" - fn foo(n) { if n == 0 { 0 } else { n + foo(n-1) } } - foo(1000) - " - ) + fn foo(n) {{ if n == 0 {{ 0 }} else {{ n + foo(n-1) }} }} + foo({}) + ", + max + 1 + )) .expect_err("should error"), EvalAltResult::ErrorStackOverflow(_) ));