diff --git a/RELEASES.md b/RELEASES.md index 2d23c944..c647dcd0 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,6 +4,12 @@ Rhai Release Notes Version 0.19.12 =============== +Bug fixes +--------- + +* Empty statements (i.e. statements with only one `;`) now parse correctly and no longer hang. +* `continue`, `break` and `return` statements no longer panic inside a `try .. catch` block. + Breaking changes ---------------- diff --git a/codegen/ui_tests/export_fn_raw_return.stderr b/codegen/ui_tests/export_fn_raw_return.stderr index 7fbb58dc..65ebcd8b 100644 --- a/codegen/ui_tests/export_fn_raw_return.stderr +++ b/codegen/ui_tests/export_fn_raw_return.stderr @@ -2,9 +2,9 @@ error[E0308]: mismatched types --> $DIR/export_fn_raw_return.rs:10:8 | 9 | #[export_fn(return_raw)] - | ------------------------ expected `std::result::Result>` because of return type + | ------------------------ expected `Result>` because of return type 10 | pub fn test_fn(input: Point) -> bool { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found `bool` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `Result`, found `bool` | - = note: expected enum `std::result::Result>` + = note: expected enum `Result>` found type `bool` diff --git a/codegen/ui_tests/export_mod_raw_return.stderr b/codegen/ui_tests/export_mod_raw_return.stderr index 7d22f7a2..b2e2668f 100644 --- a/codegen/ui_tests/export_mod_raw_return.stderr +++ b/codegen/ui_tests/export_mod_raw_return.stderr @@ -2,10 +2,10 @@ error[E0308]: mismatched types --> $DIR/export_mod_raw_return.rs:12:8 | 9 | #[export_module] - | ---------------- expected `std::result::Result>` because of return type + | ---------------- expected `Result>` because of return type ... 12 | pub fn test_fn(input: Point) -> bool { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found `bool` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `Result`, found `bool` | - = note: expected enum `std::result::Result>` + = note: expected enum `Result>` found type `bool` diff --git a/src/engine.rs b/src/engine.rs index 2b0e6569..b079ae9a 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -519,8 +519,8 @@ pub struct State { /// Embedded module resolver. #[cfg(not(feature = "no_module"))] pub resolver: Option>, - /// Cached lookup values for function hashes. - pub functions_caches: StaticVec< + /// Functions resolution cache. + fn_resolution_caches: StaticVec< HashMap< NonZeroU64, Option<(CallableFunction, Option)>, @@ -535,6 +535,48 @@ impl State { pub fn is_global(&self) -> bool { self.scope_level == 0 } + /// Get the current functions resolution cache. + pub fn fn_resolution_cache( + &self, + ) -> Option< + &HashMap< + NonZeroU64, + Option<(CallableFunction, Option)>, + StraightHasherBuilder, + >, + > { + self.fn_resolution_caches.last() + } + /// Get a mutable reference to the current functions resolution cache. + pub fn fn_resolution_cache_mut( + &mut self, + ) -> &mut HashMap< + NonZeroU64, + Option<(CallableFunction, Option)>, + StraightHasherBuilder, + > { + if self.fn_resolution_caches.is_empty() { + self.fn_resolution_caches + .push(HashMap::with_capacity_and_hasher(16, StraightHasherBuilder)); + } + self.fn_resolution_caches.last_mut().unwrap() + } + /// Push an empty functions resolution cache onto the stack and make it current. + pub fn push_fn_resolution_cache(&mut self) { + self.fn_resolution_caches.push(Default::default()); + } + /// Remove the current functions resolution cache and make the last one current. + pub fn pop_fn_resolution_cache(&mut self) { + self.fn_resolution_caches.pop(); + } + /// Clear the current functions resolution cache. + /// + /// # Panics + /// + /// Panics if there is no current functions resolution cache. + pub fn clear_fn_resolution_cache(&mut self) { + self.fn_resolution_caches.last_mut().unwrap().clear(); + } } /// _(INTERNALS)_ A type containing all the limits imposed by the [`Engine`]. @@ -1712,7 +1754,7 @@ impl Engine { // Statement block Expr::Stmt(x, _) => { - self.eval_stmt_block(scope, mods, state, lib, this_ptr, x.as_ref(), level) + self.eval_stmt_block(scope, mods, state, lib, this_ptr, x.as_ref(), true, level) } // lhs[idx_expr] @@ -1856,45 +1898,49 @@ impl Engine { lib: &[&Module], this_ptr: &mut Option<&mut Dynamic>, statements: impl IntoIterator, + restore: bool, level: usize, ) -> Result> { - let mut has_imports = false; + let mut _has_imports = false; let prev_always_search = state.always_search; let prev_scope_len = scope.len(); let prev_mods_len = mods.len(); - state.scope_level += 1; - let result = statements - .into_iter() - .try_fold(Default::default(), |_, stmt| { - match stmt { - #[cfg(not(feature = "no_module"))] - Stmt::Import(_, _, _) => { - // When imports list is modified, clear the functions lookup cache - if has_imports { - state.functions_caches.last_mut().map(|c| c.clear()); - } else { - state.functions_caches.push(Default::default()); - } - has_imports = true; - } - _ => (), - } - - self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level) - }); - - scope.rewind(prev_scope_len); - if has_imports { - // If imports list is modified, pop the functions lookup cache - state.functions_caches.pop(); + if restore { + state.scope_level += 1; } - mods.truncate(prev_mods_len); - state.scope_level -= 1; - // The impact of new local variables goes away at the end of a block - // because any new variables introduced will go out of scope - state.always_search = prev_always_search; + let result = statements.into_iter().try_fold(Dynamic::UNIT, |_, stmt| { + #[cfg(not(feature = "no_module"))] + match stmt { + Stmt::Import(_, _, _) => { + // When imports list is modified, clear the functions lookup cache + if _has_imports { + state.clear_fn_resolution_cache(); + } else if restore { + state.push_fn_resolution_cache(); + _has_imports = true; + } + } + _ => (), + } + + self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level) + }); + + if restore { + scope.rewind(prev_scope_len); + if _has_imports { + // If imports list is modified, pop the functions lookup cache + state.pop_fn_resolution_cache(); + } + mods.truncate(prev_mods_len); + state.scope_level -= 1; + + // The impact of new local variables goes away at the end of a block + // because any new variables introduced will go out of scope + state.always_search = prev_always_search; + } result } @@ -2088,7 +2134,7 @@ impl Engine { // Block scope Stmt::Block(statements, _) => { - self.eval_stmt_block(scope, mods, state, lib, this_ptr, statements, level) + self.eval_stmt_block(scope, mods, state, lib, this_ptr, statements, true, level) } // If statement @@ -2246,6 +2292,7 @@ impl Engine { match result { Ok(_) => result, + Err(err) if err.is_pseudo_error() => Err(err), Err(err) if !err.is_catchable() => Err(err), Err(mut err) => { let value = match *err { diff --git a/src/engine_api.rs b/src/engine_api.rs index b861c06c..ebecaa40 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1527,13 +1527,16 @@ impl Engine { ast: &'a AST, level: usize, ) -> Result> { - let state = &mut State { - source: ast.clone_source(), - #[cfg(not(feature = "no_module"))] - resolver: ast.resolver(), - ..Default::default() - }; - self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()], level) + let mut state: State = Default::default(); + state.source = ast.clone_source(); + #[cfg(not(feature = "no_module"))] + { + state.resolver = ast.resolver(); + } + + let statements = ast.statements(); + let lib = &[ast.lib()]; + self.eval_global_statements(scope, mods, &mut state, statements, 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. @@ -1596,13 +1599,15 @@ impl Engine { ast: &AST, ) -> Result<(), Box> { let mods = &mut (&self.global_sub_modules).into(); - let state = &mut State { - source: ast.clone_source(), - #[cfg(not(feature = "no_module"))] - resolver: ast.resolver(), - ..Default::default() - }; - self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()], 0)?; + let mut state: State = Default::default(); + state.source = ast.clone_source(); + #[cfg(not(feature = "no_module"))] + { + state.resolver = ast.resolver(); + } + let statements = ast.statements(); + let lib = &[ast.lib()]; + self.eval_global_statements(scope, mods, &mut state, statements, lib, 0)?; Ok(()) } /// Call a script function defined in an [`AST`] with multiple arguments. diff --git a/src/fn_call.rs b/src/fn_call.rs index 15307458..ffe88602 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -175,15 +175,11 @@ impl Engine { ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state, pos)?; - // Check if function access already in the cache - if state.functions_caches.is_empty() { - state.functions_caches.push(Default::default()); - } + let source = state.source.clone(); + // Check if function access already in the cache let func = &*state - .functions_caches - .last_mut() - .unwrap() + .fn_resolution_cache_mut() .entry(hash_fn) .or_insert_with(|| { // Search for the native function @@ -208,7 +204,7 @@ impl Engine { }) }); - if let Some((func, source)) = func { + if let Some((func, src)) = func { assert!(func.is_native()); // Calling pure function but the first argument is a reference? @@ -216,12 +212,7 @@ impl Engine { backup.change_first_arg_to_copy(is_ref && func.is_pure(), args); // Run external function - let source = if source.is_none() { - state.source.as_ref() - } else { - source.as_ref() - } - .map(|s| s.as_str()); + let source = src.as_ref().or_else(|| source.as_ref()).map(|s| s.as_str()); let result = if func.is_plugin_fn() { func.get_plugin_fn() .call((self, fn_name, source, mods, lib).into(), args) @@ -405,7 +396,7 @@ impl Engine { let unified_lib = if let Some(ref env_lib) = fn_def.lib { unified = true; - state.functions_caches.push(Default::default()); + state.push_fn_resolution_cache(); lib_merged = Default::default(); lib_merged.push(env_lib.as_ref()); lib_merged.extend(lib.iter().cloned()); @@ -477,7 +468,7 @@ impl Engine { state.scope_level = orig_scope_level; if unified { - state.functions_caches.pop(); + state.pop_fn_resolution_cache(); } result @@ -515,13 +506,13 @@ impl Engine { // Check if it is already in the cache if let Some(state) = state.as_mut() { if let Some(hash) = hash_script { - match state.functions_caches.last().map_or(None, |c| c.get(&hash)) { + match state.fn_resolution_cache().map_or(None, |c| c.get(&hash)) { Some(v) => return v.is_some(), None => (), } } if let Some(hash) = hash_fn { - match state.functions_caches.last().map_or(None, |c| c.get(&hash)) { + match state.fn_resolution_cache().map_or(None, |c| c.get(&hash)) { Some(v) => return v.is_some(), None => (), } @@ -545,24 +536,10 @@ impl Engine { if !r { if let Some(state) = state.as_mut() { if let Some(hash) = hash_script { - if state.functions_caches.is_empty() { - state.functions_caches.push(Default::default()); - } - state - .functions_caches - .last_mut() - .unwrap() - .insert(hash, None); + state.fn_resolution_cache_mut().insert(hash, None); } if let Some(hash) = hash_fn { - if state.functions_caches.is_empty() { - state.functions_caches.push(Default::default()); - } - state - .functions_caches - .last_mut() - .unwrap() - .insert(hash, None); + state.fn_resolution_cache_mut().insert(hash, None); } } } @@ -653,14 +630,8 @@ impl Engine { let hash_script = hash_script.unwrap(); // Check if function access already in the cache - if state.functions_caches.is_empty() { - state.functions_caches.push(Default::default()); - } - let (func, source) = state - .functions_caches - .last_mut() - .unwrap() + .fn_resolution_cache_mut() .entry(hash_script) .or_insert_with(|| { lib.iter() @@ -768,10 +739,10 @@ impl Engine { } } - /// Evaluate a list of statements with an empty state and no `this` pointer. + /// Evaluate a list of statements with no `this` pointer. /// This is commonly used to evaluate a list of statements in an [`AST`] or a script function body. #[inline] - pub(crate) fn eval_statements_raw<'a>( + pub(crate) fn eval_global_statements<'a>( &self, scope: &mut Scope, mods: &mut Imports, @@ -780,11 +751,7 @@ impl Engine { lib: &[&Module], level: usize, ) -> Result> { - statements - .into_iter() - .try_fold(Dynamic::UNIT, |_, stmt| { - self.eval_stmt(scope, mods, state, lib, &mut None, stmt, level) - }) + self.eval_stmt_block(scope, mods, state, lib, &mut None, statements, false, level) .or_else(|err| match *err { EvalAltResult::Return(out, _) => Ok(out), EvalAltResult::LoopBreak(_, _) => { @@ -826,14 +793,12 @@ impl Engine { } // Evaluate the AST - let mut new_state = State { - source: state.source.clone(), - operations: state.operations, - ..Default::default() - }; + let mut new_state: State = Default::default(); + new_state.source = state.source.clone(); + new_state.operations = state.operations; let result = - self.eval_statements_raw(scope, mods, &mut new_state, ast.statements(), lib, level); + self.eval_global_statements(scope, mods, &mut new_state, ast.statements(), lib, level); state.operations = new_state.operations; result diff --git a/src/module/mod.rs b/src/module/mod.rs index 35e12393..181d4f3b 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -374,7 +374,7 @@ impl Module { /// Generate signatures for all the functions in the [`Module`]. #[inline(always)] - pub fn gen_fn_signatures<'a>(&'a self) -> impl Iterator + 'a { + pub fn gen_fn_signatures(&self) -> impl Iterator + '_ { self.functions .values() .filter(|FuncInfo { access, .. }| !access.is_private()) @@ -1691,9 +1691,9 @@ impl Module { /// 5) Shared reference to function definition [`ScriptFnDef`][crate::ast::ScriptFnDef]. #[cfg(not(feature = "no_function"))] #[inline(always)] - pub(crate) fn iter_script_fn<'a>( - &'a self, - ) -> impl Iterator + 'a { + pub(crate) fn iter_script_fn( + &self, + ) -> impl Iterator + '_ { self.functions.values().filter(|f| f.func.is_script()).map( |FuncInfo { namespace, diff --git a/src/parser.rs b/src/parser.rs index dd4b5812..eb40b99f 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2571,7 +2571,10 @@ fn parse_stmt( match token { // ; - empty statement - Token::SemiColon => Ok(Stmt::Noop(settings.pos)), + Token::SemiColon => { + eat_token(input, Token::SemiColon); + Ok(Stmt::Noop(settings.pos)) + } // { - statements block Token::LeftBrace => Ok(parse_block(input, state, lib, settings.level_up())?), diff --git a/src/result.rs b/src/result.rs index 39a0b315..8fa83ce6 100644 --- a/src/result.rs +++ b/src/result.rs @@ -272,6 +272,15 @@ impl> From for Box { } impl EvalAltResult { + /// Is this a pseudo error? A pseudo error is one that does not occur naturally. + /// + /// [`LoopBreak`][EvalAltResult::LoopBreak] or [`Return`][EvalAltResult::Return] are pseudo errors. + pub fn is_pseudo_error(&self) -> bool { + match self { + Self::LoopBreak(_, _) | Self::Return(_, _) => true, + _ => false, + } + } /// Can this error be caught? /// /// # Panics diff --git a/src/scope.rs b/src/scope.rs index d60aab40..77c74e9e 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -449,14 +449,14 @@ impl<'a> Scope<'a> { /// /// let mut iter = my_scope.iter(); /// - /// let (name, constant, value) = iter.next().unwrap(); + /// let (name, is_constant, value) = iter.next().unwrap(); /// assert_eq!(name, "x"); - /// assert!(!constant); + /// assert!(!is_constant); /// assert_eq!(value.cast::(), 42); /// - /// let (name, constant, value) = iter.next().unwrap(); + /// let (name, is_constant, value) = iter.next().unwrap(); /// assert_eq!(name, "foo"); - /// assert!(constant); + /// assert!(is_constant); /// assert_eq!(value.cast::(), "hello"); /// ``` #[inline(always)] @@ -467,7 +467,7 @@ impl<'a> Scope<'a> { /// Get an iterator to entries in the [`Scope`]. /// Shared values are not expanded. #[inline(always)] - pub fn iter_raw<'x: 'a>(&'x self) -> impl Iterator + 'x { + pub fn iter_raw(&self) -> impl Iterator { self.names .iter() .zip(self.values.iter()) diff --git a/tests/eval.rs b/tests/eval.rs index 568a5e5b..c59ccfff 100644 --- a/tests/eval.rs +++ b/tests/eval.rs @@ -4,14 +4,7 @@ use rhai::{Engine, EvalAltResult, LexError, ParseErrorType, RegisterFn, Scope, I fn test_eval() -> Result<(), Box> { let engine = Engine::new(); - assert_eq!( - engine.eval::( - r#" - eval("40 + 2") - "# - )?, - 42 - ); + assert_eq!(engine.eval::(r#"eval("40 + 2")"#)?, 42); Ok(()) } @@ -62,7 +55,7 @@ fn test_eval_function() -> Result<(), Box> { script += "x + y"; eval(script) + x + y - "# + "# )?, 84 );