From 6ce1dae11046d816f67eb7a812fe6777e6ea05d2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 10:27:39 +0800 Subject: [PATCH 01/11] Simplify integer bits iterator. --- src/packages/iter_basic.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index 4473cfbb..163f93a4 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -122,7 +122,7 @@ impl FusedIterator for StepRange {} // Bit-field iterator with step #[derive(Debug, Clone, Hash, Eq, PartialEq)] -pub struct BitRange(INT, INT, usize); +pub struct BitRange(INT, usize); impl BitRange { pub fn new(value: INT, from: INT, len: INT) -> RhaiResultOf { @@ -138,7 +138,7 @@ impl BitRange { len as usize }; - Ok(Self(value, 1 << from, len)) + Ok(Self(value >> from, len)) } } @@ -146,19 +146,19 @@ impl Iterator for BitRange { type Item = bool; fn next(&mut self) -> Option { - if self.2 == 0 { + if self.1 == 0 { None } else { - let r = (self.0 & self.1) != 0; - self.1 <<= 1; - self.2 -= 1; + let r = (self.0 & 0x0001) != 0; + self.0 >>= 1; + self.1 -= 1; Some(r) } } #[inline(always)] fn size_hint(&self) -> (usize, Option) { - (self.2, Some(self.2)) + (self.1, Some(self.1)) } } @@ -167,7 +167,7 @@ impl FusedIterator for BitRange {} impl ExactSizeIterator for BitRange { #[inline(always)] fn len(&self) -> usize { - self.2 + self.1 } } From 6af66d3ed384b222148d66938c4b3e2b9022cc2a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 10:40:07 +0800 Subject: [PATCH 02/11] Code cleanup. --- src/ast/expr.rs | 2 +- src/bin/rhai-repl.rs | 8 +------ src/eval/chaining.rs | 48 +++++++++++++++++++------------------- src/eval/data_check.rs | 1 + src/packages/time_basic.rs | 2 +- 5 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 7a9e1cae..88bac79b 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -609,7 +609,7 @@ impl Expr { let mut s = SmartString::new_const(); for segment in x.iter() { let v = segment.get_literal_value().unwrap(); - write!(&mut s, "{}", v).unwrap(); + write!(&mut s, "{v}").unwrap(); } s.into() } diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index ea452269..71801d82 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -309,13 +309,7 @@ fn main() { } // Register sample functions - engine - .register_global_module(exported_module!(sample_functions).into()) - .register_get_set( - "test", - |x: &mut INT| *x % 2 == 0, - |x: &mut INT, y: bool| if y { *x *= 2 } else { *x /= 2 }, - ); + engine.register_global_module(exported_module!(sample_functions).into()); // Create scope let mut scope = Scope::new(); diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 78acb0ff..dbf40e3a 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -45,8 +45,8 @@ impl Engine { target: &mut Target, root: (&str, Position), _parent: &Expr, + parent_options: ASTFlags, rhs: &Expr, - _parent_options: ASTFlags, idx_values: &mut FnArgsVec, chain_type: ChainType, level: usize, @@ -61,7 +61,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] ChainType::Indexing => { // Check for existence with the null conditional operator - if _parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() { + if parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() { return Ok((Dynamic::UNIT, false)); } @@ -70,7 +70,7 @@ impl Engine { match rhs { // xxx[idx].expr... | xxx[idx][expr]... Expr::Dot(x, options, x_pos) | Expr::Index(x, options, x_pos) - if !_parent_options.contains(ASTFlags::BREAK) => + if !parent_options.contains(ASTFlags::BREAK) => { #[cfg(feature = "debugging")] self.run_debugger(scope, global, lib, this_ptr, _parent, level)?; @@ -88,8 +88,8 @@ impl Engine { let obj_ptr = &mut obj; match self.eval_dot_index_chain_helper( - global, caches, lib, this_ptr, obj_ptr, root, rhs, &x.rhs, - *options, idx_values, rhs_chain, level, new_val, + global, caches, lib, this_ptr, obj_ptr, root, rhs, *options, + &x.rhs, idx_values, rhs_chain, level, new_val, ) { Ok((result, true)) if is_obj_temp_val => { (Some(obj.take_or_clone()), (result, true)) @@ -190,7 +190,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] ChainType::Dotting => { // Check for existence with the Elvis operator - if _parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() { + if parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() { return Ok((Dynamic::UNIT, false)); } @@ -407,7 +407,7 @@ impl Engine { let rhs_chain = rhs.into(); self.eval_dot_index_chain_helper( - global, caches, lib, this_ptr, val_target, root, rhs, &x.rhs, *options, + global, caches, lib, this_ptr, val_target, root, rhs, *options, &x.rhs, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*x_pos)) @@ -455,8 +455,8 @@ impl Engine { let (result, may_be_changed) = self .eval_dot_index_chain_helper( - global, caches, lib, this_ptr, val, root, rhs, &x.rhs, - *options, idx_values, rhs_chain, level, new_val, + global, caches, lib, this_ptr, val, root, rhs, *options, + &x.rhs, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*x_pos))?; @@ -525,8 +525,8 @@ impl Engine { let val = &mut val.into(); self.eval_dot_index_chain_helper( - global, caches, lib, this_ptr, val, root, rhs, &x.rhs, - *options, idx_values, rhs_chain, level, new_val, + global, caches, lib, this_ptr, val, root, rhs, *options, + &x.rhs, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(pos)) } @@ -612,7 +612,7 @@ impl Engine { let root = (x.3.as_str(), *var_pos); self.eval_dot_index_chain_helper( - global, caches, lib, &mut None, obj_ptr, root, expr, rhs, options, idx_values, + global, caches, lib, &mut None, obj_ptr, root, expr, options, rhs, idx_values, chain_type, level, new_val, ) } @@ -627,7 +627,7 @@ impl Engine { let root = ("", expr.start_position()); self.eval_dot_index_chain_helper( - global, caches, lib, this_ptr, obj_ptr, root, expr, rhs, options, idx_values, + global, caches, lib, this_ptr, obj_ptr, root, expr, options, rhs, idx_values, chain_type, level, new_val, ) } @@ -646,7 +646,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, expr: &Expr, parent_options: ASTFlags, - _parent_chain_type: ChainType, + parent_chain_type: ChainType, idx_values: &mut FnArgsVec, level: usize, ) -> RhaiResultOf<()> { @@ -655,7 +655,7 @@ impl Engine { match expr { #[cfg(not(feature = "no_object"))] Expr::MethodCall(x, ..) - if _parent_chain_type == ChainType::Dotting && !x.is_qualified() => + if parent_chain_type == ChainType::Dotting && !x.is_qualified() => { for arg_expr in &x.args { idx_values.push( @@ -666,12 +666,12 @@ impl Engine { } } #[cfg(not(feature = "no_object"))] - Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => { + Expr::MethodCall(..) if parent_chain_type == ChainType::Dotting => { unreachable!("function call in dot chain should not be namespace-qualified") } #[cfg(not(feature = "no_object"))] - Expr::Property(..) if _parent_chain_type == ChainType::Dotting => (), + Expr::Property(..) if parent_chain_type == ChainType::Dotting => (), Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), Expr::Index(x, options, ..) | Expr::Dot(x, options, ..) @@ -684,12 +684,12 @@ impl Engine { // Evaluate in left-to-right order match lhs { #[cfg(not(feature = "no_object"))] - Expr::Property(..) if _parent_chain_type == ChainType::Dotting => (), + Expr::Property(..) if parent_chain_type == ChainType::Dotting => (), Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"), #[cfg(not(feature = "no_object"))] Expr::MethodCall(x, ..) - if _parent_chain_type == ChainType::Dotting && !x.is_qualified() => + if parent_chain_type == ChainType::Dotting && !x.is_qualified() => { for arg_expr in &x.args { _arg_values.push( @@ -702,15 +702,15 @@ impl Engine { } } #[cfg(not(feature = "no_object"))] - Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => { + Expr::MethodCall(..) if parent_chain_type == ChainType::Dotting => { unreachable!("function call in dot chain should not be namespace-qualified") } #[cfg(not(feature = "no_object"))] - expr if _parent_chain_type == ChainType::Dotting => { + expr if parent_chain_type == ChainType::Dotting => { unreachable!("invalid dot expression: {:?}", expr); } #[cfg(not(feature = "no_index"))] - _ if _parent_chain_type == ChainType::Indexing => { + _ if parent_chain_type == ChainType::Indexing => { _arg_values.push( self.eval_expr(scope, global, caches, lib, this_ptr, lhs, level)? .flatten(), @@ -733,11 +733,11 @@ impl Engine { } #[cfg(not(feature = "no_object"))] - _ if _parent_chain_type == ChainType::Dotting => { + _ if parent_chain_type == ChainType::Dotting => { unreachable!("invalid dot expression: {:?}", expr); } #[cfg(not(feature = "no_index"))] - _ if _parent_chain_type == ChainType::Indexing => idx_values.push( + _ if parent_chain_type == ChainType::Indexing => idx_values.push( self.eval_expr(scope, global, caches, lib, this_ptr, expr, level)? .flatten(), ), diff --git a/src/eval/data_check.rs b/src/eval/data_check.rs index 9195f632..abda0e45 100644 --- a/src/eval/data_check.rs +++ b/src/eval/data_check.rs @@ -110,6 +110,7 @@ impl Engine { } /// Check whether the size of a [`Dynamic`] is within limits. + #[inline] pub(crate) fn check_data_size(&self, value: &Dynamic, pos: Position) -> RhaiResultOf<()> { // If no data size limits, just return if !self.has_data_size_limit() { diff --git a/src/packages/time_basic.rs b/src/packages/time_basic.rs index cf7f3dc4..59e198b8 100644 --- a/src/packages/time_basic.rs +++ b/src/packages/time_basic.rs @@ -2,7 +2,7 @@ use super::arithmetic::make_err as make_arithmetic_err; use crate::plugin::*; -use crate::{def_package, Dynamic, EvalAltResult, RhaiResult, RhaiResultOf, INT}; +use crate::{def_package, Dynamic, RhaiResult, RhaiResultOf, INT}; #[cfg(not(feature = "no_float"))] use crate::FLOAT; From c14fbdb14da7e16bef240496335d4c0cc1425ca5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 12:09:18 +0800 Subject: [PATCH 03/11] Add loop expressions. --- CHANGELOG.md | 7 +++++++ src/api/options.rs | 28 ++++++++++++++++++------- src/ast/stmt.rs | 6 +++--- src/eval/stmt.rs | 29 ++++++++++++++++---------- src/optimizer.rs | 49 ++++---------------------------------------- src/parser.rs | 38 +++++++++++++++++++++++++++++++--- src/types/error.rs | 2 +- tests/expressions.rs | 9 +++++--- tests/for.rs | 16 +++++++++++++++ tests/while_loop.rs | 35 +++++++++++++++++++++++++++++++ 10 files changed, 146 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49aa5966..b6f6d5c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Bug fixes * `Engine::parse_json` now returns an error on unquoted keys to be consistent with JSON specifications. * `import` statements inside `eval` no longer cause errors in subsequent code. * Functions marked `global` in `import`ed modules with no alias names now work properly. +* Incorrect loop optimizations that are too aggressive (e.g. unrolling a `do { ... } until true` with a `break` statement inside) and cause crashes are removed. Speed Improvements ------------------ @@ -19,6 +20,12 @@ Speed Improvements New features ------------ +### Loop expressions + +* Loops (such as `loop`, `do`, `while` and `for`) can now act as _expressions_, with the `break` statement returning an optional value. +* Normal loops return `()` as the value. +* Loop expressions can be enabled/disabled via `Engine::set_allow_loop_expressions` + ### Stable hashing * It is now possible to specify a fixed _seed_ for use with the `ahash` hasher, via an environment variable, in order to force stable (i.e. deterministic) hashes for function signatures. diff --git a/src/api/options.rs b/src/api/options.rs index d5596cfd..42b57456 100644 --- a/src/api/options.rs +++ b/src/api/options.rs @@ -12,23 +12,25 @@ bitflags! { const IF_EXPR = 0b_0000_0000_0001; /// Is `switch` expression allowed? const SWITCH_EXPR = 0b_0000_0000_0010; + /// Are loop expressions allowed? + const LOOP_EXPR = 0b_0000_0000_0100; /// Is statement-expression allowed? - const STMT_EXPR = 0b_0000_0000_0100; + const STMT_EXPR = 0b_0000_0000_1000; /// Is anonymous function allowed? #[cfg(not(feature = "no_function"))] - const ANON_FN = 0b_0000_0000_1000; + const ANON_FN = 0b_0000_0001_0000; /// Is looping allowed? - const LOOPING = 0b_0000_0001_0000; + const LOOPING = 0b_0000_0010_0000; /// Is variables shadowing allowed? - const SHADOW = 0b_0000_0010_0000; + const SHADOW = 0b_0000_0100_0000; /// Strict variables mode? - const STRICT_VAR = 0b_0000_0100_0000; + const STRICT_VAR = 0b_0000_1000_0000; /// Raise error if an object map property does not exist? /// Returns `()` if `false`. #[cfg(not(feature = "no_object"))] - const FAIL_ON_INVALID_MAP_PROPERTY = 0b_0000_1000_0000; + const FAIL_ON_INVALID_MAP_PROPERTY = 0b_0001_0000_0000; /// Fast operators mode? - const FAST_OPS = 0b_0001_0000_0000; + const FAST_OPS = 0b_0010_0000_0000; } } @@ -81,6 +83,18 @@ impl Engine { pub fn set_allow_switch_expression(&mut self, enable: bool) { self.options.set(LangOptions::SWITCH_EXPR, enable); } + /// Are loop expressions allowed? + /// Default is `true`. + #[inline(always)] + #[must_use] + pub const fn allow_loop_expressions(&self) -> bool { + self.options.contains(LangOptions::LOOP_EXPR) + } + /// Set whether loop expressions are allowed. + #[inline(always)] + pub fn set_allow_loop_expressions(&mut self, enable: bool) { + self.options.set(LangOptions::LOOP_EXPR, enable); + } /// Is statement-expression allowed? /// Default is `true`. #[inline(always)] diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 284d0133..ed6c51a3 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -581,14 +581,14 @@ pub enum Stmt { TryCatch(Box, Position), /// [expression][Expr] Expr(Box), - /// `continue`/`break` + /// `continue`/`break` expr /// /// ### Flags /// /// * [`NONE`][ASTFlags::NONE] = `continue` /// * [`BREAK`][ASTFlags::BREAK] = `break` - BreakLoop(ASTFlags, Position), - /// `return`/`throw` + BreakLoop(Option>, ASTFlags, Position), + /// `return`/`throw` expr /// /// ### Flags /// diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 770d4174..ae933564 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -493,7 +493,7 @@ impl Engine { Ok(_) => (), Err(err) => match *err { ERR::LoopBreak(false, ..) => (), - ERR::LoopBreak(true, ..) => break Ok(Dynamic::UNIT), + ERR::LoopBreak(true, value, ..) => break Ok(value), _ => break Err(err), }, } @@ -524,7 +524,7 @@ impl Engine { Ok(_) => (), Err(err) => match *err { ERR::LoopBreak(false, ..) => (), - ERR::LoopBreak(true, ..) => break Ok(Dynamic::UNIT), + ERR::LoopBreak(true, value, ..) => break Ok(value), _ => break Err(err), }, } @@ -547,7 +547,7 @@ impl Engine { Ok(_) => (), Err(err) => match *err { ERR::LoopBreak(false, ..) => continue, - ERR::LoopBreak(true, ..) => break Ok(Dynamic::UNIT), + ERR::LoopBreak(true, value, ..) => break Ok(value), _ => break Err(err), }, } @@ -614,7 +614,7 @@ impl Engine { let loop_result = func(iter_obj) .enumerate() - .try_for_each(|(x, iter_value)| { + .try_fold(Dynamic::UNIT, |_, (x, iter_value)| { // Increment counter if counter_index < usize::MAX { // As the variable increments from 0, this should always work @@ -644,26 +644,26 @@ impl Engine { self.track_operation(global, statements.position())?; if statements.is_empty() { - return Ok(()); + return Ok(Dynamic::UNIT); } self.eval_stmt_block( scope, global, caches, lib, this_ptr, statements, true, level, ) - .map(|_| ()) + .map(|_| Dynamic::UNIT) .or_else(|err| match *err { - ERR::LoopBreak(false, ..) => Ok(()), + ERR::LoopBreak(false, ..) => Ok(Dynamic::UNIT), _ => Err(err), }) }) .or_else(|err| match *err { - ERR::LoopBreak(true, ..) => Ok(()), + ERR::LoopBreak(true, value, ..) => Ok(value), _ => Err(err), }); scope.rewind(orig_scope_len); - loop_result.map(|_| Dynamic::UNIT) + loop_result } else { Err(ERR::ErrorFor(expr.start_position()).into()) } @@ -673,8 +673,15 @@ impl Engine { } // Continue/Break statement - Stmt::BreakLoop(options, pos) => { - Err(ERR::LoopBreak(options.contains(ASTFlags::BREAK), *pos).into()) + Stmt::BreakLoop(expr, options, pos) => { + let is_break = options.contains(ASTFlags::BREAK); + + if let Some(ref expr) = expr { + self.eval_expr(scope, global, caches, lib, this_ptr, expr, level) + .and_then(|v| ERR::LoopBreak(is_break, v, *pos).into()) + } else { + Err(ERR::LoopBreak(is_break, Dynamic::UNIT, *pos).into()) + } } // Try/Catch statement diff --git a/src/optimizer.rs b/src/optimizer.rs index bd51a7c4..08abb8f2 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -8,7 +8,7 @@ use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, use crate::eval::{Caches, GlobalRuntimeState}; use crate::func::builtin::get_builtin_binary_op_fn; use crate::func::hashing::get_hasher; -use crate::tokenizer::{Span, Token}; +use crate::tokenizer::Token; use crate::types::dynamic::AccessMode; use crate::{ calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, FnPtr, Identifier, @@ -785,50 +785,6 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b *condition = Expr::Unit(*pos); } **body = optimize_stmt_block(mem::take(&mut **body), state, false, true, false); - - if body.len() == 1 { - match body[0] { - // while expr { break; } -> { expr; } - Stmt::BreakLoop(options, pos) if options.contains(ASTFlags::BREAK) => { - // Only a single break statement - turn into running the guard expression once - state.set_dirty(); - if condition.is_unit() { - *stmt = Stmt::Noop(pos); - } else { - let mut statements = vec![Stmt::Expr(mem::take(condition).into())]; - if preserve_result { - statements.push(Stmt::Noop(pos)); - } - *stmt = (statements, Span::new(pos, Position::NONE)).into(); - }; - } - _ => (), - } - } - } - // do { block } until true -> { block } - Stmt::Do(x, options, ..) - if matches!(x.0, Expr::BoolConstant(true, ..)) - && options.contains(ASTFlags::NEGATED) => - { - state.set_dirty(); - *stmt = ( - optimize_stmt_block(mem::take(&mut *x.1), state, false, true, false), - x.1.span(), - ) - .into(); - } - // do { block } while false -> { block } - Stmt::Do(x, options, ..) - if matches!(x.0, Expr::BoolConstant(false, ..)) - && !options.contains(ASTFlags::NEGATED) => - { - state.set_dirty(); - *stmt = ( - optimize_stmt_block(mem::take(&mut *x.1), state, false, true, false), - x.1.span(), - ) - .into(); } // do { block } while|until expr Stmt::Do(x, ..) => { @@ -916,6 +872,9 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b } } + // break expr; + Stmt::BreakLoop(Some(ref mut expr), ..) => optimize_expr(expr, state, false), + // return expr; Stmt::Return(Some(ref mut expr), ..) => optimize_expr(expr, state, false), diff --git a/src/parser.rs b/src/parser.rs index e2bcb181..9ca3a8a0 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1380,6 +1380,23 @@ impl Engine { self.parse_if(input, state, lib, settings.level_up())? .into(), )), + // Loops are allowed to act as expressions + Token::While | Token::Loop if settings.options.contains(LangOptions::LOOP_EXPR) => { + Expr::Stmt(Box::new( + self.parse_while_loop(input, state, lib, settings.level_up())? + .into(), + )) + } + Token::Do if settings.options.contains(LangOptions::LOOP_EXPR) => Expr::Stmt(Box::new( + self.parse_do(input, state, lib, settings.level_up())? + .into(), + )), + Token::For if settings.options.contains(LangOptions::LOOP_EXPR) => { + Expr::Stmt(Box::new( + self.parse_for(input, state, lib, settings.level_up())? + .into(), + )) + } // Switch statement is allowed to act as expressions Token::Switch if settings.options.contains(LangOptions::SWITCH_EXPR) => { Expr::Stmt(Box::new( @@ -3411,11 +3428,26 @@ impl Engine { Token::Continue if self.allow_looping() && settings.is_breakable => { let pos = eat_token(input, Token::Continue); - Ok(Stmt::BreakLoop(ASTFlags::NONE, pos)) + Ok(Stmt::BreakLoop(None, ASTFlags::NONE, pos)) } Token::Break if self.allow_looping() && settings.is_breakable => { let pos = eat_token(input, Token::Break); - Ok(Stmt::BreakLoop(ASTFlags::BREAK, pos)) + + let expr = match input.peek().expect(NEVER_ENDS) { + // `break` at + (Token::EOF, ..) => None, + // `break` at end of block + (Token::RightBrace, ..) => None, + // `break;` + (Token::SemiColon, ..) => None, + // `break` with expression + _ => Some( + self.parse_expr(input, state, lib, settings.level_up())? + .into(), + ), + }; + + Ok(Stmt::BreakLoop(expr, ASTFlags::BREAK, pos)) } Token::Continue | Token::Break if self.allow_looping() => { Err(PERR::LoopBreak.into_err(token_pos)) @@ -3840,7 +3872,7 @@ impl Engine { let mut functions = StraightHashMap::default(); let mut options = self.options; - options.remove(LangOptions::STMT_EXPR); + options.remove(LangOptions::STMT_EXPR | LangOptions::LOOP_EXPR); #[cfg(not(feature = "no_function"))] options.remove(LangOptions::ANON_FN); diff --git a/src/types/error.rs b/src/types/error.rs index 7991c44f..906f32c9 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -117,7 +117,7 @@ pub enum EvalAltResult { /// Breaking out of loops - not an error if within a loop. /// The wrapped value, if true, means breaking clean out of the loop (i.e. a `break` statement). /// The wrapped value, if false, means breaking the current context (i.e. a `continue` statement). - LoopBreak(bool, Position), + LoopBreak(bool, Dynamic, Position), /// Not an error: Value returned from a script via the `return` keyword. /// Wrapped value is the result value. Return(Dynamic, Position), diff --git a/tests/expressions.rs b/tests/expressions.rs index 4d0b4888..ed5e3a16 100644 --- a/tests/expressions.rs +++ b/tests/expressions.rs @@ -55,10 +55,13 @@ fn test_expressions() -> Result<(), Box> { ) .is_err()); - assert!(engine.eval_expression::<()>("40 + 2;").is_err()); - assert!(engine.eval_expression::<()>("40 + { 2 }").is_err()); - assert!(engine.eval_expression::<()>("x = 42").is_err()); + assert!(engine.compile_expression("40 + 2;").is_err()); + assert!(engine.compile_expression("40 + { 2 }").is_err()); + assert!(engine.compile_expression("x = 42").is_err()); assert!(engine.compile_expression("let x = 42").is_err()); + assert!(engine + .compile_expression("do { break 42; } while true") + .is_err()); engine.compile("40 + { let x = 2; x }")?; diff --git a/tests/for.rs b/tests/for.rs index 92e68af1..af2eed86 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -231,6 +231,22 @@ fn test_for_loop() -> Result<(), Box> { ); } + assert_eq!( + engine.eval::( + r#" + let a = [123, 999, 42, 0, true, "hello", "world!", 987.6543]; + + for (item, count) in a { + switch item.type_of() { + "i64" if item.is_even => break count, + "f64" if item.to_int().is_even => break count, + } + } + "# + )?, + 2 + ); + Ok(()) } diff --git a/tests/while_loop.rs b/tests/while_loop.rs index 4e18a371..ebcdb8a0 100644 --- a/tests/while_loop.rs +++ b/tests/while_loop.rs @@ -22,6 +22,22 @@ fn test_while() -> Result<(), Box> { 6 ); + assert_eq!( + engine.eval::( + " + let x = 0; + + while x < 10 { + x += 1; + if x > 5 { break x * 2; } + if x > 3 { continue; } + x += 3; + } + ", + )?, + 12 + ); + Ok(()) } @@ -46,6 +62,25 @@ fn test_do() -> Result<(), Box> { )?, 6 ); + assert_eq!( + engine.eval::( + " + let x = 0; + + do { + x += 1; + if x > 5 { break x * 2; } + if x > 3 { continue; } + x += 3; + } while x < 10; + ", + )?, + 12 + ); + + engine.run("do {} while false")?; + + assert_eq!(engine.eval::("do { break 42; } while false")?, 42); Ok(()) } From 68bd84417aa79c0427a6e89a1c430cebc162d83a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 12:56:35 +0800 Subject: [PATCH 04/11] Fix tests. --- src/engine.rs | 2 ++ tests/for.rs | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/engine.rs b/src/engine.rs index 8071b9de..0afadf75 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -239,8 +239,10 @@ impl Engine { engine.print = Box::new(|s| println!("{s}")); engine.debug = Box::new(|s, source, pos| match (source, pos) { (Some(source), crate::Position::NONE) => println!("{source} | {s}"), + #[cfg(not(feature = "no_position"))] (Some(source), pos) => println!("{source} @ {pos:?} | {s}"), (None, crate::Position::NONE) => println!("{s}"), + #[cfg(not(feature = "no_position"))] (None, pos) => println!("{pos:?} | {s}"), }); } diff --git a/tests/for.rs b/tests/for.rs index af2eed86..63c8ae92 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -231,10 +231,12 @@ fn test_for_loop() -> Result<(), Box> { ); } + #[cfg(not(feature = "no_index"))] + #[cfg(not(feature = "no_object"))] assert_eq!( engine.eval::( r#" - let a = [123, 999, 42, 0, true, "hello", "world!", 987.6543]; + let a = [123, 999, 42, 0, true, "hello", "world!"]; for (item, count) in a { switch item.type_of() { From de81941c2c2c5036d24328e304413bb485af6ccf Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 12:57:58 +0800 Subject: [PATCH 05/11] Fix test. --- tests/for.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/for.rs b/tests/for.rs index 63c8ae92..c8353b51 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -233,10 +233,12 @@ fn test_for_loop() -> Result<(), Box> { #[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_object"))] + #[cfg(not(feature = "only_i32"))] + #[cfg(not(feature = "no_float"))] assert_eq!( engine.eval::( r#" - let a = [123, 999, 42, 0, true, "hello", "world!"]; + let a = [123, 999, 42, 0, true, "hello", "world!", 987.654]; for (item, count) in a { switch item.type_of() { From 905b0b83325b5d48f97a7c52145b7a87610a625b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 12:58:38 +0800 Subject: [PATCH 06/11] Fix test. --- tests/for.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/for.rs b/tests/for.rs index c8353b51..a379a977 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -233,7 +233,6 @@ fn test_for_loop() -> Result<(), Box> { #[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_object"))] - #[cfg(not(feature = "only_i32"))] #[cfg(not(feature = "no_float"))] assert_eq!( engine.eval::( @@ -242,8 +241,8 @@ fn test_for_loop() -> Result<(), Box> { for (item, count) in a { switch item.type_of() { - "i64" if item.is_even => break count, - "f64" if item.to_int().is_even => break count, + "i64" | "i32 if item.is_even => break count, + "f64" | "f32 if item.to_int().is_even => break count, } } "# From d0998a44b9fbcaecbc506823abd63659627473e9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 14:12:09 +0800 Subject: [PATCH 07/11] Fix test again. --- tests/for.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/for.rs b/tests/for.rs index a379a977..48139e70 100644 --- a/tests/for.rs +++ b/tests/for.rs @@ -241,8 +241,8 @@ fn test_for_loop() -> Result<(), Box> { for (item, count) in a { switch item.type_of() { - "i64" | "i32 if item.is_even => break count, - "f64" | "f32 if item.to_int().is_even => break count, + "i64" | "i32" if item.is_even => break count, + "f64" | "f32" if item.to_int().is_even => break count, } } "# From 4100e6da64f6a152335d720d40d61f429e8260e7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 14:12:18 +0800 Subject: [PATCH 08/11] Use ImmutableString for source. --- src/api/eval.rs | 2 +- src/api/run.rs | 2 +- src/ast/ast.rs | 52 ++++++++++++++++++++------------------- src/eval/cache.rs | 4 +-- src/eval/debugger.rs | 33 +++++++++++++------------ src/eval/eval_context.rs | 6 +---- src/eval/global_state.rs | 44 +++++++++++++++++---------------- src/eval/stmt.rs | 4 +-- src/func/call.rs | 39 +++++++++-------------------- src/func/script.rs | 2 +- src/module/mod.rs | 32 +++++++++++++----------- src/packages/debugging.rs | 4 +-- tests/modules.rs | 2 +- tests/optimizer.rs | 12 ++++----- 14 files changed, 112 insertions(+), 126 deletions(-) diff --git a/src/api/eval.rs b/src/api/eval.rs index 6808e2e6..3f37264d 100644 --- a/src/api/eval.rs +++ b/src/api/eval.rs @@ -217,7 +217,7 @@ impl Engine { level: usize, ) -> RhaiResult { let mut caches = Caches::new(); - global.source = ast.source_raw().clone(); + global.source = ast.source_raw().cloned(); #[cfg(not(feature = "no_module"))] let orig_embedded_module_resolver = std::mem::replace( diff --git a/src/api/run.rs b/src/api/run.rs index b361200a..e32136e6 100644 --- a/src/api/run.rs +++ b/src/api/run.rs @@ -113,7 +113,7 @@ impl Engine { pub fn run_ast_with_scope(&self, scope: &mut Scope, ast: &AST) -> RhaiResultOf<()> { let caches = &mut Caches::new(); let global = &mut GlobalRuntimeState::new(self); - global.source = ast.source_raw().clone(); + global.source = ast.source_raw().cloned(); #[cfg(not(feature = "no_module"))] { diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 98ed86b3..aa18b3ac 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -1,7 +1,7 @@ //! Module defining the AST (abstract syntax tree). use super::{ASTFlags, Expr, FnAccess, Stmt, StmtBlock, StmtBlockContainer}; -use crate::{Dynamic, FnNamespace, Identifier, Position}; +use crate::{Dynamic, FnNamespace, ImmutableString, Position}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ @@ -20,8 +20,7 @@ use std::{ #[derive(Clone)] pub struct AST { /// Source of the [`AST`]. - /// No source if string is empty. - source: Identifier, + source: Option, /// [`AST`] documentation. #[cfg(feature = "metadata")] doc: crate::SmartString, @@ -98,7 +97,7 @@ impl AST { #[cfg(not(feature = "no_function"))] functions: impl Into>, ) -> Self { Self { - source: Identifier::new_const(), + source: None, #[cfg(feature = "metadata")] doc: crate::SmartString::new_const(), body: StmtBlock::new(statements, Position::NONE, Position::NONE), @@ -133,7 +132,7 @@ impl AST { pub fn new_with_source( statements: impl IntoIterator, #[cfg(not(feature = "no_function"))] functions: impl Into>, - source: impl Into, + source: impl Into, ) -> Self { let mut ast = Self::new( statements, @@ -148,7 +147,7 @@ impl AST { #[must_use] pub fn empty() -> Self { Self { - source: Identifier::new_const(), + source: None, #[cfg(feature = "metadata")] doc: crate::SmartString::new_const(), body: StmtBlock::NONE, @@ -159,36 +158,39 @@ impl AST { } } /// Get the source, if any. - #[inline] + #[inline(always)] #[must_use] pub fn source(&self) -> Option<&str> { - if self.source.is_empty() { - None - } else { - Some(self.source.as_str()) - } + self.source.as_ref().map(|s| s.as_str()) } /// Get a reference to the source. #[inline(always)] #[must_use] - pub(crate) const fn source_raw(&self) -> &Identifier { - &self.source + pub(crate) const fn source_raw(&self) -> Option<&ImmutableString> { + self.source.as_ref() } /// Set the source. #[inline] - pub fn set_source(&mut self, source: impl Into) -> &mut Self { + pub fn set_source(&mut self, source: impl Into) -> &mut Self { let source = source.into(); + #[cfg(not(feature = "no_function"))] crate::Shared::get_mut(&mut self.lib) .as_mut() .map(|m| m.set_id(source.clone())); - self.source = source; + + if source.is_empty() { + self.source = None; + } else { + self.source = Some(source); + } + self } /// Clear the source. #[inline(always)] pub fn clear_source(&mut self) -> &mut Self { - self.source.clear(); + self.source = None; self } /// Get the documentation (if any). @@ -559,18 +561,18 @@ impl AST { lib }; - let mut _ast = if other.source.is_empty() { - Self::new( - merged, - #[cfg(not(feature = "no_function"))] - lib, - ) - } else { + let mut _ast = if let Some(ref source) = other.source { Self::new_with_source( merged, #[cfg(not(feature = "no_function"))] lib, - other.source.clone(), + source.clone(), + ) + } else { + Self::new( + merged, + #[cfg(not(feature = "no_function"))] + lib, ) }; diff --git a/src/eval/cache.rs b/src/eval/cache.rs index cf96dc24..b71eb8cf 100644 --- a/src/eval/cache.rs +++ b/src/eval/cache.rs @@ -2,7 +2,7 @@ use crate::func::{CallableFunction, StraightHashMap}; use crate::types::BloomFilterU64; -use crate::{Identifier, StaticVec}; +use crate::{ImmutableString, StaticVec}; use std::marker::PhantomData; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -14,7 +14,7 @@ pub struct FnResolutionCacheEntry { /// Function. pub func: CallableFunction, /// Optional source. - pub source: Option>, + pub source: Option, } /// _(internals)_ A function resolution cache with a bloom filter. diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index 63bb3d22..913f66a5 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -3,7 +3,10 @@ use super::{EvalContext, GlobalRuntimeState}; use crate::ast::{ASTNode, Expr, Stmt}; -use crate::{Dynamic, Engine, EvalAltResult, Identifier, Module, Position, RhaiResultOf, Scope}; +use crate::{ + Dynamic, Engine, EvalAltResult, Identifier, ImmutableString, Module, Position, RhaiResultOf, + Scope, +}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{fmt, iter::repeat, mem}; @@ -226,8 +229,8 @@ pub struct CallStackFrame { pub fn_name: Identifier, /// Copies of function call arguments, if any. pub args: crate::StaticVec, - /// Source of the function, empty if none. - pub source: Identifier, + /// Source of the function. + pub source: Option, /// [Position][`Position`] of the function call. pub pos: Position, } @@ -243,8 +246,8 @@ impl fmt::Display for CallStackFrame { fp.finish()?; if !self.pos.is_none() { - if !self.source.is_empty() { - write!(f, ": {}", self.source)?; + if let Some(ref source) = self.source { + write!(f, ": {source}")?; } write!(f, " @ {:?}", self.pos)?; } @@ -295,13 +298,13 @@ impl Debugger { &mut self, fn_name: impl Into, args: crate::StaticVec, - source: impl Into, + source: Option, pos: Position, ) { self.call_stack.push(CallStackFrame { fn_name: fn_name.into(), args, - source: source.into(), + source, pos, }); } @@ -487,7 +490,10 @@ impl Engine { let event = match event { Some(e) => e, - None => match global.debugger.is_break_point(&global.source, node) { + None => match global + .debugger + .is_break_point(global.source().unwrap_or(""), node) + { Some(bp) => DebuggerEvent::BreakPoint(bp), None => return Ok(None), }, @@ -512,17 +518,12 @@ impl Engine { event: DebuggerEvent, level: usize, ) -> Result, Box> { - let source = global.source.clone(); - let source = if source.is_empty() { - None - } else { - Some(source.as_str()) - }; - + let src = global.source_raw().cloned(); + let src = src.as_ref().map(|s| s.as_str()); let context = crate::EvalContext::new(self, scope, global, None, lib, this_ptr, level); if let Some((.., ref on_debugger)) = self.debugger { - let command = on_debugger(context, event, node, source, node.position())?; + let command = on_debugger(context, event, node, src, node.position())?; match command { DebuggerCommand::Continue => { diff --git a/src/eval/eval_context.rs b/src/eval/eval_context.rs index 5ecad699..b1185a39 100644 --- a/src/eval/eval_context.rs +++ b/src/eval/eval_context.rs @@ -58,11 +58,7 @@ impl<'a, 's, 'ps, 'g, 'pg, 'c, 'pc, 't, 'pt> EvalContext<'a, 's, 'ps, 'g, 'pg, ' #[inline(always)] #[must_use] pub fn source(&self) -> Option<&str> { - if self.global.source.is_empty() { - None - } else { - Some(self.global.source.as_str()) - } + self.global.source() } /// The current [`Scope`]. #[inline(always)] diff --git a/src/eval/global_state.rs b/src/eval/global_state.rs index a22a45a2..bf4489f9 100644 --- a/src/eval/global_state.rs +++ b/src/eval/global_state.rs @@ -1,6 +1,6 @@ //! Global runtime state. -use crate::{Dynamic, Engine, Identifier}; +use crate::{Dynamic, Engine, ImmutableString}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{fmt, marker::PhantomData}; @@ -9,7 +9,7 @@ use std::{fmt, marker::PhantomData}; #[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_function"))] pub type GlobalConstants = - crate::Shared>>; + crate::Shared>>; /// _(internals)_ Global runtime states. /// Exported under the `internals` feature only. @@ -25,14 +25,14 @@ pub type GlobalConstants = pub struct GlobalRuntimeState<'a> { /// Names of imported [modules][crate::Module]. #[cfg(not(feature = "no_module"))] - imports: crate::StaticVec, + imports: crate::StaticVec, /// Stack of imported [modules][crate::Module]. #[cfg(not(feature = "no_module"))] modules: crate::StaticVec>, /// Source of the current context. /// /// No source if the string is empty. - pub source: Identifier, + pub source: Option, /// Number of operations performed. pub num_operations: u64, /// Number of modules loaded. @@ -84,7 +84,7 @@ impl GlobalRuntimeState<'_> { imports: crate::StaticVec::new_const(), #[cfg(not(feature = "no_module"))] modules: crate::StaticVec::new_const(), - source: Identifier::new_const(), + source: None, num_operations: 0, #[cfg(not(feature = "no_module"))] num_modules_loaded: 0, @@ -168,7 +168,7 @@ impl GlobalRuntimeState<'_> { #[inline(always)] pub fn push_import( &mut self, - name: impl Into, + name: impl Into, module: impl Into>, ) { self.imports.push(name.into()); @@ -202,7 +202,7 @@ impl GlobalRuntimeState<'_> { #[inline] pub(crate) fn iter_imports_raw( &self, - ) -> impl Iterator)> { + ) -> impl Iterator)> { self.imports.iter().zip(self.modules.iter()).rev() } /// Get an iterator to the stack of globally-imported [modules][crate::Module] in forward order. @@ -212,7 +212,7 @@ impl GlobalRuntimeState<'_> { #[inline] pub fn scan_imports_raw( &self, - ) -> impl Iterator)> { + ) -> impl Iterator)> { self.imports.iter().zip(self.modules.iter()) } /// Can the particular function with [`Dynamic`] parameter(s) exist in the stack of @@ -247,11 +247,11 @@ impl GlobalRuntimeState<'_> { pub fn get_qualified_fn( &self, hash: u64, - ) -> Option<(&crate::func::CallableFunction, Option<&str>)> { + ) -> Option<(&crate::func::CallableFunction, Option<&ImmutableString>)> { self.modules .iter() .rev() - .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id()))) + .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw()))) } /// Does the specified [`TypeId`][std::any::TypeId] iterator exist in the stack of /// globally-imported [modules][crate::Module]? @@ -278,14 +278,16 @@ impl GlobalRuntimeState<'_> { .find_map(|m| m.get_qualified_iter(id)) } /// Get the current source. - #[inline] + #[inline(always)] #[must_use] pub fn source(&self) -> Option<&str> { - if self.source.is_empty() { - None - } else { - Some(self.source.as_str()) - } + self.source.as_ref().map(|s| s.as_str()) + } + /// Get the current source. + #[inline(always)] + #[must_use] + pub(crate) const fn source_raw(&self) -> Option<&ImmutableString> { + self.source.as_ref() } /// Get the pre-calculated index getter hash. #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] @@ -317,10 +319,10 @@ impl GlobalRuntimeState<'_> { #[cfg(not(feature = "no_module"))] impl IntoIterator for GlobalRuntimeState<'_> { - type Item = (crate::ImmutableString, crate::Shared); + type Item = (ImmutableString, crate::Shared); type IntoIter = std::iter::Rev< std::iter::Zip< - smallvec::IntoIter<[crate::ImmutableString; crate::STATIC_VEC_INLINE_SIZE]>, + smallvec::IntoIter<[ImmutableString; crate::STATIC_VEC_INLINE_SIZE]>, smallvec::IntoIter<[crate::Shared; crate::STATIC_VEC_INLINE_SIZE]>, >, >; @@ -332,10 +334,10 @@ impl IntoIterator for GlobalRuntimeState<'_> { #[cfg(not(feature = "no_module"))] impl<'a> IntoIterator for &'a GlobalRuntimeState<'_> { - type Item = (&'a crate::ImmutableString, &'a crate::Shared); + type Item = (&'a ImmutableString, &'a crate::Shared); type IntoIter = std::iter::Rev< std::iter::Zip< - std::slice::Iter<'a, crate::ImmutableString>, + std::slice::Iter<'a, ImmutableString>, std::slice::Iter<'a, crate::Shared>, >, >; @@ -346,7 +348,7 @@ impl<'a> IntoIterator for &'a GlobalRuntimeState<'_> { } #[cfg(not(feature = "no_module"))] -impl, M: Into>> Extend<(K, M)> +impl, M: Into>> Extend<(K, M)> for GlobalRuntimeState<'_> { #[inline] diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index ae933564..4eecbf88 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -719,8 +719,8 @@ impl Engine { err_map.insert("message".into(), err.to_string().into()); - if !global.source.is_empty() { - err_map.insert("source".into(), global.source.clone().into()); + if let Some(ref source) = global.source { + err_map.insert("source".into(), source.into()); } if !err_pos.is_none() { diff --git a/src/func/call.rs b/src/func/call.rs index ae4298fe..5c3fbf43 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -218,7 +218,7 @@ impl Engine { .iter() .copied() .chain(self.global_modules.iter().map(|m| m.as_ref())) - .find_map(|m| m.get_fn(hash).map(|f| (f, m.id()))); + .find_map(|m| m.get_fn(hash).map(|f| (f, m.id_raw()))); #[cfg(not(feature = "no_module"))] let func = if args.is_none() { @@ -228,7 +228,7 @@ impl Engine { func.or_else(|| _global.get_qualified_fn(hash)).or_else(|| { self.global_sub_modules .values() - .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id()))) + .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw()))) }) }; @@ -236,7 +236,7 @@ impl Engine { // Specific version found let new_entry = Some(FnResolutionCacheEntry { func: f.clone(), - source: s.map(|s| Box::new(s.into())), + source: s.cloned(), }); return if cache.filter.is_absent_and_set(hash) { // Do not cache "one-hit wonders" @@ -358,7 +358,6 @@ impl Engine { ) -> RhaiResultOf<(Dynamic, bool)> { self.track_operation(global, pos)?; - let parent_source = global.source.clone(); let op_assign = if is_op_assign { Token::lookup_from_syntax(name) } else { @@ -398,24 +397,19 @@ impl Engine { backup.change_first_arg_to_copy(args); } - let source = match (source, parent_source.as_str()) { - (None, "") => None, - (None, s) => Some(s), - (Some(s), ..) => Some(s.as_str()), - }; - #[cfg(feature = "debugging")] if self.debugger.is_some() { global.debugger.push_call_stack_frame( name, args.iter().map(|v| (*v).clone()).collect(), - source.unwrap_or(""), + source.clone().or_else(|| global.source.clone()), pos, ); } // Run external function - let context = (self, name, source, &*global, lib, pos, level).into(); + let src = source.as_ref().map(|s| s.as_str()); + let context = (self, name, src, &*global, lib, pos, level).into(); let result = if func.is_plugin_fn() { let f = func.get_plugin_fn().unwrap(); @@ -484,12 +478,7 @@ impl Engine { let t = self.map_type_name(type_name::()).into(); ERR::ErrorMismatchOutputType(t, typ.into(), pos) })?; - let source = if global.source.is_empty() { - None - } else { - Some(global.source.as_str()) - }; - ((*self.debug)(&text, source, pos).into(), false) + ((*self.debug)(&text, global.source(), pos).into(), false) } _ => (result, is_method), }); @@ -685,12 +674,7 @@ impl Engine { } }; - let orig_source = mem::replace( - &mut global.source, - source - .as_ref() - .map_or(crate::Identifier::new_const(), |s| (**s).clone()), - ); + let orig_source = mem::replace(&mut global.source, source.clone()); let result = if _is_method_call { // Method call of script function - map first argument to `this` @@ -1172,7 +1156,7 @@ impl Engine { return result.map_err(|err| { ERR::ErrorInFunctionCall( KEYWORD_EVAL.to_string(), - global.source.to_string(), + global.source().unwrap_or("").to_string(), err, pos, ) @@ -1416,14 +1400,13 @@ impl Engine { Some(f) if f.is_script() => { let fn_def = f.get_script_fn_def().expect("script-defined function"); let new_scope = &mut Scope::new(); - let mut source = module.id_raw().clone(); - mem::swap(&mut global.source, &mut source); + let orig_source = mem::replace(&mut global.source, module.id_raw().cloned()); let result = self.call_script_fn( new_scope, global, caches, lib, &mut None, fn_def, &mut args, true, pos, level, ); - global.source = source; + global.source = orig_source; result } diff --git a/src/func/script.rs b/src/func/script.rs index c387b3dc..476343af 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -54,7 +54,7 @@ impl Engine { Err(ERR::ErrorInFunctionCall( name, - source.unwrap_or_else(|| global.source.to_string()), + source.unwrap_or_else(|| global.source().unwrap_or("").to_string()), err, pos, ) diff --git a/src/module/mod.rs b/src/module/mod.rs index fe48d77f..37a0e167 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -160,8 +160,7 @@ pub fn calc_native_fn_hash<'a>( #[derive(Clone)] pub struct Module { /// ID identifying the module. - /// No ID if string is empty. - id: Identifier, + id: Option, /// Module documentation. #[cfg(feature = "metadata")] doc: crate::SmartString, @@ -292,7 +291,7 @@ impl Module { #[must_use] pub fn with_capacity(capacity: usize) -> Self { Self { - id: Identifier::new_const(), + id: None, #[cfg(feature = "metadata")] doc: crate::SmartString::new_const(), internal: false, @@ -324,18 +323,14 @@ impl Module { #[inline] #[must_use] pub fn id(&self) -> Option<&str> { - if self.id_raw().is_empty() { - None - } else { - Some(self.id_raw()) - } + self.id.as_ref().map(|s| s.as_str()) } /// Get the ID of the [`Module`] as an [`Identifier`], if any. #[inline(always)] #[must_use] - pub(crate) const fn id_raw(&self) -> &Identifier { - &self.id + pub(crate) const fn id_raw(&self) -> Option<&ImmutableString> { + self.id.as_ref() } /// Set the ID of the [`Module`]. @@ -351,8 +346,15 @@ impl Module { /// assert_eq!(module.id(), Some("hello")); /// ``` #[inline(always)] - pub fn set_id(&mut self, id: impl Into) -> &mut Self { - self.id = id.into(); + pub fn set_id(&mut self, id: impl Into) -> &mut Self { + let id = id.into(); + + if id.is_empty() { + self.id = None; + } else { + self.id = Some(id); + } + self } @@ -370,7 +372,7 @@ impl Module { /// ``` #[inline(always)] pub fn clear_id(&mut self) -> &mut Self { - self.id.clear(); + self.id = None; self } @@ -434,7 +436,7 @@ impl Module { /// Clear the [`Module`]. #[inline(always)] pub fn clear(&mut self) { - self.id.clear(); + self.id = None; #[cfg(feature = "metadata")] self.doc.clear(); self.internal = false; @@ -2078,7 +2080,7 @@ impl Module { }); } - module.set_id(ast.source_raw().clone()); + module.id = ast.source_raw().cloned(); #[cfg(feature = "metadata")] module.set_doc(ast.doc()); diff --git a/src/packages/debugging.rs b/src/packages/debugging.rs index e48571a8..eba64f5e 100644 --- a/src/packages/debugging.rs +++ b/src/packages/debugging.rs @@ -62,8 +62,8 @@ mod debugging_functions { Dynamic::from_array(_args.clone().to_vec()), ); } - if !_source.is_empty() { - map.insert("source".into(), _source.into()); + if let Some(source) = _source { + map.insert("source".into(), source.into()); } if !_pos.is_none() { map.insert( diff --git a/tests/modules.rs b/tests/modules.rs index 77e60b2b..f18d9aa7 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -578,7 +578,7 @@ fn test_module_context() -> Result<(), Box> { let new_context = NativeCallContext::new_with_all_fields( engine, &fn_name, - source.as_ref().map(|s| s.as_str()), + source.as_ref().map(String::as_str), &global, &lib, pos, diff --git a/tests/optimizer.rs b/tests/optimizer.rs index 248a15f0..b72cab02 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -89,21 +89,21 @@ fn test_optimizer_parse() -> Result<(), Box> { assert_eq!( format!("{ast:?}"), - r#"AST { source: "", doc: "", resolver: None, body: [Expr(123 @ 1:53)] }"# + r#"AST { source: None, doc: "", resolver: None, body: [Expr(123 @ 1:53)] }"# ); let ast = engine.compile("const DECISION = false; if DECISION { 42 } else { 123 }")?; assert_eq!( format!("{ast:?}"), - r#"AST { source: "", doc: "", resolver: None, body: [Var(("DECISION" @ 1:7, false @ 1:18, None), CONSTANT, 1:1), Expr(123 @ 1:51)] }"# + r#"AST { source: None, doc: "", resolver: None, body: [Var(("DECISION" @ 1:7, false @ 1:18, None), CONSTANT, 1:1), Expr(123 @ 1:51)] }"# ); let ast = engine.compile("if 1 == 2 { 42 }")?; assert_eq!( format!("{ast:?}"), - r#"AST { source: "", doc: "", resolver: None, body: [] }"# + r#"AST { source: None, doc: "", resolver: None, body: [] }"# ); engine.set_optimization_level(OptimizationLevel::Full); @@ -112,14 +112,14 @@ fn test_optimizer_parse() -> Result<(), Box> { assert_eq!( format!("{ast:?}"), - r#"AST { source: "", doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"# + r#"AST { source: None, doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"# ); let ast = engine.compile("NUMBER")?; assert_eq!( format!("{ast:?}"), - r#"AST { source: "", doc: "", resolver: None, body: [Expr(Variable(NUMBER) @ 1:1)] }"# + r#"AST { source: None, doc: "", resolver: None, body: [Expr(Variable(NUMBER) @ 1:1)] }"# ); let mut module = Module::new(); @@ -131,7 +131,7 @@ fn test_optimizer_parse() -> Result<(), Box> { assert_eq!( format!("{ast:?}"), - r#"AST { source: "", doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"# + r#"AST { source: None, doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"# ); Ok(()) From 8e35f9847733df6210d67b4daf210935cd129e17 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 14:27:30 +0800 Subject: [PATCH 09/11] Fix build. --- src/ast/ast.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ast/ast.rs b/src/ast/ast.rs index aa18b3ac..9c186e67 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -77,7 +77,7 @@ impl AST { #[cfg(not(feature = "no_function"))] functions: impl Into>, ) -> Self { Self { - source: Identifier::new_const(), + source: None, #[cfg(feature = "metadata")] doc: crate::SmartString::new_const(), body: StmtBlock::new(statements, Position::NONE, Position::NONE), @@ -114,7 +114,7 @@ impl AST { pub(crate) fn new_with_source( statements: impl IntoIterator, #[cfg(not(feature = "no_function"))] functions: impl Into>, - source: impl Into, + source: impl Into, ) -> Self { let mut ast = Self::new( statements, From 91415b9750c77abca2831b8cfe3cf13867cbfd63 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 14:59:20 +0800 Subject: [PATCH 10/11] Use ImmutableString in more places. --- src/api/definitions/mod.rs | 4 +-- src/ast/expr.rs | 2 +- src/bin/rhai-dbg.rs | 4 +-- src/eval/debugger.rs | 31 ++++++++----------- src/func/call.rs | 2 +- src/module/mod.rs | 2 +- src/packages/debugging.rs | 2 +- src/packages/fn_basic.rs | 2 +- src/tests.rs | 2 +- src/types/fn_ptr.rs | 61 ++++++++------------------------------ src/types/scope.rs | 18 ++++++----- 11 files changed, 45 insertions(+), 85 deletions(-) diff --git a/src/api/definitions/mod.rs b/src/api/definitions/mod.rs index 18d9c992..3ee43af0 100644 --- a/src/api/definitions/mod.rs +++ b/src/api/definitions/mod.rs @@ -442,8 +442,8 @@ impl Module { if f.access != FnAccess::Private { #[cfg(not(feature = "no_custom_syntax"))] - let operator = def.engine.custom_keywords.contains_key(&f.name) - || (!f.name.contains('$') && !is_valid_function_name(&f.name)); + let operator = def.engine.custom_keywords.contains_key(f.name.as_str()) + || (!f.name.contains('$') && !is_valid_function_name(f.name.as_str())); #[cfg(feature = "no_custom_syntax")] let operator = !f.name.contains('$') && !is_valid_function_name(&f.name); diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 88bac79b..863299c8 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -619,7 +619,7 @@ impl Expr { if !x.is_qualified() && x.args.len() == 1 && x.name == KEYWORD_FN_PTR => { if let Self::StringConstant(ref s, ..) = x.args[0] { - FnPtr::new(s).ok()?.into() + FnPtr::new(s.clone()).ok()?.into() } else { return None; } diff --git a/src/bin/rhai-dbg.rs b/src/bin/rhai-dbg.rs index 5649c0c1..04284050 100644 --- a/src/bin/rhai-dbg.rs +++ b/src/bin/rhai-dbg.rs @@ -516,7 +516,7 @@ fn debug_callback( if range.contains(&n) { let bp = rhai::debugger::BreakPoint::AtPosition { - source: source.unwrap_or("").into(), + source: source.map(|s| s.into()), pos: Position::new(n as u16, 0), enabled: true, }; @@ -546,7 +546,7 @@ fn debug_callback( #[cfg(not(feature = "no_position"))] ["break" | "b"] => { let bp = rhai::debugger::BreakPoint::AtPosition { - source: source.unwrap_or("").into(), + source: source.map(|s| s.into()), pos, enabled: true, }; diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index 913f66a5..f87e4a81 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -4,8 +4,7 @@ use super::{EvalContext, GlobalRuntimeState}; use crate::ast::{ASTNode, Expr, Stmt}; use crate::{ - Dynamic, Engine, EvalAltResult, Identifier, ImmutableString, Module, Position, RhaiResultOf, - Scope, + Dynamic, Engine, EvalAltResult, ImmutableString, Module, Position, RhaiResultOf, Scope, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -103,12 +102,10 @@ pub enum BreakPoint { /// Break at a particular position under a particular source. /// /// Not available under `no_position`. - /// - /// Source is empty if not available. #[cfg(not(feature = "no_position"))] AtPosition { /// Source (empty if not available) of the break-point. - source: Identifier, + source: Option, /// [Position] of the break-point. pos: Position, /// Is the break-point enabled? @@ -117,14 +114,14 @@ pub enum BreakPoint { /// Break at a particular function call. AtFunctionName { /// Function name. - name: Identifier, + name: ImmutableString, /// Is the break-point enabled? enabled: bool, }, /// Break at a particular function call with a particular number of arguments. AtFunctionCall { /// Function name. - name: Identifier, + name: ImmutableString, /// Number of arguments. args: usize, /// Is the break-point enabled? @@ -136,7 +133,7 @@ pub enum BreakPoint { #[cfg(not(feature = "no_object"))] AtProperty { /// Property name. - name: Identifier, + name: ImmutableString, /// Is the break-point enabled? enabled: bool, }, @@ -151,7 +148,7 @@ impl fmt::Display for BreakPoint { pos, enabled, } => { - if !source.is_empty() { + if let Some(ref source) = source { write!(f, "{source} ")?; } write!(f, "@ {pos:?}")?; @@ -226,7 +223,7 @@ impl BreakPoint { #[derive(Debug, Clone, Hash)] pub struct CallStackFrame { /// Function name. - pub fn_name: Identifier, + pub fn_name: ImmutableString, /// Copies of function call arguments, if any. pub args: crate::StaticVec, /// Source of the function. @@ -296,7 +293,7 @@ impl Debugger { #[inline(always)] pub(crate) fn push_call_stack_frame( &mut self, - fn_name: impl Into, + fn_name: ImmutableString, args: crate::StaticVec, source: Option, pos: Position, @@ -331,7 +328,7 @@ impl Debugger { } /// Returns the first break-point triggered by a particular [`AST` Node][ASTNode]. #[must_use] - pub fn is_break_point(&self, src: &str, node: ASTNode) -> Option { + pub fn is_break_point(&self, src: Option<&str>, node: ASTNode) -> Option { let _src = src; self.break_points() @@ -343,11 +340,12 @@ impl Debugger { BreakPoint::AtPosition { pos, .. } if pos.is_none() => false, #[cfg(not(feature = "no_position"))] BreakPoint::AtPosition { source, pos, .. } if pos.is_beginning_of_line() => { - node.position().line().unwrap_or(0) == pos.line().unwrap() && _src == source + node.position().line().unwrap_or(0) == pos.line().unwrap() + && _src == source.as_ref().map(|s| s.as_str()) } #[cfg(not(feature = "no_position"))] BreakPoint::AtPosition { source, pos, .. } => { - node.position() == *pos && _src == source + node.position() == *pos && _src == source.as_ref().map(|s| s.as_str()) } BreakPoint::AtFunctionName { name, .. } => match node { ASTNode::Expr(Expr::FnCall(x, ..)) | ASTNode::Stmt(Stmt::FnCall(x, ..)) => { @@ -490,10 +488,7 @@ impl Engine { let event = match event { Some(e) => e, - None => match global - .debugger - .is_break_point(global.source().unwrap_or(""), node) - { + None => match global.debugger.is_break_point(global.source(), node) { Some(bp) => DebuggerEvent::BreakPoint(bp), None => return Ok(None), }, diff --git a/src/func/call.rs b/src/func/call.rs index 5c3fbf43..ef5d77e7 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -400,7 +400,7 @@ impl Engine { #[cfg(feature = "debugging")] if self.debugger.is_some() { global.debugger.push_call_stack_frame( - name, + self.get_interned_string(name), args.iter().map(|v| (*v).clone()).collect(), source.clone().or_else(|| global.source.clone()), pos, diff --git a/src/module/mod.rs b/src/module/mod.rs index 37a0e167..59ead075 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -72,7 +72,7 @@ pub struct FuncInfo { /// Function access mode. pub access: FnAccess, /// Function name. - pub name: Identifier, + pub name: ImmutableString, /// Number of parameters. pub num_params: usize, /// Parameter types (if applicable). diff --git a/src/packages/debugging.rs b/src/packages/debugging.rs index eba64f5e..8a800c1b 100644 --- a/src/packages/debugging.rs +++ b/src/packages/debugging.rs @@ -40,7 +40,7 @@ mod debugging_functions { .iter() .rev() .filter(|crate::debugger::CallStackFrame { fn_name, args, .. }| { - fn_name != "back_trace" || !args.is_empty() + fn_name.as_str() != "back_trace" || !args.is_empty() }) .map( |frame @ crate::debugger::CallStackFrame { diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs index 505a08b5..130646c3 100644 --- a/src/packages/fn_basic.rs +++ b/src/packages/fn_basic.rs @@ -27,7 +27,7 @@ mod fn_ptr_functions { /// ``` #[rhai_fn(name = "name", get = "name", pure)] pub fn name(fn_ptr: &mut FnPtr) -> ImmutableString { - fn_ptr.fn_name_raw().into() + fn_ptr.fn_name_raw().clone() } /// Return `true` if the function is an anonymous function. diff --git a/src/tests.rs b/src/tests.rs index c91a1e1e..34186402 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -35,7 +35,7 @@ fn check_struct_sizes() { #[cfg(target_pointer_width = "64")] { assert_eq!(size_of::(), 536); - assert_eq!(size_of::(), 80); + assert_eq!(size_of::(), 64); assert_eq!(size_of::(), 56); assert_eq!( size_of::(), diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index ab39e9b9..8c5d4b1f 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -3,7 +3,7 @@ use crate::tokenizer::is_valid_identifier; use crate::types::dynamic::Variant; use crate::{ - Dynamic, Engine, FuncArgs, Identifier, Module, NativeCallContext, Position, RhaiError, + Dynamic, Engine, FuncArgs, ImmutableString, Module, NativeCallContext, Position, RhaiError, RhaiResult, RhaiResultOf, StaticVec, AST, ERR, }; #[cfg(feature = "no_std")] @@ -18,7 +18,7 @@ use std::{ /// to be passed onto a function during a call. #[derive(Clone, Hash)] pub struct FnPtr { - name: Identifier, + name: ImmutableString, curry: StaticVec, } @@ -42,13 +42,16 @@ impl fmt::Debug for FnPtr { impl FnPtr { /// Create a new function pointer. #[inline(always)] - pub fn new(name: impl Into) -> RhaiResultOf { + pub fn new(name: impl Into) -> RhaiResultOf { name.into().try_into() } /// Create a new function pointer without checking its parameters. #[inline(always)] #[must_use] - pub(crate) fn new_unchecked(name: impl Into, curry: StaticVec) -> Self { + pub(crate) fn new_unchecked( + name: impl Into, + curry: StaticVec, + ) -> Self { Self { name: name.into(), curry, @@ -63,13 +66,13 @@ impl FnPtr { /// Get the name of the function. #[inline(always)] #[must_use] - pub(crate) const fn fn_name_raw(&self) -> &Identifier { + pub(crate) const fn fn_name_raw(&self) -> &ImmutableString { &self.name } /// Get the underlying data of the function pointer. #[inline(always)] #[must_use] - pub(crate) fn take_data(self) -> (Identifier, StaticVec) { + pub(crate) fn take_data(self) -> (ImmutableString, StaticVec) { (self.name, self.curry) } /// Get the curried arguments. @@ -246,11 +249,11 @@ impl fmt::Display for FnPtr { } } -impl TryFrom for FnPtr { +impl TryFrom for FnPtr { type Error = RhaiError; - #[inline] - fn try_from(value: Identifier) -> RhaiResultOf { + #[inline(always)] + fn try_from(value: ImmutableString) -> RhaiResultOf { if is_valid_identifier(value.chars()) { Ok(Self { name: value, @@ -261,43 +264,3 @@ impl TryFrom for FnPtr { } } } - -impl TryFrom for FnPtr { - type Error = RhaiError; - - #[inline(always)] - fn try_from(value: crate::ImmutableString) -> RhaiResultOf { - let s: Identifier = value.into(); - Self::try_from(s) - } -} - -impl TryFrom for FnPtr { - type Error = RhaiError; - - #[inline(always)] - fn try_from(value: String) -> RhaiResultOf { - let s: Identifier = value.into(); - Self::try_from(s) - } -} - -impl TryFrom> for FnPtr { - type Error = RhaiError; - - #[inline(always)] - fn try_from(value: Box) -> RhaiResultOf { - let s: Identifier = value.into(); - Self::try_from(s) - } -} - -impl TryFrom<&str> for FnPtr { - type Error = RhaiError; - - #[inline(always)] - fn try_from(value: &str) -> RhaiResultOf { - let s: Identifier = value.into(); - Self::try_from(s) - } -} diff --git a/src/types/scope.rs b/src/types/scope.rs index 435042e8..ba699c24 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -1,7 +1,7 @@ //! Module that defines the [`Scope`] type representing a function call-stack scope. use super::dynamic::{AccessMode, Variant}; -use crate::{Dynamic, Identifier}; +use crate::{Dynamic, Identifier, ImmutableString}; use smallvec::SmallVec; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -75,7 +75,7 @@ pub struct Scope<'a, const N: usize = SCOPE_ENTRIES_INLINED> { /// Name of the entry. names: SmallVec<[Identifier; SCOPE_ENTRIES_INLINED]>, /// Aliases of the entry. - aliases: SmallVec<[Vec; SCOPE_ENTRIES_INLINED]>, + aliases: SmallVec<[Vec; SCOPE_ENTRIES_INLINED]>, /// Phantom to keep the lifetime parameter in order not to break existing code. dummy: PhantomData<&'a ()>, } @@ -125,7 +125,7 @@ impl Clone for Scope<'_> { } impl IntoIterator for Scope<'_> { - type Item = (String, Dynamic, Vec); + type Item = (String, Dynamic, Vec); type IntoIter = Box>; #[must_use] @@ -140,7 +140,7 @@ impl IntoIterator for Scope<'_> { } impl<'a> IntoIterator for &'a Scope<'_> { - type Item = (&'a Identifier, &'a Dynamic, &'a Vec); + type Item = (&'a Identifier, &'a Dynamic, &'a Vec); type IntoIter = Box + 'a>; #[must_use] @@ -669,7 +669,7 @@ impl Scope<'_> { /// Panics if the index is out of bounds. #[cfg(not(feature = "no_module"))] #[inline] - pub(crate) fn add_alias_by_index(&mut self, index: usize, alias: Identifier) -> &mut Self { + pub(crate) fn add_alias_by_index(&mut self, index: usize, alias: ImmutableString) -> &mut Self { let aliases = self.aliases.get_mut(index).unwrap(); if aliases.is_empty() || !aliases.contains(&alias) { aliases.push(alias); @@ -690,11 +690,11 @@ impl Scope<'_> { pub fn set_alias( &mut self, name: impl AsRef + Into, - alias: impl Into, + alias: impl Into, ) { if let Some(index) = self.search(name.as_ref()) { let alias = match alias.into() { - x if x.is_empty() => name.into(), + x if x.is_empty() => name.into().into(), x => x, }; self.add_alias_by_index(index, alias); @@ -727,7 +727,9 @@ impl Scope<'_> { } /// Get an iterator to entries in the [`Scope`]. #[allow(dead_code)] - pub(crate) fn into_iter(self) -> impl Iterator)> { + pub(crate) fn into_iter( + self, + ) -> impl Iterator)> { self.names .into_iter() .zip(self.values.into_iter().zip(self.aliases.into_iter())) From d97f3f7ec44b31b8321077de30d0ee00a6924f62 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 29 Oct 2022 15:17:12 +0800 Subject: [PATCH 11/11] Merge variables in Stmt::Share. --- src/ast/stmt.rs | 13 +++++-------- src/eval/stmt.rs | 34 ++++++++++++++++++---------------- src/parser.rs | 12 +++++++----- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index ed6c51a3..e272e1e9 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -605,19 +605,16 @@ pub enum Stmt { /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] Export(Box<(Ident, Ident)>, Position), - /// Convert a variable to shared. + /// Convert a list of variables to shared. /// /// Not available under `no_closure`. /// /// # Notes /// /// This variant does not map to any language structure. It is currently only used only to - /// convert a normal variable into a shared variable when the variable is _captured_ by a closure. + /// convert normal variables into shared variables when they are _captured_ by a closure. #[cfg(not(feature = "no_closure"))] - Share( - Box<(crate::ImmutableString, Option)>, - Position, - ), + Share(Box, Position)>>), } impl Default for Stmt { @@ -684,7 +681,7 @@ impl Stmt { Self::Export(.., pos) => *pos, #[cfg(not(feature = "no_closure"))] - Self::Share(.., pos) => *pos, + Self::Share(x) => x[0].2, } } /// Override the [position][Position] of this statement. @@ -716,7 +713,7 @@ impl Stmt { Self::Export(.., pos) => *pos = new_pos, #[cfg(not(feature = "no_closure"))] - Self::Share(.., pos) => *pos = new_pos, + Self::Share(x) => x.iter_mut().for_each(|(_, _, pos)| *pos = new_pos), } self diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 4eecbf88..0a302ee6 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -977,23 +977,25 @@ impl Engine { // Share statement #[cfg(not(feature = "no_closure"))] - Stmt::Share(x, pos) => { - let (name, index) = &**x; + Stmt::Share(x) => { + x.iter() + .try_for_each(|(name, index, pos)| { + if let Some(index) = index + .map(|n| scope.len() - n.get()) + .or_else(|| scope.search(name)) + { + let val = scope.get_mut_by_index(index); - if let Some(index) = index - .map(|n| scope.len() - n.get()) - .or_else(|| scope.search(name)) - { - let val = scope.get_mut_by_index(index); - - if !val.is_shared() { - // Replace the variable with a shared value. - *val = std::mem::take(val).into_shared(); - } - Ok(Dynamic::UNIT) - } else { - Err(ERR::ErrorVariableNotFound(name.to_string(), *pos).into()) - } + if !val.is_shared() { + // Replace the variable with a shared value. + *val = std::mem::take(val).into_shared(); + } + Ok(()) + } else { + Err(ERR::ErrorVariableNotFound(name.to_string(), *pos).into()) + } + }) + .map(|_| Dynamic::UNIT) } _ => unreachable!("statement cannot be evaluated: {:?}", stmt), diff --git a/src/parser.rs b/src/parser.rs index 9ca3a8a0..c2e02aa7 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3739,15 +3739,17 @@ impl Engine { // Convert the entire expression into a statement block, then insert the relevant // [`Share`][Stmt::Share] statements. - let mut statements = StaticVec::with_capacity(externals.len() + 1); - statements.extend( + let mut statements = StaticVec::with_capacity(2); + statements.push(Stmt::Share( externals .into_iter() .map(|crate::ast::Ident { name, pos }| { let (index, _) = parent.access_var(&name, lib, pos); - Stmt::Share((name, index).into(), pos) - }), - ); + (name, index, pos) + }) + .collect::>() + .into(), + )); statements.push(Stmt::Expr(expr.into())); Expr::Stmt(crate::ast::StmtBlock::new(statements, pos, Position::NONE).into()) }