From 7b92a80c32a68b52000a7745ac0e278d8eca6d6d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 30 Jan 2022 17:27:13 +0800 Subject: [PATCH 01/22] Fix encapsulated environment in module functions. --- CHANGELOG.md | 1 + src/ast/mod.rs | 3 ++ src/ast/script_fn.rs | 31 +++++++++++++++---- src/bin/rhai-repl.rs | 1 + src/eval/global_state.rs | 10 ++++-- src/eval/mod.rs | 3 ++ src/func/call.rs | 5 +-- src/func/script.rs | 67 +++++++++++++++++++++++++++------------- src/lib.rs | 5 +++ src/module/mod.rs | 34 +++++++------------- src/optimizer.rs | 3 +- src/parser.rs | 6 ++-- tests/modules.rs | 44 ++++++++++++++++++++++++++ 13 files changed, 150 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c9de46..8595e784 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Bug fixes * In `Scope::clone_visible`, constants are now properly cloned as constants. * Variables introduced inside `try` blocks are now properly cleaned up upon an exception. * Off-by-one error in character positions after a comment line is now fixed. +* Globally-defined constants are now encapsulated correctly inside a loaded module and no longer spill across call boundaries. Script-breaking changes ----------------------- diff --git a/src/ast/mod.rs b/src/ast/mod.rs index aebf79fc..009b1199 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -11,6 +11,9 @@ pub use ast::{ASTNode, AST}; pub use expr::{BinaryExpr, CustomExpr, Expr, FnCallExpr, FnCallHashes}; pub use flags::{FnAccess, OptionFlags, AST_OPTION_FLAGS}; pub use ident::Ident; +#[cfg(not(feature = "no_module"))] +#[cfg(not(feature = "no_function"))] +pub use script_fn::EncapsulatedEnviron; #[cfg(not(feature = "no_function"))] pub use script_fn::{ScriptFnDef, ScriptFnMetadata}; pub use stmt::{ConditionalStmtBlock, OpAssignment, Stmt, StmtBlock, SwitchCases, TryCatchBlock}; diff --git a/src/ast/script_fn.rs b/src/ast/script_fn.rs index a9d924e7..0daf388f 100644 --- a/src/ast/script_fn.rs +++ b/src/ast/script_fn.rs @@ -2,24 +2,43 @@ #![cfg(not(feature = "no_function"))] use super::{FnAccess, StmtBlock}; -use crate::{Identifier, Module, Shared, StaticVec}; +use crate::{Identifier, StaticVec}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{fmt, hash::Hash}; +/// _(internals)_ Encapsulated AST environment. +/// Exported under the `internals` feature only. +/// +/// 1) other functions defined within the same AST +/// 2) the stack of imported [modules][crate::Module] +/// 3) global constants +/// +/// Not available under `no_module` or `no_function`. +#[cfg(not(feature = "no_module"))] +#[cfg(not(feature = "no_function"))] +#[derive(Debug, Clone)] +pub struct EncapsulatedEnviron { + /// Functions defined within the same [`AST`][crate::AST]. + pub lib: crate::Shared, + /// Imported [modules][crate::Module]. + pub imports: Box<[(Identifier, crate::Shared)]>, + /// Globally-defined constants. + pub constants: Option, +} + /// _(internals)_ A type containing information on a script-defined function. /// Exported under the `internals` feature only. #[derive(Debug, Clone)] pub struct ScriptFnDef { /// Function body. pub body: StmtBlock, - /// Encapsulated running environment, if any. - pub lib: Option>, - /// Encapsulated stack of imported modules, if any. + /// Encapsulated AST environment, if any. /// - /// Not available under `no_module`. + /// Not available under `no_module` or `no_function`. #[cfg(not(feature = "no_module"))] - pub global: Option)]>>, + #[cfg(not(feature = "no_function"))] + pub environ: Option, /// Function name. pub name: Identifier, /// Function access mode. diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index 6e964182..9a6f68d3 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -295,6 +295,7 @@ fn main() { engine.set_module_resolver(resolver); } + // Register sample functions engine .register_fn("test", |x: INT, y: INT| format!("{} {}", x, y)) .register_fn("test", |x: &mut INT, y: INT, z: &str| { diff --git a/src/eval/global_state.rs b/src/eval/global_state.rs index fa7979b7..22f2574f 100644 --- a/src/eval/global_state.rs +++ b/src/eval/global_state.rs @@ -5,6 +5,12 @@ use crate::{Engine, Identifier}; use std::prelude::v1::*; use std::{fmt, marker::PhantomData}; +/// Collection of globally-defined constants. +#[cfg(not(feature = "no_module"))] +#[cfg(not(feature = "no_function"))] +pub type GlobalConstants = + crate::Shared>>; + /// _(internals)_ Global runtime states. /// Exported under the `internals` feature only. // @@ -44,9 +50,7 @@ pub struct GlobalRuntimeState<'a> { /// Interior mutability is needed because it is shared in order to aid in cloning. #[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_function"))] - pub(crate) constants: Option< - crate::Shared>>, - >, + pub(crate) constants: Option, /// Debugging interface. #[cfg(feature = "debugging")] pub debugger: super::Debugger, diff --git a/src/eval/mod.rs b/src/eval/mod.rs index b76619e4..0609c9b8 100644 --- a/src/eval/mod.rs +++ b/src/eval/mod.rs @@ -17,5 +17,8 @@ pub use debugger::CallStackFrame; pub use debugger::{BreakPoint, Debugger, DebuggerCommand, OnDebuggerCallback, OnDebuggingInit}; pub use eval_context::EvalContext; pub use eval_state::EvalState; +#[cfg(not(feature = "no_module"))] +#[cfg(not(feature = "no_function"))] +pub use global_state::GlobalConstants; pub use global_state::GlobalRuntimeState; pub use target::{calc_index, calc_offset_len, Target}; diff --git a/src/func/call.rs b/src/func/call.rs index f7c45e27..95ea4758 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -646,13 +646,12 @@ impl Engine { }; mem::swap(&mut global.source, &mut source); + let level = _level + 1; let result = if _is_method_call { // Method call of script function - map first argument to `this` let (first_arg, rest_args) = args.split_first_mut().unwrap(); - let level = _level + 1; - let result = self.call_script_fn( scope, global, @@ -679,8 +678,6 @@ impl Engine { .change_first_arg_to_copy(args); } - let level = _level + 1; - let result = self.call_script_fn( scope, global, state, lib, &mut None, func, args, pos, true, level, ); diff --git a/src/func/script.rs b/src/func/script.rs index c087053e..32d13f4d 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -4,7 +4,7 @@ use super::call::FnCallArgs; use crate::ast::ScriptFnDef; use crate::eval::{EvalState, GlobalRuntimeState}; -use crate::{Dynamic, Engine, Module, Position, RhaiError, RhaiResult, Scope, StaticVec, ERR}; +use crate::{Dynamic, Engine, Module, Position, RhaiError, RhaiResult, Scope, ERR}; use std::mem; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -41,13 +41,19 @@ impl Engine { err: RhaiError, pos: Position, ) -> RhaiResult { + let _fn_def = fn_def; + + #[cfg(not(feature = "no_module"))] + let source = _fn_def + .environ + .as_ref() + .and_then(|environ| environ.lib.id().map(str::to_string)); + #[cfg(feature = "no_module")] + let source = None; + Err(ERR::ErrorInFunctionCall( name, - fn_def - .lib - .as_ref() - .and_then(|m| m.id().map(str::to_string)) - .unwrap_or_else(|| global.source.to_string()), + source.unwrap_or_else(|| global.source.to_string()), err, pos, ) @@ -98,28 +104,39 @@ impl Engine { ); // Merge in encapsulated environment, if any - let mut lib_merged = StaticVec::with_capacity(lib.len() + 1); let orig_fn_resolution_caches_len = state.fn_resolution_caches_len(); - let lib = if let Some(ref fn_lib) = fn_def.lib { - if fn_lib.is_empty() { - lib - } else { - state.push_fn_resolution_cache(); - lib_merged.push(fn_lib.as_ref()); - lib_merged.extend(lib.iter().cloned()); - &lib_merged - } - } else { - lib - }; + #[cfg(not(feature = "no_module"))] + let mut lib_merged = crate::StaticVec::with_capacity(lib.len() + 1); #[cfg(not(feature = "no_module"))] - if let Some(ref modules) = fn_def.global { - for (n, m) in modules.iter().cloned() { + let (lib, constants) = if let Some(crate::ast::EncapsulatedEnviron { + lib: ref fn_lib, + ref imports, + #[cfg(not(feature = "no_function"))] + ref constants, + }) = fn_def.environ + { + for (n, m) in imports.iter().cloned() { global.push_import(n, m) } - } + ( + if fn_lib.is_empty() { + lib + } else { + state.push_fn_resolution_cache(); + lib_merged.push(fn_lib.as_ref()); + lib_merged.extend(lib.iter().cloned()); + &lib_merged + }, + #[cfg(not(feature = "no_function"))] + Some(mem::replace(&mut global.constants, constants.clone())), + #[cfg(feature = "no_function")] + None, + ) + } else { + (lib, None) + }; // Evaluate the function let result = self @@ -165,6 +182,12 @@ impl Engine { #[cfg(not(feature = "no_module"))] global.truncate_imports(orig_imports_len); + // Restore constants + #[cfg(not(feature = "no_function"))] + if let Some(constants) = constants { + global.constants = constants; + } + // Restore state state.rewind_fn_resolution_caches(orig_fn_resolution_caches_len); diff --git a/src/lib.rs b/src/lib.rs index 3e514dd8..e7d347d0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -259,6 +259,11 @@ pub use ast::{ AST_OPTION_FLAGS::*, }; +#[cfg(feature = "internals")] +#[cfg(not(feature = "no_module"))] +#[cfg(not(feature = "no_function"))] +pub use ast::EncapsulatedEnviron; + #[cfg(feature = "internals")] #[cfg(not(feature = "no_float"))] pub use ast::FloatWrapper; diff --git a/src/module/mod.rs b/src/module/mod.rs index e842fc1d..d9b16a17 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1696,33 +1696,21 @@ impl Module { let mut module = Module::new(); // Extra modules left become sub-modules - #[cfg(not(feature = "no_function"))] - let mut func_global = None; + let mut imports = StaticVec::new_const(); if result.is_ok() { global .scan_imports_raw() .skip(orig_imports_len) .for_each(|(k, m)| { - #[cfg(not(feature = "no_function"))] - if func_global.is_none() { - func_global = Some(StaticVec::new()); - } - #[cfg(not(feature = "no_function"))] - func_global - .as_mut() - .expect("`Some`") - .push((k.clone(), m.clone())); - + imports.push((k.clone(), m.clone())); module.set_sub_module(k.clone(), m.clone()); }); } // Restore global state #[cfg(not(feature = "no_function"))] - { - global.constants = orig_constants; - } + let constants = std::mem::replace(&mut global.constants, orig_constants); global.truncate_imports(orig_imports_len); global.source = orig_source; @@ -1747,12 +1735,9 @@ impl Module { } } - #[cfg(not(feature = "no_function"))] - let func_global = func_global.map(|v| v.into_boxed_slice()); - // Non-private functions defined become module functions #[cfg(not(feature = "no_function"))] - if ast.has_functions() { + { ast.shared_lib() .iter_fn() .filter(|&f| match f.metadata.access { @@ -1761,15 +1746,20 @@ impl Module { }) .filter(|&f| f.func.is_script()) .for_each(|f| { - // Encapsulate AST environment let mut func = f .func .get_script_fn_def() .expect("script-defined function") .as_ref() .clone(); - func.lib = Some(ast.shared_lib().clone()); - func.global = func_global.clone(); + + // Encapsulate AST environment + func.environ = Some(crate::ast::EncapsulatedEnviron { + lib: ast.shared_lib().clone(), + imports: imports.clone().into_boxed_slice(), + constants: constants.clone(), + }); + module.set_script_fn(func); }); } diff --git a/src/optimizer.rs b/src/optimizer.rs index 7e2cb6e7..d430ac76 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -1211,9 +1211,8 @@ pub fn optimize_into_ast( access: fn_def.access, body: crate::ast::StmtBlock::NONE, params: fn_def.params.clone(), - lib: None, #[cfg(not(feature = "no_module"))] - global: None, + environ: None, #[cfg(not(feature = "no_function"))] #[cfg(feature = "metadata")] comments: None, diff --git a/src/parser.rs b/src/parser.rs index 21d12147..a24af289 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3185,9 +3185,8 @@ fn parse_fn( access, params, body, - lib: None, #[cfg(not(feature = "no_module"))] - global: None, + environ: None, #[cfg(not(feature = "no_function"))] #[cfg(feature = "metadata")] comments: if comments.is_empty() { @@ -3341,9 +3340,8 @@ fn parse_anon_fn( access: crate::FnAccess::Public, params, body: body.into(), - lib: None, #[cfg(not(feature = "no_module"))] - global: None, + environ: None, #[cfg(not(feature = "no_function"))] #[cfg(feature = "metadata")] comments: None, diff --git a/tests/modules.rs b/tests/modules.rs index 9b3e62d4..db0038e7 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -514,3 +514,47 @@ fn test_module_file() -> Result<(), Box> { Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; Ok(()) } + +#[cfg(not(feature = "no_function"))] +#[test] +fn test_module_environ() -> Result<(), Box> { + let mut engine = Engine::new(); + + let ast = engine.compile( + r#" + const SECRET = 42; + + fn foo(x) { + print(global::SECRET); + global::SECRET + x + } + "#, + )?; + + let mut m = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; + + m.set_id("test"); + m.build_index(); + + engine.register_static_module("test", m.into()); + + assert_eq!( + engine.eval::( + r#" + const SECRET = "hello"; + + fn foo(x) { + print(global::SECRET); + global::SECRET + x + } + + let t = test::foo(0); + + foo(t) + "# + )?, + "hello42" + ); + + Ok(()) +} From ff06bb98a139210e9196e26272bc23dafc8d59e9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 30 Jan 2022 23:28:02 +0800 Subject: [PATCH 02/22] Fix no_module build. --- src/func/script.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/func/script.rs b/src/func/script.rs index 32d13f4d..e42a28fd 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -80,7 +80,6 @@ impl Engine { let orig_imports_len = global.num_imports(); #[cfg(feature = "debugging")] - #[cfg(not(feature = "no_function"))] let orig_call_stack_len = global.debugger.call_stack().len(); // Put arguments into scope as variables @@ -91,7 +90,6 @@ impl Engine { // Push a new call stack frame #[cfg(feature = "debugging")] - #[cfg(not(feature = "no_function"))] global.debugger.push_call_stack_frame( fn_def.name.clone(), scope @@ -113,7 +111,6 @@ impl Engine { let (lib, constants) = if let Some(crate::ast::EncapsulatedEnviron { lib: ref fn_lib, ref imports, - #[cfg(not(feature = "no_function"))] ref constants, }) = fn_def.environ { @@ -129,10 +126,7 @@ impl Engine { lib_merged.extend(lib.iter().cloned()); &lib_merged }, - #[cfg(not(feature = "no_function"))] Some(mem::replace(&mut global.constants, constants.clone())), - #[cfg(feature = "no_function")] - None, ) } else { (lib, None) @@ -183,7 +177,7 @@ impl Engine { global.truncate_imports(orig_imports_len); // Restore constants - #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_module"))] if let Some(constants) = constants { global.constants = constants; } @@ -193,7 +187,6 @@ impl Engine { // Pop the call stack #[cfg(feature = "debugging")] - #[cfg(not(feature = "no_function"))] global.debugger.rewind_call_stack(orig_call_stack_len); result From f1458e79e0db3f588fa844dc7bca47a45e6c09f5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 31 Jan 2022 13:38:27 +0800 Subject: [PATCH 03/22] Improve AST debug display. --- src/ast/ast.rs | 109 +++++++++++++++++++++++++++++---------------- src/ast/expr.rs | 38 +++++++++------- src/lib.rs | 2 +- src/parser.rs | 44 ++++++++++-------- src/types/error.rs | 4 +- tests/optimizer.rs | 17 ++----- 6 files changed, 124 insertions(+), 90 deletions(-) diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 948df827..18b02cff 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -1,10 +1,11 @@ //! Module defining the AST (abstract syntax tree). -use super::{Expr, FnAccess, Stmt, StmtBlock, AST_OPTION_FLAGS}; +use super::{Expr, FnAccess, Stmt, StmtBlock, AST_OPTION_FLAGS::*}; use crate::{Dynamic, FnNamespace, Identifier, Position, StaticVec}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ + fmt, hash::Hash, ops::{Add, AddAssign}, }; @@ -14,7 +15,7 @@ use std::{ /// # Thread Safety /// /// Currently, [`AST`] is neither `Send` nor `Sync`. Turn on the `sync` feature to make it `Send + Sync`. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct AST { /// Source of the [`AST`]. /// No source if string is empty. @@ -23,7 +24,7 @@ pub struct AST { body: StmtBlock, /// Script-defined functions. #[cfg(not(feature = "no_function"))] - functions: crate::Shared, + lib: crate::Shared, /// Embedded module resolver, if any. #[cfg(not(feature = "no_module"))] resolver: Option>, @@ -36,6 +37,31 @@ impl Default for AST { } } +impl fmt::Debug for AST { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut fp = f.debug_struct("AST"); + + if !self.source.is_empty() { + fp.field("source: ", &self.source); + } + #[cfg(not(feature = "no_module"))] + if let Some(ref resolver) = self.resolver { + fp.field("resolver: ", resolver); + } + + fp.field("body", &self.body.as_slice()); + + #[cfg(not(feature = "no_function"))] + if !self.lib.is_empty() { + for (_, _, _, _, ref fn_def) in self.lib.iter_script_fn() { + let sig = fn_def.to_string(); + fp.field(&sig, &fn_def.body.as_slice()); + } + } + fp.finish() + } +} + impl AST { /// Create a new [`AST`]. #[cfg(not(feature = "internals"))] @@ -49,7 +75,7 @@ impl AST { source: Identifier::new_const(), body: StmtBlock::new(statements, Position::NONE), #[cfg(not(feature = "no_function"))] - functions: functions.into(), + lib: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, } @@ -67,7 +93,7 @@ impl AST { source: Identifier::new_const(), body: StmtBlock::new(statements, Position::NONE), #[cfg(not(feature = "no_function"))] - functions: functions.into(), + lib: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, } @@ -115,7 +141,7 @@ impl AST { source: Identifier::new_const(), body: StmtBlock::NONE, #[cfg(not(feature = "no_function"))] - functions: crate::Module::new().into(), + lib: crate::Module::new().into(), #[cfg(not(feature = "no_module"))] resolver: None, } @@ -140,7 +166,7 @@ impl AST { 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.functions) + crate::Shared::get_mut(&mut self.lib) .as_mut() .map(|m| m.set_id(source.clone())); self.source = source; @@ -181,7 +207,7 @@ impl AST { #[inline(always)] #[must_use] pub fn has_functions(&self) -> bool { - !self.functions.is_empty() + !self.lib.is_empty() } /// Get the internal shared [`Module`][crate::Module] containing all script-defined functions. #[cfg(not(feature = "internals"))] @@ -189,7 +215,7 @@ impl AST { #[inline(always)] #[must_use] pub(crate) fn shared_lib(&self) -> &crate::Shared { - &self.functions + &self.lib } /// _(internals)_ Get the internal shared [`Module`][crate::Module] containing all script-defined functions. /// Exported under the `internals` feature only. @@ -200,7 +226,7 @@ impl AST { #[inline(always)] #[must_use] pub fn shared_lib(&self) -> &crate::Shared { - &self.functions + &self.lib } /// Get the embedded [module resolver][`ModuleResolver`]. #[cfg(not(feature = "internals"))] @@ -260,12 +286,12 @@ impl AST { &self, filter: impl Fn(FnNamespace, FnAccess, bool, &str, usize) -> bool, ) -> Self { - let mut functions = crate::Module::new(); - functions.merge_filtered(&self.functions, &filter); + let mut lib = crate::Module::new(); + lib.merge_filtered(&self.lib, &filter); Self { source: self.source.clone(), body: StmtBlock::NONE, - functions: functions.into(), + lib: lib.into(), #[cfg(not(feature = "no_module"))] resolver: self.resolver.clone(), } @@ -279,7 +305,7 @@ impl AST { source: self.source.clone(), body: self.body.clone(), #[cfg(not(feature = "no_function"))] - functions: crate::Module::new().into(), + lib: crate::Module::new().into(), #[cfg(not(feature = "no_module"))] resolver: self.resolver.clone(), } @@ -472,24 +498,24 @@ impl AST { }; #[cfg(not(feature = "no_function"))] - let functions = { - let mut functions = self.functions.as_ref().clone(); - functions.merge_filtered(&other.functions, &_filter); - functions + let lib = { + let mut lib = self.lib.as_ref().clone(); + lib.merge_filtered(&other.lib, &_filter); + lib }; if !other.source.is_empty() { Self::new_with_source( merged, #[cfg(not(feature = "no_function"))] - functions, + lib, other.source.clone(), ) } else { Self::new( merged, #[cfg(not(feature = "no_function"))] - functions, + lib, ) } } @@ -562,9 +588,9 @@ impl AST { self.body.extend(other.body.into_iter()); #[cfg(not(feature = "no_function"))] - if !other.functions.is_empty() { - crate::func::native::shared_make_mut(&mut self.functions) - .merge_filtered(&other.functions, &_filter); + if !other.lib.is_empty() { + crate::func::native::shared_make_mut(&mut self.lib) + .merge_filtered(&other.lib, &_filter); } self } @@ -599,20 +625,32 @@ impl AST { &mut self, filter: impl Fn(FnNamespace, FnAccess, &str, usize) -> bool, ) -> &mut Self { - if !self.functions.is_empty() { - crate::func::native::shared_make_mut(&mut self.functions) - .retain_script_functions(filter); + if !self.lib.is_empty() { + crate::func::native::shared_make_mut(&mut self.lib).retain_script_functions(filter); } self } + /// _(internals)_ Iterate through all function definitions. + /// Exported under the `internals` feature only. + /// + /// Not available under `no_function`. + #[cfg(feature = "internals")] + #[cfg(not(feature = "no_function"))] + #[inline] + pub fn iter_fn_def(&self) -> impl Iterator { + self.lib + .iter_script_fn() + .map(|(_, _, _, _, fn_def)| fn_def.as_ref()) + } /// Iterate through all function definitions. /// /// Not available under `no_function`. + #[cfg(not(feature = "internals"))] #[cfg(not(feature = "no_function"))] #[allow(dead_code)] #[inline] pub(crate) fn iter_fn_def(&self) -> impl Iterator { - self.functions + self.lib .iter_script_fn() .map(|(_, _, _, _, fn_def)| fn_def.as_ref()) } @@ -622,7 +660,7 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline] pub fn iter_functions<'a>(&'a self) -> impl Iterator + 'a { - self.functions + self.lib .iter_script_fn() .map(|(_, _, _, _, fn_def)| fn_def.as_ref().into()) } @@ -632,7 +670,7 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn clear_functions(&mut self) -> &mut Self { - self.functions = crate::Module::new().into(); + self.lib = crate::Module::new().into(); self } /// Clear all statements in the [`AST`], leaving only function definitions. @@ -707,16 +745,11 @@ impl AST { ) -> impl Iterator { self.statements().iter().filter_map(move |stmt| match stmt { Stmt::Var(expr, name, options, _) - if options.contains(AST_OPTION_FLAGS::AST_OPTION_CONSTANT) && include_constants - || !options.contains(AST_OPTION_FLAGS::AST_OPTION_CONSTANT) - && include_variables => + if options.contains(AST_OPTION_CONSTANT) && include_constants + || !options.contains(AST_OPTION_CONSTANT) && include_variables => { if let Some(value) = expr.get_literal_value() { - Some(( - name.as_str(), - options.contains(AST_OPTION_FLAGS::AST_OPTION_CONSTANT), - value, - )) + Some((name.as_str(), options.contains(AST_OPTION_CONSTANT), value)) } else { None } @@ -871,6 +904,6 @@ impl AST { #[inline(always)] #[must_use] pub fn lib(&self) -> &crate::Module { - &self.functions + &self.lib } } diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 72dea97c..a9881804 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -165,7 +165,7 @@ impl FnCallHashes { /// _(internals)_ A function call. /// Exported under the `internals` feature only. -#[derive(Debug, Clone, Default, Hash)] +#[derive(Clone, Default, Hash)] pub struct FnCallExpr { /// Namespace of the function, if any. /// @@ -193,6 +193,24 @@ pub struct FnCallExpr { pub capture_parent_scope: bool, } +impl fmt::Debug for FnCallExpr { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut ff = f.debug_struct("FnCallExpr"); + #[cfg(not(feature = "no_module"))] + self.namespace.as_ref().map(|ns| ff.field("namespace", ns)); + ff.field("name", &self.name) + .field("hash", &self.hashes) + .field("arg_exprs", &self.args); + if !self.constants.is_empty() { + ff.field("constant_args", &self.constants); + } + if self.capture_parent_scope { + ff.field("capture_parent_scope", &self.capture_parent_scope); + } + ff.finish() + } +} + impl FnCallExpr { /// Does this function call contain a qualified namespace? /// @@ -459,26 +477,12 @@ impl fmt::Debug for Expr { f.write_str(")") } Self::Property(x, _) => write!(f, "Property({})", x.2), - Self::Stack(x, _) => write!(f, "StackSlot({})", x), + Self::Stack(x, _) => write!(f, "ConstantArg#{}", x), Self::Stmt(x) => { f.write_str("ExprStmtBlock")?; f.debug_list().entries(x.iter()).finish() } - Self::FnCall(x, _) => { - let mut ff = f.debug_struct("FnCall"); - #[cfg(not(feature = "no_module"))] - x.namespace.as_ref().map(|ns| ff.field("namespace", ns)); - ff.field("name", &x.name) - .field("hash", &x.hashes) - .field("args", &x.args); - if !x.constants.is_empty() { - ff.field("constants", &x.constants); - } - if x.capture_parent_scope { - ff.field("capture_parent_scope", &x.capture_parent_scope); - } - ff.finish() - } + Self::FnCall(x, _) => fmt::Debug::fmt(x, f), Self::Index(x, term, pos) => { display_pos = *pos; diff --git a/src/lib.rs b/src/lib.rs index e7d347d0..6d19cde0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -256,7 +256,7 @@ pub use parser::ParseState; pub use ast::{ ASTNode, BinaryExpr, ConditionalStmtBlock, CustomExpr, Expr, FnCallExpr, FnCallHashes, Ident, OpAssignment, OptionFlags, ScriptFnDef, Stmt, StmtBlock, SwitchCases, TryCatchBlock, - AST_OPTION_FLAGS::*, + AST_OPTION_FLAGS, }; #[cfg(feature = "internals")] diff --git a/src/parser.rs b/src/parser.rs index a24af289..2efdccee 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -52,11 +52,11 @@ pub struct ParseState<'e> { pub entry_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] - pub external_vars: BTreeMap, + pub external_vars: Vec, /// An indicator that disables variable capturing into externals one single time /// up until the nearest consumed Identifier token. /// If set to false the next call to [`access_var`][ParseState::access_var] will not capture the variable. - /// All consequent calls to [`access_var`][ParseState::access_var] will not be affected + /// All consequent calls to [`access_var`][ParseState::access_var] will not be affected. #[cfg(not(feature = "no_closure"))] pub allow_capture: bool, /// Encapsulates a local stack with imported [module][crate::Module] names. @@ -85,7 +85,7 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_function"))] max_function_expr_depth: NonZeroUsize::new(engine.max_function_expr_depth()), #[cfg(not(feature = "no_closure"))] - external_vars: BTreeMap::new(), + external_vars: Vec::new(), #[cfg(not(feature = "no_closure"))] allow_capture: true, interned_strings: StringsInterner::new(), @@ -128,8 +128,11 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_closure"))] if self.allow_capture { - if index.is_none() && !self.external_vars.contains_key(name) { - self.external_vars.insert(name.into(), _pos); + if index.is_none() && !self.external_vars.iter().any(|v| v.name == name) { + self.external_vars.push(crate::ast::Ident { + name: name.into(), + pos: _pos, + }); } } else { self.allow_capture = true @@ -1258,12 +1261,14 @@ fn parse_primary( #[cfg(not(feature = "no_closure"))] new_state.external_vars.iter().try_for_each( - |(captured_var, &pos)| -> ParseResult<_> { - let index = state.access_var(captured_var, pos); + |crate::ast::Ident { name, pos }| -> ParseResult<_> { + let index = state.access_var(name, *pos); - if !settings.is_closure && settings.strict_var && index.is_none() { + if settings.strict_var && !settings.is_closure && index.is_none() { // If the parent scope is not inside another capturing closure - Err(PERR::VariableUndefined(captured_var.to_string()).into_err(pos)) + // then we can conclude that the captured variable doesn't exist. + // Under Strict Variables mode, this is not allowed. + Err(PERR::VariableUndefined(name.to_string()).into_err(*pos)) } else { Ok(()) } @@ -3311,20 +3316,21 @@ fn parse_anon_fn( // External variables may need to be processed in a consistent order, // so extract them into a list. #[cfg(not(feature = "no_closure"))] - let externals: StaticVec = state - .external_vars - .iter() - .map(|(name, _)| name.clone()) - .collect(); + let (mut params, externals) = { + let externals: StaticVec = state + .external_vars + .iter() + .map(|crate::ast::Ident { name, .. }| name.clone()) + .collect(); - #[cfg(not(feature = "no_closure"))] - let mut params = StaticVec::with_capacity(params_list.len() + externals.len()); + let mut params = StaticVec::with_capacity(params_list.len() + externals.len()); + params.extend(externals.iter().cloned()); + + (params, externals) + }; #[cfg(feature = "no_closure")] let mut params = StaticVec::with_capacity(params_list.len()); - #[cfg(not(feature = "no_closure"))] - params.extend(externals.iter().cloned()); - params.append(&mut params_list); // Create unique function name by hashing the script body plus the parameters. diff --git a/src/types/error.rs b/src/types/error.rs index da507a9b..8131b7b0 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -149,7 +149,7 @@ impl fmt::Display for EvalAltResult { "" => f.write_str("Malformed dot expression"), s => f.write_str(s), }?, - Self::ErrorIndexingType(s, _) => write!(f, "Indexer not registered for {}", s)?, + Self::ErrorIndexingType(s, _) => write!(f, "Indexer not registered: {}", s)?, Self::ErrorUnboundThis(_) => f.write_str("'this' is not bound")?, Self::ErrorFor(_) => f.write_str("For loop expects a type that is iterable")?, Self::ErrorTooManyOperations(_) => f.write_str("Too many operations")?, @@ -166,7 +166,7 @@ impl fmt::Display for EvalAltResult { } Self::ErrorRuntime(d, _) => write!(f, "Runtime error: {}", d)?, - Self::ErrorAssignmentToConstant(s, _) => write!(f, "Cannot modify constant {}", s)?, + Self::ErrorAssignmentToConstant(s, _) => write!(f, "Cannot modify constant: {}", s)?, Self::ErrorMismatchOutputType(s, r, _) => match (r.as_str(), s.as_str()) { ("", s) => write!(f, "Output type is incorrect, expecting {}", s), (r, "") => write!(f, "Output type is incorrect: {}", r), diff --git a/tests/optimizer.rs b/tests/optimizer.rs index b6d6a289..af9754f4 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -78,33 +78,24 @@ fn test_optimizer_parse() -> Result<(), Box> { let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert_eq!( - format!("{:?}", ast), - "AST { source: \"\", body: Block[Expr(123 @ 1:53)], functions: Module, resolver: None }" - ); + assert_eq!(format!("{:?}", ast), "AST { body: [Expr(123 @ 1:53)] }"); let ast = engine.compile("const DECISION = false; if DECISION { 42 } else { 123 }")?; assert_eq!( format!("{:?}", ast), - r#"AST { source: "", body: Block[Var(false @ 1:18, "DECISION" @ 1:7, (Constant), 1:1), Expr(123 @ 1:51)], functions: Module, resolver: None }"# + r#"AST { body: [Var(false @ 1:18, "DECISION" @ 1:7, (Constant), 1:1), Expr(123 @ 1:51)] }"# ); let ast = engine.compile("if 1 == 2 { 42 }")?; - assert_eq!( - format!("{:?}", ast), - "AST { source: \"\", body: Block[], functions: Module, resolver: None }" - ); + assert_eq!(format!("{:?}", ast), "AST { body: [] }"); engine.set_optimization_level(OptimizationLevel::Full); let ast = engine.compile("abs(-42)")?; - assert_eq!( - format!("{:?}", ast), - "AST { source: \"\", body: Block[Expr(42 @ 1:1)], functions: Module, resolver: None }" - ); + assert_eq!(format!("{:?}", ast), "AST { body: [Expr(42 @ 1:1)] }"); Ok(()) } From 389bb9bf66c3b8ec944e579867beeaf71f9eb665 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 31 Jan 2022 21:02:36 +0800 Subject: [PATCH 04/22] Add history recall to repl. --- CHANGELOG.md | 25 ++++++++- src/bin/rhai-repl.rs | 131 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 128 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8595e784..0afcd1a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,9 +29,32 @@ New features Enhancements ------------ -* `rhai-repl` tool has a few more commands, such as `strict` to turn on/off _Strict Variables Mode_ and `optimize` to turn on/off script optimization. * Default features for dependencies (such as `ahash/std` and `num-traits/std`) are no longer required. * The `no_module` feature now eliminates large sections of code via feature gates. +* Debug display of `AST` is improved. + +REPL tool changes +----------------- + +The REPL bin tool, `rhai-rpl`, has been enhanced. + +### Build changes + +* The `rustyline` feature is now required in order to build `rhai-repl`. +* Therefore, `rhai-repl` is no longer automatically built when using a simple `cargo build` with default features. + +### Line editor + +* `rhai-repl` now uses [`rustyline`](https://crates.io/crates/rustyline) as a line editor with history. +* Shift-Enter can now be used to enter multiple lines without having to attach the `\` continuation character the end of each line. + +### New commands + +* `strict` to turn on/off _Strict Variables Mode_. +* `optimize` to turn on/off script optimization. +* `history` to print lines history. +* `!!`, `!`_num_ and `!`_text_ to recall a history line. +* `keys` to print all key bindings. Version 1.4.1 diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index 9a6f68d3..c23eab2e 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -44,9 +44,13 @@ fn print_error(input: &str, mut err: EvalAltResult) { /// Print help text. fn print_help() { println!("help => print this help"); - println!("keys => print list of key bindings"); println!("quit, exit => quit"); + println!("keys => print list of key bindings"); println!("history => print lines history"); + println!("!! => repeat the last history line"); + println!("!<#> => repeat a particular history line"); + println!("! => repeat the last history line starting with some text"); + println!("!? => repeat the last history line containing some text"); println!("scope => print all variables in the scope"); println!("strict => toggle on/off Strict Variables Mode"); #[cfg(not(feature = "no_optimize"))] @@ -311,6 +315,10 @@ fn main() { // REPL loop let mut input = String::new(); + let mut replacement = None; + let mut replacement_index = 0; + let mut history_offset = 1; + let mut main_ast = AST::empty(); let mut ast_u = AST::empty(); let mut ast = AST::empty(); @@ -318,35 +326,53 @@ fn main() { print_help(); 'main_loop: loop { - input.clear(); - - loop { - let prompt = if input.is_empty() { - "rhai-repl> " + if let Some(replace) = replacement.take() { + input = replace; + if rl.add_history_entry(input.clone()) { + history_offset += 1; + } + if input.contains('\n') { + println!("[{}] ~~~~", replacement_index); + println!("{}", input); + println!("~~~~"); } else { - " > " - }; + println!("[{}] {}", replacement_index, input); + } + replacement_index = 0; + } else { + input.clear(); - match rl.readline(prompt) { - // Line continuation - Ok(mut line) if line.ends_with("\\") => { - line.pop(); - input += line.trim_end(); - input.push('\n'); - } - Ok(line) => { - input += line.trim_end(); - if !input.is_empty() { - rl.add_history_entry(input.clone()); + loop { + let prompt = if input.is_empty() { + "rhai-repl> " + } else { + " > " + }; + + match rl.readline(prompt) { + // Line continuation + Ok(mut line) if line.ends_with("\\") => { + line.pop(); + input += line.trim_end(); + input.push('\n'); + } + Ok(line) => { + input += line.trim_end(); + if !input.is_empty() && !input.starts_with('!') && input.trim() != "history" + { + if rl.add_history_entry(input.clone()) { + history_offset += 1; + } + } + break; } - break; - } - Err(ReadlineError::Interrupted) | Err(ReadlineError::Eof) => break 'main_loop, + Err(ReadlineError::Interrupted) | Err(ReadlineError::Eof) => break 'main_loop, - Err(err) => { - eprintln!("Error: {:?}", err); - break 'main_loop; + Err(err) => { + eprintln!("Error: {:?}", err); + break 'main_loop; + } } } } @@ -371,10 +397,10 @@ fn main() { "history" => { for (i, h) in rl.history().iter().enumerate() { match &h.split('\n').collect::>()[..] { - [line] => println!("[{}] {}", i + 1, line), + [line] => println!("[{}] {}", history_offset + i, line), lines => { for (x, line) in lines.iter().enumerate() { - let number = format!("[{}]", i + 1); + let number = format!("[{}]", history_offset + i); if x == 0 { println!("{} {}", number, line.trim_end()); } else { @@ -447,6 +473,57 @@ fn main() { ); continue; } + "!!" => { + if let Some(line) = rl.history().last() { + replacement = Some(line.clone()); + replacement_index = history_offset + rl.history().len() - 1; + } else { + eprintln!("No lines history!"); + } + continue; + } + _ if script.starts_with("!?") => { + let text = script[2..].trim(); + if let Some((n, line)) = rl + .history() + .iter() + .rev() + .enumerate() + .find(|&(_, h)| h.contains(text)) + { + replacement = Some(line.clone()); + replacement_index = history_offset + (rl.history().len() - 1 - n); + } else { + eprintln!("History line not found: {}", text); + } + continue; + } + _ if script.starts_with('!') => { + if let Ok(num) = script[1..].parse::() { + if num >= history_offset { + if let Some(line) = rl.history().get(num - history_offset) { + replacement = Some(line.clone()); + replacement_index = num; + continue; + } + } + } else { + let prefix = script[1..].trim(); + if let Some((n, line)) = rl + .history() + .iter() + .rev() + .enumerate() + .find(|&(_, h)| h.trim_start().starts_with(prefix)) + { + replacement = Some(line.clone()); + replacement_index = history_offset + (rl.history().len() - 1 - n); + continue; + } + } + eprintln!("History line not found: {}", &script[1..]); + continue; + } _ => (), } From dca0185323e08b247b09eea567b5650f0405f363 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 1 Feb 2022 14:07:06 +0800 Subject: [PATCH 05/22] Change on_debugger to register_debugger. --- src/api/events.rs | 2 +- src/bin/rhai-dbg.rs | 2 +- tests/debugging.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/api/events.rs b/src/api/events.rs index 6fb5227c..4077b506 100644 --- a/src/api/events.rs +++ b/src/api/events.rs @@ -264,7 +264,7 @@ impl Engine { /// Exported under the `debugging` feature only. #[cfg(feature = "debugging")] #[inline(always)] - pub fn on_debugger( + pub fn register_debugger( &mut self, init: impl Fn() -> Dynamic + SendSync + 'static, callback: impl Fn( diff --git a/src/bin/rhai-dbg.rs b/src/bin/rhai-dbg.rs index a506b640..df668bfc 100644 --- a/src/bin/rhai-dbg.rs +++ b/src/bin/rhai-dbg.rs @@ -220,7 +220,7 @@ fn main() { // Hook up debugger let lines: Vec<_> = script.trim().split('\n').map(|s| s.to_string()).collect(); - engine.on_debugger( + engine.register_debugger( // Store the current source in the debugger state || "".into(), // Main debugging interface diff --git a/tests/debugging.rs b/tests/debugging.rs index a329a533..d0ed9cfe 100644 --- a/tests/debugging.rs +++ b/tests/debugging.rs @@ -41,7 +41,7 @@ fn test_debugging() -> Result<(), Box> { fn test_debugger_state() -> Result<(), Box> { let mut engine = Engine::new(); - engine.on_debugger( + engine.register_debugger( || { // Say, use an object map for the debugger state let mut state = Map::new(); From 7163a7331a3feca1bf12a5f1cb552ca4799027d6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 1 Feb 2022 22:30:05 +0800 Subject: [PATCH 06/22] Add commands and status to debugging interface. --- src/api/events.rs | 1 + src/bin/rhai-dbg.rs | 178 +++++++++++++++++++++++----------------- src/eval/chaining.rs | 78 ++++++++++++------ src/eval/debugger.rs | 187 ++++++++++++++++++++++++++++--------------- src/eval/expr.rs | 18 +++-- src/eval/mod.rs | 5 +- src/eval/stmt.rs | 7 +- src/func/call.rs | 17 +++- src/func/script.rs | 39 ++++++--- src/lib.rs | 2 +- tests/debugging.rs | 2 +- 11 files changed, 351 insertions(+), 183 deletions(-) diff --git a/src/api/events.rs b/src/api/events.rs index 4077b506..53de16f5 100644 --- a/src/api/events.rs +++ b/src/api/events.rs @@ -269,6 +269,7 @@ impl Engine { init: impl Fn() -> Dynamic + SendSync + 'static, callback: impl Fn( &mut EvalContext, + crate::eval::DebuggerEvent, crate::ast::ASTNode, Option<&str>, Position, diff --git a/src/bin/rhai-dbg.rs b/src/bin/rhai-dbg.rs index df668bfc..c90881cf 100644 --- a/src/bin/rhai-dbg.rs +++ b/src/bin/rhai-dbg.rs @@ -1,4 +1,4 @@ -use rhai::debugger::DebuggerCommand; +use rhai::debugger::{BreakPoint, DebuggerCommand, DebuggerEvent}; use rhai::{Dynamic, Engine, EvalAltResult, ImmutableString, Position, Scope}; use std::{ @@ -36,6 +36,32 @@ fn print_source(lines: &[String], pos: Position, offset: usize) { } } +fn print_current_source( + context: &mut rhai::EvalContext, + source: Option<&str>, + pos: Position, + lines: &Vec, +) { + let current_source = &mut *context + .global_runtime_state_mut() + .debugger + .state_mut() + .write_lock::() + .unwrap(); + let src = source.unwrap_or(""); + if src != current_source { + println!(">>> Source => {}", source.unwrap_or("main script")); + *current_source = src.into(); + } + if !src.is_empty() { + // Print just a line number for imported modules + println!("{} @ {:?}", src, pos); + } else { + // Print the current source line + print_source(lines, pos, 0); + } +} + /// Pretty-print error. fn print_error(input: &str, mut err: EvalAltResult) { let lines: Vec<_> = input.trim().split('\n').collect(); @@ -71,36 +97,38 @@ fn print_error(input: &str, mut err: EvalAltResult) { /// Print debug help. fn print_debug_help() { - println!("help => print this help"); - println!("quit, exit, kill => quit"); - println!("scope => print the scope"); - println!("print => print all variables de-duplicated"); - println!("print => print the current value of a variable"); + println!("help, h => print this help"); + println!("quit, q, exit, kill => quit"); + println!("scope => print the scope"); + println!("print, p => print all variables de-duplicated"); + println!("print/p => print the current value of a variable"); #[cfg(not(feature = "no_module"))] - println!("imports => print all imported modules"); - println!("node => print the current AST node"); - println!("backtrace => print the current call-stack"); - println!("breakpoints => print all break-points"); - println!("enable => enable a break-point"); - println!("disable => disable a break-point"); - println!("delete => delete a break-point"); - println!("clear => delete all break-points"); + println!("imports => print all imported modules"); + println!("node => print the current AST node"); + println!("list, l => print the current source line"); + println!("backtrace, bt => print the current call-stack"); + println!("info break, i b => print all break-points"); + println!("enable/en => enable a break-point"); + println!("disable/dis => disable a break-point"); + println!("delete, d => delete all break-points"); + println!("delete/d => delete a break-point"); #[cfg(not(feature = "no_position"))] - println!("break => set a new break-point at the current position"); + println!("break, b => set a new break-point at the current position"); #[cfg(not(feature = "no_position"))] - println!("break => set a new break-point at a line number"); + println!("break/b => set a new break-point at a line number"); #[cfg(not(feature = "no_object"))] - println!("break . => set a new break-point for a property access"); - println!("break => set a new break-point for a function call"); + println!("break/b . => set a new break-point for a property access"); + println!("break/b => set a new break-point for a function call"); println!( - "break <#args> => set a new break-point for a function call with #args arguments" + "break/b <#args> => set a new break-point for a function call with #args arguments" ); - println!("throw [message] => throw an exception (message optional)"); - println!("run => restart the script evaluation from beginning"); - println!("step => go to the next expression, diving into functions"); - println!("over => go to the next expression, skipping oer functions"); - println!("next => go to the next statement, skipping over functions"); - println!("continue => continue normal execution"); + println!("throw [message] => throw an exception (message optional)"); + println!("run, r => restart the script evaluation from beginning"); + println!("step, s => go to the next expression, diving into functions"); + println!("over => go to the next expression, skipping oer functions"); + println!("next, n, => go to the next statement, skipping over functions"); + println!("finish, f => continue until the end of the current function call"); + println!("continue, c => continue normal execution"); println!(); } @@ -224,32 +252,34 @@ fn main() { // Store the current source in the debugger state || "".into(), // Main debugging interface - move |context, node, source, pos| { - { - let current_source = &mut *context - .global_runtime_state_mut() - .debugger - .state_mut() - .write_lock::() - .unwrap(); - - let src = source.unwrap_or(""); - - // Check source - if src != current_source { - println!(">>> Source => {}", source.unwrap_or("main script")); - *current_source = src.into(); + move |context, event, node, source, pos| { + match event { + DebuggerEvent::Step => (), + DebuggerEvent::BreakPoint(n) => { + match context.global_runtime_state().debugger.break_points()[n] { + #[cfg(not(feature = "no_position"))] + BreakPoint::AtPosition { .. } => (), + BreakPoint::AtFunctionName { ref name, .. } + | BreakPoint::AtFunctionCall { ref name, .. } => { + println!("! Call to function {}.", name) + } + #[cfg(not(feature = "no_object"))] + BreakPoint::AtProperty { ref name, .. } => { + println!("! Property {} accessed.", name) + } + } } - - if !src.is_empty() { - // Print just a line number for imported modules - println!("{} @ {:?}", src, pos); - } else { - // Print the current source line - print_source(&lines, pos, 0); + DebuggerEvent::FunctionExitWithValue(r) => { + println!("! Return from function call = {}", r) + } + DebuggerEvent::FunctionExitWithError(err) => { + println!("! Return from function call with error: {}", err) } } + // Print current source line + print_current_source(context, source, pos, &lines); + // Read stdin for commands let mut input = String::new(); @@ -267,8 +297,8 @@ fn main() { .collect::>() .as_slice() { - ["help", ..] => print_debug_help(), - ["exit", ..] | ["quit", ..] | ["kill", ..] => { + ["help" | "h", ..] => print_debug_help(), + ["exit" | "quit" | "q" | "kill", ..] => { println!("Script terminated. Bye!"); exit(0); } @@ -276,12 +306,14 @@ fn main() { println!("{:?} {}@{:?}", node, source.unwrap_or_default(), pos); println!(); } - ["continue", ..] => break Ok(DebuggerCommand::Continue), - [] | ["step", ..] => break Ok(DebuggerCommand::StepInto), + ["list" | "l", ..] => print_current_source(context, source, pos, &lines), + ["continue" | "c", ..] => break Ok(DebuggerCommand::Continue), + ["finish" | "f", ..] => break Ok(DebuggerCommand::FunctionExit), + [] | ["step" | "s", ..] => break Ok(DebuggerCommand::StepInto), ["over", ..] => break Ok(DebuggerCommand::StepOver), - ["next", ..] => break Ok(DebuggerCommand::Next), + ["next" | "n", ..] => break Ok(DebuggerCommand::Next), ["scope", ..] => print_scope(context.scope(), false), - ["print", var_name, ..] => { + ["print" | "p", var_name, ..] => { if let Some(value) = context.scope().get_value::(var_name) { if value.is::<()>() { println!("=> ()"); @@ -292,7 +324,7 @@ fn main() { eprintln!("Variable not found: {}", var_name); } } - ["print", ..] => print_scope(context.scope(), true), + ["print" | "p"] => print_scope(context.scope(), true), #[cfg(not(feature = "no_module"))] ["imports", ..] => { for (i, (name, module)) in context @@ -311,7 +343,7 @@ fn main() { println!(); } #[cfg(not(feature = "no_function"))] - ["backtrace", ..] => { + ["backtrace" | "bt", ..] => { for frame in context .global_runtime_state() .debugger @@ -322,15 +354,7 @@ fn main() { println!("{}", frame) } } - ["clear", ..] => { - context - .global_runtime_state_mut() - .debugger - .break_points_mut() - .clear(); - println!("All break-points cleared."); - } - ["breakpoints", ..] => Iterator::for_each( + ["info", "break", ..] | ["i", "b", ..] => Iterator::for_each( context .global_runtime_state() .debugger @@ -347,7 +371,7 @@ fn main() { _ => println!("[{}] {}", i + 1, bp), }, ), - ["enable", n, ..] => { + ["enable" | "en", n, ..] => { if let Ok(n) = n.parse::() { let range = 1..=context .global_runtime_state_mut() @@ -370,7 +394,7 @@ fn main() { eprintln!("Invalid break-point: '{}'", n); } } - ["disable", n, ..] => { + ["disable" | "dis", n, ..] => { if let Ok(n) = n.parse::() { let range = 1..=context .global_runtime_state_mut() @@ -393,7 +417,7 @@ fn main() { eprintln!("Invalid break-point: '{}'", n); } } - ["delete", n, ..] => { + ["delete" | "d", n, ..] => { if let Ok(n) = n.parse::() { let range = 1..=context .global_runtime_state_mut() @@ -414,7 +438,15 @@ fn main() { eprintln!("Invalid break-point: '{}'", n); } } - ["break", fn_name, args, ..] => { + ["delete" | "d", ..] => { + context + .global_runtime_state_mut() + .debugger + .break_points_mut() + .clear(); + println!("All break-points deleted."); + } + ["break" | "b", fn_name, args, ..] => { if let Ok(args) = args.parse::() { let bp = rhai::debugger::BreakPoint::AtFunctionCall { name: fn_name.trim().into(), @@ -433,7 +465,7 @@ fn main() { } // Property name #[cfg(not(feature = "no_object"))] - ["break", param] if param.starts_with('.') && param.len() > 1 => { + ["break" | "b", param] if param.starts_with('.') && param.len() > 1 => { let bp = rhai::debugger::BreakPoint::AtProperty { name: param[1..].into(), enabled: true, @@ -447,7 +479,7 @@ fn main() { } // Numeric parameter #[cfg(not(feature = "no_position"))] - ["break", param] if param.parse::().is_ok() => { + ["break" | "b", param] if param.parse::().is_ok() => { let n = param.parse::().unwrap(); let range = if source.is_none() { 1..=lines.len() @@ -472,7 +504,7 @@ fn main() { } } // Function name parameter - ["break", param] => { + ["break" | "b", param] => { let bp = rhai::debugger::BreakPoint::AtFunctionName { name: param.trim().into(), enabled: true, @@ -485,7 +517,7 @@ fn main() { .push(bp); } #[cfg(not(feature = "no_position"))] - ["break", ..] => { + ["break" | "b"] => { let bp = rhai::debugger::BreakPoint::AtPosition { source: source.unwrap_or("").into(), pos, @@ -505,7 +537,7 @@ fn main() { let msg = input.trim().splitn(2, ' ').skip(1).next().unwrap_or(""); break Err(EvalAltResult::ErrorRuntime(msg.trim().into(), pos).into()); } - ["run", ..] => { + ["run" | "r", ..] => { println!("Restarting script..."); break Err(EvalAltResult::ErrorTerminated(Dynamic::UNIT, pos).into()); } diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 53cd1767..43a4e4a8 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -156,7 +156,9 @@ impl Engine { if !_terminate_chaining => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; + } let mut idx_val_for_setter = idx_val.clone(); let idx_pos = x.lhs.position(); @@ -204,7 +206,9 @@ impl Engine { // xxx[rhs] op= new_val _ if new_val.is_some() => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; + } let ((new_val, new_pos), (op_info, op_pos)) = new_val.expect("`Some`"); let mut idx_val_for_setter = idx_val.clone(); @@ -249,7 +253,9 @@ impl Engine { // xxx[rhs] _ => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; + } self.get_indexed_mut( global, state, lib, target, idx_val, pos, false, true, level, @@ -268,9 +274,13 @@ impl Engine { let call_args = &mut idx_val.into_fn_call_args(); #[cfg(feature = "debugging")] - let reset_debugger = self.run_debugger_with_reset( - scope, global, state, lib, this_ptr, rhs, level, - )?; + let reset_debugger = if self.debugger.is_some() { + self.run_debugger_with_reset( + scope, global, state, lib, this_ptr, rhs, level, + )? + } else { + None + }; let result = self.make_method_call( global, state, lib, name, *hashes, target, call_args, *pos, level, @@ -292,7 +302,9 @@ impl Engine { // {xxx:map}.id op= ??? Expr::Property(x, pos) if target.is::() && new_val.is_some() => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + } let index = x.2.clone().into(); let ((new_val, new_pos), (op_info, op_pos)) = new_val.expect("`Some`"); @@ -312,7 +324,9 @@ impl Engine { // {xxx:map}.id Expr::Property(x, pos) if target.is::() => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + } let index = x.2.clone().into(); let val = self.get_indexed_mut( @@ -323,7 +337,9 @@ impl Engine { // xxx.id op= ??? Expr::Property(x, pos) if new_val.is_some() => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + } let ((getter, hash_get), (setter, hash_set), name) = x.as_ref(); let ((mut new_val, new_pos), (op_info, op_pos)) = new_val.expect("`Some`"); @@ -402,7 +418,9 @@ impl Engine { // xxx.id Expr::Property(x, pos) => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; + } let ((getter, hash_get), _, name) = x.as_ref(); let hash = crate::ast::FnCallHashes::from_native(*hash_get); @@ -442,9 +460,11 @@ impl Engine { let val_target = &mut match x.lhs { Expr::Property(ref p, pos) => { #[cfg(feature = "debugging")] - self.run_debugger( - scope, global, state, lib, this_ptr, _node, level, - )?; + if self.debugger.is_some() { + self.run_debugger( + scope, global, state, lib, this_ptr, _node, level, + )?; + } let index = p.2.clone().into(); self.get_indexed_mut( @@ -457,9 +477,13 @@ impl Engine { let call_args = &mut idx_val.into_fn_call_args(); #[cfg(feature = "debugging")] - let reset_debugger = self.run_debugger_with_reset( - scope, global, state, lib, this_ptr, _node, level, - )?; + let reset_debugger = if self.debugger.is_some() { + self.run_debugger_with_reset( + scope, global, state, lib, this_ptr, _node, level, + )? + } else { + None + }; let result = self.make_method_call( global, state, lib, name, *hashes, target, call_args, pos, @@ -494,9 +518,11 @@ impl Engine { // xxx.prop[expr] | xxx.prop.expr Expr::Property(ref p, pos) => { #[cfg(feature = "debugging")] - self.run_debugger( - scope, global, state, lib, this_ptr, _node, level, - )?; + if self.debugger.is_some() { + self.run_debugger( + scope, global, state, lib, this_ptr, _node, level, + )?; + } let ((getter, hash_get), (setter, hash_set), name) = p.as_ref(); let rhs_chain = rhs.into(); @@ -597,9 +623,13 @@ impl Engine { let args = &mut idx_val.into_fn_call_args(); #[cfg(feature = "debugging")] - let reset_debugger = self.run_debugger_with_reset( - scope, global, state, lib, this_ptr, _node, level, - )?; + let reset_debugger = if self.debugger.is_some() { + self.run_debugger_with_reset( + scope, global, state, lib, this_ptr, _node, level, + )? + } else { + None + }; let result = self.make_method_call( global, state, lib, name, *hashes, target, args, pos, level, @@ -664,7 +694,9 @@ impl Engine { // id.??? or id[???] Expr::Variable(_, var_pos, x) => { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, lhs, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, lhs, level)?; + } #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, *var_pos)?; diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index b6c1e487..28c86e4d 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -3,7 +3,7 @@ use super::{EvalContext, EvalState, GlobalRuntimeState}; use crate::ast::{ASTNode, Expr, Stmt}; -use crate::{Dynamic, Engine, Identifier, Module, Position, RhaiResultOf, Scope}; +use crate::{Dynamic, Engine, EvalAltResult, Identifier, Module, Position, RhaiResultOf, Scope}; use std::fmt; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -17,11 +17,22 @@ pub type OnDebuggingInit = dyn Fn() -> Dynamic + Send + Sync; /// Callback function for debugging. #[cfg(not(feature = "sync"))] -pub type OnDebuggerCallback = - dyn Fn(&mut EvalContext, ASTNode, Option<&str>, Position) -> RhaiResultOf; +pub type OnDebuggerCallback = dyn Fn( + &mut EvalContext, + DebuggerEvent, + ASTNode, + Option<&str>, + Position, +) -> RhaiResultOf; /// Callback function for debugging. #[cfg(feature = "sync")] -pub type OnDebuggerCallback = dyn Fn(&mut EvalContext, ASTNode, Option<&str>, Position) -> RhaiResultOf +pub type OnDebuggerCallback = dyn Fn( + &mut EvalContext, + DebuggerEvent, + ASTNode, + Option<&str>, + Position, + ) -> RhaiResultOf + Send + Sync; @@ -36,6 +47,30 @@ pub enum DebuggerCommand { StepOver, // Run to the next statement, skipping over functions. Next, + // Run to the end of the current function call. + FunctionExit, +} + +/// The debugger status. +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] +pub enum DebuggerStatus { + // Stop at the next statement or expression. + Next(bool, bool), + // Run to the end of the current function call. + FunctionExit(usize), +} + +/// A event that triggers the debugger. +#[derive(Debug, Clone, Copy)] +pub enum DebuggerEvent<'a> { + // Break on next step. + Step, + // Break on break-point. + BreakPoint(usize), + // Return from a function with a value. + FunctionExitWithValue(&'a Dynamic), + // Return from a function with a value. + FunctionExitWithError(&'a EvalAltResult), } /// A break-point for debugging. @@ -198,7 +233,7 @@ impl fmt::Display for CallStackFrame { #[derive(Debug, Clone, Hash)] pub struct Debugger { /// The current status command. - status: DebuggerCommand, + status: DebuggerStatus, /// The current state. state: Dynamic, /// The current set of break-points. @@ -215,9 +250,9 @@ impl Debugger { pub fn new(engine: &Engine) -> Self { Self { status: if engine.debugger.is_some() { - DebuggerCommand::StepInto + DebuggerStatus::Next(true, true) } else { - DebuggerCommand::Continue + DebuggerStatus::Next(false, false) }, state: if let Some((ref init, _)) = engine.debugger { init() @@ -280,31 +315,26 @@ impl Debugger { /// Get the current status of this [`Debugger`]. #[inline(always)] #[must_use] - pub fn status(&self) -> DebuggerCommand { + pub(crate) fn status(&self) -> DebuggerStatus { self.status } - /// Get a mutable reference to the current status of this [`Debugger`]. - #[inline(always)] - #[must_use] - pub fn status_mut(&mut self) -> &mut DebuggerCommand { - &mut self.status - } /// Set the status of this [`Debugger`]. #[inline(always)] - pub fn reset_status(&mut self, status: Option) { + pub(crate) fn reset_status(&mut self, status: Option) { if let Some(cmd) = status { self.status = cmd; } } - /// Does a particular [`AST` Node][ASTNode] trigger a break-point? + /// Returns the first break-point triggered by a particular [`AST` Node][ASTNode]. #[must_use] - pub fn is_break_point(&self, src: &str, node: ASTNode) -> bool { + pub fn is_break_point(&self, src: &str, node: ASTNode) -> Option { let _src = src; self.break_points() .iter() - .filter(|&bp| bp.is_enabled()) - .any(|bp| match bp { + .enumerate() + .filter(|&(_, bp)| bp.is_enabled()) + .find(|&(_, bp)| match bp { #[cfg(not(feature = "no_position"))] BreakPoint::AtPosition { pos, .. } if pos.is_none() => false, #[cfg(not(feature = "no_position"))] @@ -333,6 +363,7 @@ impl Debugger { _ => false, }, }) + .map(|(i, _)| i) } /// Get a slice of all [`BreakPoint`]'s. #[inline(always)] @@ -371,14 +402,9 @@ impl Engine { } /// Run the debugger callback. /// - /// Returns `true` if the debugger needs to be reactivated at the end of the block, statement or + /// Returns `Some` if the debugger needs to be reactivated at the end of the block, statement or /// function call. /// - /// # Note - /// - /// When the debugger callback return [`DebuggerCommand::StepOver`], the debugger if temporarily - /// disabled and `true` is returned. - /// /// It is up to the [`Engine`] to reactivate the debugger. #[inline] #[must_use] @@ -391,61 +417,94 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, node: impl Into>, level: usize, - ) -> RhaiResultOf> { - if let Some((_, ref on_debugger)) = self.debugger { - let node = node.into(); + ) -> RhaiResultOf> { + let node = node.into(); - // Skip transitive nodes - match node { - ASTNode::Expr(Expr::Stmt(_)) | ASTNode::Stmt(Stmt::Expr(_)) => return Ok(None), - _ => (), - } + // Skip transitive nodes + match node { + ASTNode::Expr(Expr::Stmt(_)) | ASTNode::Stmt(Stmt::Expr(_)) => return Ok(None), + _ => (), + } - let stop = match global.debugger.status { - DebuggerCommand::Continue => false, - DebuggerCommand::Next => matches!(node, ASTNode::Stmt(_)), - DebuggerCommand::StepInto | DebuggerCommand::StepOver => true, - }; + let stop = match global.debugger.status { + DebuggerStatus::Next(false, false) => false, + DebuggerStatus::Next(true, false) => matches!(node, ASTNode::Stmt(_)), + DebuggerStatus::Next(false, true) => matches!(node, ASTNode::Expr(_)), + DebuggerStatus::Next(true, true) => true, + DebuggerStatus::FunctionExit(_) => false, + }; - if !stop && !global.debugger.is_break_point(&global.source, node) { + let event = if stop { + DebuggerEvent::Step + } else { + if let Some(bp) = global.debugger.is_break_point(&global.source, node) { + DebuggerEvent::BreakPoint(bp) + } else { return Ok(None); } + }; - let source = global.source.clone(); - let source = if source.is_empty() { - None - } else { - Some(source.as_str()) - }; + self.run_debugger_raw(scope, global, state, lib, this_ptr, node, event, level) + } + /// Run the debugger callback unconditionally. + /// + /// Returns `Some` if the debugger needs to be reactivated at the end of the block, statement or + /// function call. + /// + /// It is up to the [`Engine`] to reactivate the debugger. + #[inline] + #[must_use] + pub(crate) fn run_debugger_raw<'a>( + &self, + scope: &mut Scope, + global: &mut GlobalRuntimeState, + state: &mut EvalState, + lib: &[&Module], + this_ptr: &mut Option<&mut Dynamic>, + node: ASTNode<'a>, + event: DebuggerEvent, + level: usize, + ) -> Result, Box> { + let source = global.source.clone(); + let source = if source.is_empty() { + None + } else { + Some(source.as_str()) + }; - let mut context = crate::EvalContext { - engine: self, - scope, - global, - state, - lib, - this_ptr, - level, - }; + let mut context = crate::EvalContext { + engine: self, + scope, + global, + state, + lib, + this_ptr, + level, + }; - let command = on_debugger(&mut context, node, source, node.position())?; + if let Some((_, ref on_debugger)) = self.debugger { + let command = on_debugger(&mut context, event, node, source, node.position())?; match command { DebuggerCommand::Continue => { - global.debugger.status = DebuggerCommand::Continue; + global.debugger.status = DebuggerStatus::Next(false, false); Ok(None) } DebuggerCommand::Next => { - global.debugger.status = DebuggerCommand::Continue; - Ok(Some(DebuggerCommand::Next)) - } - DebuggerCommand::StepInto => { - global.debugger.status = DebuggerCommand::StepInto; - Ok(None) + global.debugger.status = DebuggerStatus::Next(false, false); + Ok(Some(DebuggerStatus::Next(true, false))) } DebuggerCommand::StepOver => { - global.debugger.status = DebuggerCommand::Continue; - Ok(Some(DebuggerCommand::StepOver)) + global.debugger.status = DebuggerStatus::Next(false, false); + Ok(Some(DebuggerStatus::Next(true, true))) + } + DebuggerCommand::StepInto => { + global.debugger.status = DebuggerStatus::Next(true, true); + Ok(None) + } + DebuggerCommand::FunctionExit => { + global.debugger.status = DebuggerStatus::FunctionExit(context.call_level()); + Ok(None) } } } else { diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 6b8a1287..0b848b9c 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -266,8 +266,11 @@ impl Engine { // binary operators are also function calls. if let Expr::FnCall(x, pos) = expr { #[cfg(feature = "debugging")] - let reset_debugger = - self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)?; + let reset_debugger = if self.debugger.is_some() { + self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)? + } else { + None + }; #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, expr.position())?; @@ -286,7 +289,9 @@ impl Engine { // will cost more than the mis-predicted `match` branch. if let Expr::Variable(index, var_pos, x) = expr { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, expr, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, expr, level)?; + } #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, expr.position())?; @@ -303,8 +308,11 @@ impl Engine { } #[cfg(feature = "debugging")] - let reset_debugger = - self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)?; + let reset_debugger = if self.debugger.is_some() { + self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)? + } else { + None + }; #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, expr.position())?; diff --git a/src/eval/mod.rs b/src/eval/mod.rs index 0609c9b8..7550c3ab 100644 --- a/src/eval/mod.rs +++ b/src/eval/mod.rs @@ -14,7 +14,10 @@ pub use chaining::{ChainArgument, ChainType}; #[cfg(not(feature = "no_function"))] pub use debugger::CallStackFrame; #[cfg(feature = "debugging")] -pub use debugger::{BreakPoint, Debugger, DebuggerCommand, OnDebuggerCallback, OnDebuggingInit}; +pub use debugger::{ + BreakPoint, Debugger, DebuggerCommand, DebuggerEvent, DebuggerStatus, OnDebuggerCallback, + OnDebuggingInit, +}; pub use eval_context::EvalContext; pub use eval_state::EvalState; #[cfg(not(feature = "no_module"))] diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index a5dd2da4..b853d3dc 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -202,8 +202,11 @@ impl Engine { level: usize, ) -> RhaiResult { #[cfg(feature = "debugging")] - let reset_debugger = - self.run_debugger_with_reset(scope, global, state, lib, this_ptr, stmt, level)?; + let reset_debugger = if self.debugger.is_some() { + self.run_debugger_with_reset(scope, global, state, lib, this_ptr, stmt, level)? + } else { + None + }; // Coded this way for better branch prediction. // Popular branches are lifted out of the `match` statement into their own branches. diff --git a/src/func/call.rs b/src/func/call.rs index 95ea4758..612982e4 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -905,11 +905,15 @@ impl Engine { Ok(( if let Expr::Stack(slot, _) = arg_expr { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; + } constants[*slot].clone() } else if let Some(value) = arg_expr.get_literal_value() { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; + } value } else { self.eval_expr(scope, global, state, lib, this_ptr, arg_expr, level)? @@ -1155,7 +1159,9 @@ impl Engine { let first_expr = first_arg.unwrap(); #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, first_expr, level)?; + if self.debugger.is_some() { + self.run_debugger(scope, global, state, lib, this_ptr, first_expr, level)?; + } // func(x, ...) -> x.func(...) a_expr.iter().try_for_each(|expr| { @@ -1239,7 +1245,10 @@ impl Engine { // &mut first argument and avoid cloning the value if !args_expr.is_empty() && args_expr[0].is_variable_access(true) { #[cfg(feature = "debugging")] - self.run_debugger(scope, global, state, lib, this_ptr, &args_expr[0], level)?; + if self.debugger.is_some() { + let node = &args_expr[0]; + self.run_debugger(scope, global, state, lib, this_ptr, node, level)?; + } // func(x, ...) -> x.func(...) arg_values.push(Dynamic::UNIT); diff --git a/src/func/script.rs b/src/func/script.rs index e42a28fd..650f9db2 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -65,16 +65,16 @@ impl Engine { #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, pos)?; - if fn_def.body.is_empty() { - return Ok(Dynamic::UNIT); - } - // Check for stack overflow #[cfg(not(feature = "unchecked"))] if level > self.max_call_levels() { return Err(ERR::ErrorStackOverflow(pos).into()); } + if fn_def.body.is_empty() { + return Ok(Dynamic::UNIT); + } + let orig_scope_len = scope.len(); #[cfg(not(feature = "no_module"))] let orig_imports_len = global.num_imports(); @@ -133,7 +133,7 @@ impl Engine { }; // Evaluate the function - let result = self + let mut result = self .eval_stmt_block( scope, global, @@ -166,6 +166,31 @@ impl Engine { _ => make_error(fn_def.name.to_string(), fn_def, global, err, pos), }); + #[cfg(feature = "debugging")] + { + if self.debugger.is_some() { + match global.debugger.status() { + crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { + let node = crate::ast::Stmt::Noop(pos); + let node = (&node).into(); + let event = match result { + Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), + Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), + }; + if let Err(err) = self.run_debugger_raw( + scope, global, state, lib, this_ptr, node, event, level, + ) { + result = Err(err); + } + } + _ => (), + } + } + + // Pop the call stack + global.debugger.rewind_call_stack(orig_call_stack_len); + } + // Remove all local variables and imported modules if rewind_scope { scope.rewind(orig_scope_len); @@ -185,10 +210,6 @@ impl Engine { // Restore state state.rewind_fn_resolution_caches(orig_fn_resolution_caches_len); - // Pop the call stack - #[cfg(feature = "debugging")] - global.debugger.rewind_call_stack(orig_call_stack_len); - result } diff --git a/src/lib.rs b/src/lib.rs index 6d19cde0..a94f146e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -164,7 +164,7 @@ pub use types::{ pub mod debugger { #[cfg(not(feature = "no_function"))] pub use super::eval::CallStackFrame; - pub use super::eval::{BreakPoint, Debugger, DebuggerCommand}; + pub use super::eval::{BreakPoint, Debugger, DebuggerCommand, DebuggerEvent}; } /// An identifier in Rhai. [`SmartString`](https://crates.io/crates/smartstring) is used because most diff --git a/tests/debugging.rs b/tests/debugging.rs index d0ed9cfe..036bd4fb 100644 --- a/tests/debugging.rs +++ b/tests/debugging.rs @@ -50,7 +50,7 @@ fn test_debugger_state() -> Result<(), Box> { state.insert("foo".into(), false.into()); Dynamic::from_map(state) }, - |context, _, _, _| { + |context, _, _, _, _| { // Get global runtime state let global = context.global_runtime_state_mut(); From 4a80483749149b8b873298d82c2f4c4d802f10b9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 2 Feb 2022 14:47:35 +0800 Subject: [PATCH 07/22] Support call stack and FunctionExit for native functions. --- CHANGELOG.md | 3 +- src/bin/rhai-dbg.rs | 24 +++++++- src/eval/chaining.rs | 3 + src/eval/debugger.rs | 19 ++++-- src/eval/expr.rs | 1 + src/eval/stmt.rs | 6 +- src/func/call.rs | 119 +++++++++++++++++++++++++++----------- src/func/native.rs | 17 +++++- src/func/script.rs | 56 +++++++++--------- src/optimizer.rs | 1 + src/packages/debugging.rs | 10 +++- src/tests.rs | 4 +- tests/debugging.rs | 4 +- 13 files changed, 190 insertions(+), 77 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0afcd1a1..0de82a1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ New features * A debugging interface is added. * A new bin tool, `rhai-dbg` (aka _The Rhai Debugger_), is added to showcase the debugging interface. -* A new package, `DebuggingPackage`, is added which contains the `stack_trace` function to get the current call stack anywhere in a script. +* A new package, `DebuggingPackage`, is added which contains the `back_trace` function to get the current call stack anywhere in a script. Enhancements ------------ @@ -32,6 +32,7 @@ Enhancements * Default features for dependencies (such as `ahash/std` and `num-traits/std`) are no longer required. * The `no_module` feature now eliminates large sections of code via feature gates. * Debug display of `AST` is improved. +* `NativeCallContext::call_level()` is added to give the current nesting level of function calls. REPL tool changes ----------------- diff --git a/src/bin/rhai-dbg.rs b/src/bin/rhai-dbg.rs index c90881cf..5158eebb 100644 --- a/src/bin/rhai-dbg.rs +++ b/src/bin/rhai-dbg.rs @@ -270,10 +270,30 @@ fn main() { } } DebuggerEvent::FunctionExitWithValue(r) => { - println!("! Return from function call = {}", r) + println!( + "! Return from function call '{}' => {}", + context + .global_runtime_state() + .debugger + .call_stack() + .last() + .unwrap() + .fn_name, + r + ) } DebuggerEvent::FunctionExitWithError(err) => { - println!("! Return from function call with error: {}", err) + println!( + "! Return from function call '{}' with error: {}", + context + .global_runtime_state() + .debugger + .call_stack() + .last() + .unwrap() + .fn_name, + err + ) } } diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 43a4e4a8..16daa9e5 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -220,6 +220,7 @@ impl Engine { Ok(ref mut obj_ptr) => { self.eval_op_assignment( global, state, lib, op_info, op_pos, obj_ptr, root, new_val, + level, ) .map_err(|err| err.fill_position(new_pos))?; #[cfg(not(feature = "unchecked"))] @@ -314,6 +315,7 @@ impl Engine { )?; self.eval_op_assignment( global, state, lib, op_info, op_pos, val_target, root, new_val, + level, ) .map_err(|err| err.fill_position(new_pos))?; } @@ -380,6 +382,7 @@ impl Engine { &mut (&mut orig_val).into(), root, new_val, + level, ) .map_err(|err| err.fill_position(new_pos))?; diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index 28c86e4d..d2d2234d 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -346,13 +346,15 @@ impl Debugger { node.position() == *pos && _src == source } BreakPoint::AtFunctionName { name, .. } => match node { - ASTNode::Expr(Expr::FnCall(x, _)) | ASTNode::Stmt(Stmt::FnCall(x, _)) => { - x.name == *name - } + ASTNode::Expr(Expr::FnCall(x, _)) + | ASTNode::Stmt(Stmt::FnCall(x, _)) + | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(x, _))) => x.name == *name, _ => false, }, BreakPoint::AtFunctionCall { name, args, .. } => match node { - ASTNode::Expr(Expr::FnCall(x, _)) | ASTNode::Stmt(Stmt::FnCall(x, _)) => { + ASTNode::Expr(Expr::FnCall(x, _)) + | ASTNode::Stmt(Stmt::FnCall(x, _)) + | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(x, _))) => { x.args.len() == *args && x.name == *name } _ => false, @@ -503,7 +505,14 @@ impl Engine { Ok(None) } DebuggerCommand::FunctionExit => { - global.debugger.status = DebuggerStatus::FunctionExit(context.call_level()); + // Bump a level if it is a function call + let level = match node { + ASTNode::Expr(Expr::FnCall(_, _)) + | ASTNode::Stmt(Stmt::FnCall(_, _)) + | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(_, _))) => context.call_level() + 1, + _ => context.call_level(), + }; + global.debugger.status = DebuggerStatus::FunctionExit(level); Ok(None) } } diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 0b848b9c..3b6015b7 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -353,6 +353,7 @@ impl Engine { &mut (&mut concat).into(), ("", Position::NONE), item, + level, ) { result = Err(err.fill_position(expr.position())); break; diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index b853d3dc..2adb7829 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -120,6 +120,7 @@ impl Engine { target: &mut Target, root: (&str, Position), new_val: Dynamic, + level: usize, ) -> RhaiResultOf<()> { if target.is_read_only() { // Assignment to constant variable @@ -154,7 +155,7 @@ impl Engine { let args = &mut [lhs_ptr_inner, &mut new_val]; match self.call_native_fn( - global, state, lib, op_assign, hash, args, true, true, op_pos, + global, state, lib, op_assign, hash, args, true, true, op_pos, level, ) { Ok(_) => { #[cfg(not(feature = "unchecked"))] @@ -164,7 +165,7 @@ impl Engine { { // Expand to `var = var op rhs` let (value, _) = self.call_native_fn( - global, state, lib, op, hash_op, args, true, false, op_pos, + global, state, lib, op, hash_op, args, true, false, op_pos, level, )?; #[cfg(not(feature = "unchecked"))] @@ -266,6 +267,7 @@ impl Engine { &mut lhs_ptr, (var_name, pos), rhs_val, + level, ) .map_err(|err| err.fill_position(rhs.position())) .map(|_| Dynamic::UNIT) diff --git a/src/func/call.rs b/src/func/call.rs index 612982e4..a73bd7bb 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -328,6 +328,8 @@ impl Engine { result.as_ref().map(Box::as_ref) } + /// # Main Entry-Point + /// /// Call a native Rust function registered with the [`Engine`]. /// /// # WARNING @@ -347,6 +349,7 @@ impl Engine { is_ref_mut: bool, is_op_assign: bool, pos: Position, + level: usize, ) -> RhaiResultOf<(Dynamic, bool)> { #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, pos)?; @@ -365,39 +368,85 @@ impl Engine { is_op_assign, ); - if let Some(FnResolutionCacheEntry { func, source }) = func { - assert!(func.is_native()); + if func.is_some() { + let is_method = func.map(|f| f.func.is_method()).unwrap_or(false); - // Calling pure function but the first argument is a reference? - let mut backup: Option = None; - if is_ref_mut && func.is_pure() && !args.is_empty() { - // Clone the first argument - backup = Some(ArgBackup::new()); - backup - .as_mut() - .expect("`Some`") - .change_first_arg_to_copy(args); - } + // Push a new call stack frame + #[cfg(feature = "debugging")] + let orig_call_stack_len = global.debugger.call_stack().len(); - // Run external function - let source = match (source.as_str(), parent_source.as_str()) { - ("", "") => None, - ("", s) | (s, _) => Some(s), - }; + let mut result = if let Some(FnResolutionCacheEntry { func, source }) = func { + assert!(func.is_native()); - let context = (self, name, source, &*global, lib, pos).into(); + // Calling pure function but the first argument is a reference? + let mut backup: Option = None; + if is_ref_mut && func.is_pure() && !args.is_empty() { + // Clone the first argument + backup = Some(ArgBackup::new()); + backup + .as_mut() + .expect("`Some`") + .change_first_arg_to_copy(args); + } - let result = if func.is_plugin_fn() { - func.get_plugin_fn() - .expect("plugin function") - .call(context, args) + let source = match (source.as_str(), parent_source.as_str()) { + ("", "") => None, + ("", s) | (s, _) => Some(s), + }; + + #[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(""), + pos, + ); + } + + // Run external function + let context = (self, name, source, &*global, lib, pos, level).into(); + + let result = if func.is_plugin_fn() { + func.get_plugin_fn() + .expect("plugin function") + .call(context, args) + } else { + func.get_native_fn().expect("native function")(context, args) + }; + + // Restore the original reference + if let Some(bk) = backup { + bk.restore_first_arg(args) + } + + result } else { - func.get_native_fn().expect("native function")(context, args) + unreachable!("`Some`"); }; - // Restore the original reference - if let Some(bk) = backup { - bk.restore_first_arg(args) + #[cfg(feature = "debugging")] + if self.debugger.is_some() { + match global.debugger.status() { + crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { + let scope = &mut &mut Scope::new(); + let node = crate::ast::Stmt::Noop(pos); + let node = (&node).into(); + let event = match result { + Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), + Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), + }; + if let Err(err) = self.run_debugger_raw( + scope, global, state, lib, &mut None, node, event, level, + ) { + result = Err(err); + } + } + _ => (), + } + + // Pop the call stack + global.debugger.rewind_call_stack(orig_call_stack_len); } // Check the return value (including data sizes) @@ -443,7 +492,7 @@ impl Engine { (Dynamic::UNIT, false) } } - _ => (result, func.is_method()), + _ => (result, is_method), }); } @@ -530,6 +579,8 @@ impl Engine { } } + /// # Main Entry-Point + /// /// Perform an actual function call, native Rust or scripted, taking care of special functions. /// /// # WARNING @@ -562,7 +613,6 @@ impl Engine { ensure_no_data_race(fn_name, args, is_ref_mut)?; let _scope = scope; - let _level = level; let _is_method_call = is_method_call; // These may be redirected from method style calls. @@ -612,6 +662,8 @@ impl Engine { _ => (), } + let level = level + 1; + // Script-defined function call? #[cfg(not(feature = "no_function"))] if let Some(FnResolutionCacheEntry { func, mut source }) = self @@ -646,7 +698,6 @@ impl Engine { }; mem::swap(&mut global.source, &mut source); - let level = _level + 1; let result = if _is_method_call { // Method call of script function - map first argument to `this` @@ -699,7 +750,7 @@ impl Engine { // Native function call let hash = hashes.native; self.call_native_fn( - global, state, lib, fn_name, hash, args, is_ref_mut, false, pos, + global, state, lib, fn_name, hash, args, is_ref_mut, false, pos, level, ) } @@ -1318,6 +1369,8 @@ impl Engine { } } + let level = level + 1; + match func { #[cfg(not(feature = "no_function"))] Some(f) if f.is_script() => { @@ -1331,8 +1384,6 @@ impl Engine { let mut source = module.id_raw().clone(); mem::swap(&mut global.source, &mut source); - let level = level + 1; - let result = self.call_script_fn( new_scope, global, state, lib, &mut None, fn_def, &mut args, pos, true, level, @@ -1345,7 +1396,7 @@ impl Engine { } Some(f) if f.is_plugin_fn() => { - let context = (self, fn_name, module.id(), &*global, lib, pos).into(); + let context = (self, fn_name, module.id(), &*global, lib, pos, level).into(); let result = f .get_plugin_fn() .expect("plugin function") @@ -1356,7 +1407,7 @@ impl Engine { Some(f) if f.is_native() => { let func = f.get_native_fn().expect("native function"); - let context = (self, fn_name, module.id(), &*global, lib, pos).into(); + let context = (self, fn_name, module.id(), &*global, lib, pos, level).into(); let result = func(context, &mut args); self.check_return_value(result, pos) } diff --git a/src/func/native.rs b/src/func/native.rs index 7938f4cd..c5787386 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -70,6 +70,8 @@ pub struct NativeCallContext<'a> { lib: &'a [&'a Module], /// [Position] of the function call. pos: Position, + /// The current nesting level of function calls. + level: usize, } impl<'a, M: AsRef<[&'a Module]> + ?Sized, S: AsRef + 'a + ?Sized> @@ -80,6 +82,7 @@ impl<'a, M: AsRef<[&'a Module]> + ?Sized, S: AsRef + 'a + ?Sized> &'a GlobalRuntimeState<'a>, &'a M, Position, + usize, )> for NativeCallContext<'a> { #[inline(always)] @@ -91,6 +94,7 @@ impl<'a, M: AsRef<[&'a Module]> + ?Sized, S: AsRef + 'a + ?Sized> &'a GlobalRuntimeState, &'a M, Position, + usize, ), ) -> Self { Self { @@ -100,6 +104,7 @@ impl<'a, M: AsRef<[&'a Module]> + ?Sized, S: AsRef + 'a + ?Sized> global: Some(value.3), lib: value.4.as_ref(), pos: value.5, + level: value.6, } } } @@ -116,6 +121,7 @@ impl<'a, M: AsRef<[&'a Module]> + ?Sized, S: AsRef + 'a + ?Sized> global: None, lib: value.2.as_ref(), pos: Position::NONE, + level: 0, } } } @@ -141,6 +147,7 @@ impl<'a> NativeCallContext<'a> { global: None, lib, pos: Position::NONE, + level: 0, } } /// _(internals)_ Create a new [`NativeCallContext`]. @@ -158,6 +165,7 @@ impl<'a> NativeCallContext<'a> { global: &'a GlobalRuntimeState, lib: &'a [&Module], pos: Position, + level: usize, ) -> Self { Self { engine, @@ -166,6 +174,7 @@ impl<'a> NativeCallContext<'a> { global: Some(global), lib, pos, + level, } } /// The current [`Engine`]. @@ -186,6 +195,12 @@ impl<'a> NativeCallContext<'a> { pub const fn position(&self) -> Position { self.pos } + /// Current nesting level of function calls. + #[inline(always)] + #[must_use] + pub const fn call_level(&self) -> usize { + self.level + } /// The current source. #[inline(always)] #[must_use] @@ -316,7 +331,7 @@ impl<'a> NativeCallContext<'a> { is_method_call, Position::NONE, None, - 0, + self.level + 1, ) .map(|(r, _)| r) } diff --git a/src/func/script.rs b/src/func/script.rs index 650f9db2..5178ed2b 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -10,6 +10,8 @@ use std::mem; use std::prelude::v1::*; impl Engine { + /// # Main Entry-Point + /// /// Call a script-defined function. /// /// If `rewind_scope` is `false`, arguments are removed from the scope but new variables are not. @@ -90,16 +92,18 @@ impl Engine { // Push a new call stack frame #[cfg(feature = "debugging")] - global.debugger.push_call_stack_frame( - fn_def.name.clone(), - scope - .iter() - .skip(orig_scope_len) - .map(|(_, _, v)| v.clone()) - .collect(), - global.source.clone(), - pos, - ); + if self.debugger.is_some() { + global.debugger.push_call_stack_frame( + fn_def.name.clone(), + scope + .iter() + .skip(orig_scope_len) + .map(|(_, _, v)| v.clone()) + .collect(), + global.source.clone(), + pos, + ); + } // Merge in encapsulated environment, if any let orig_fn_resolution_caches_len = state.fn_resolution_caches_len(); @@ -167,26 +171,24 @@ impl Engine { }); #[cfg(feature = "debugging")] - { - if self.debugger.is_some() { - match global.debugger.status() { - crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { - let node = crate::ast::Stmt::Noop(pos); - let node = (&node).into(); - let event = match result { - Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), - Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), - }; - if let Err(err) = self.run_debugger_raw( - scope, global, state, lib, this_ptr, node, event, level, - ) { - result = Err(err); - } + if self.debugger.is_some() { + match global.debugger.status() { + crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { + let node = crate::ast::Stmt::Noop(pos); + let node = (&node).into(); + let event = match result { + Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), + Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), + }; + if let Err(err) = self + .run_debugger_raw(scope, global, state, lib, this_ptr, node, event, level) + { + result = Err(err); } - _ => (), } + _ => (), } - + // Pop the call stack global.debugger.rewind_call_stack(orig_call_stack_len); } diff --git a/src/optimizer.rs b/src/optimizer.rs index d430ac76..acae357e 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -147,6 +147,7 @@ impl<'a> OptimizerState<'a> { false, false, Position::NONE, + 0, ) .ok() .map(|(v, _)| v) diff --git a/src/packages/debugging.rs b/src/packages/debugging.rs index 397f8b8e..ab24bc3a 100644 --- a/src/packages/debugging.rs +++ b/src/packages/debugging.rs @@ -27,15 +27,23 @@ def_package! { #[export_module] mod debugging_functions { + /// Get an array of object maps containing the function calls stack. + /// + /// If there is no debugging interface registered, an empty array is returned. + /// + /// An array of strings is returned under `no_object`. #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_index"))] - pub fn stack_trace(ctx: NativeCallContext) -> Array { + pub fn back_trace(ctx: NativeCallContext) -> Array { if let Some(global) = ctx.global_runtime_state() { global .debugger .call_stack() .iter() .rev() + .filter(|crate::debugger::CallStackFrame { fn_name, args, .. }| { + fn_name != "back_trace" || !args.is_empty() + }) .map( |frame @ crate::debugger::CallStackFrame { fn_name: _fn_name, diff --git a/src/tests.rs b/src/tests.rs index f4d32675..e96d8dd5 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -37,9 +37,9 @@ fn check_struct_sizes() { assert_eq!( size_of::(), if cfg!(feature = "no_position") { - 64 - } else { 72 + } else { + 80 } ); } diff --git a/tests/debugging.rs b/tests/debugging.rs index 036bd4fb..b40c1016 100644 --- a/tests/debugging.rs +++ b/tests/debugging.rs @@ -18,7 +18,7 @@ fn test_debugging() -> Result<(), Box> { " fn foo(x) { if x >= 5 { - stack_trace() + back_trace() } else { foo(x+1) } @@ -30,7 +30,7 @@ fn test_debugging() -> Result<(), Box> { assert_eq!(r.len(), 6); - assert_eq!(engine.eval::("len(stack_trace())")?, 0); + assert_eq!(engine.eval::("len(back_trace())")?, 0); } Ok(()) From 339136901d70e46db7a0098d01bf055f33479f7d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 2 Feb 2022 14:47:46 +0800 Subject: [PATCH 08/22] Benchmark with features turned on. --- .github/workflows/benchmark.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index df310705..98309937 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -12,7 +12,7 @@ jobs: - uses: actions/checkout@v2 - run: rustup toolchain update nightly && rustup default nightly - name: Run benchmark - run: cargo +nightly bench | tee output.txt + run: cargo +nightly bench --features decimal,metadata,serde,debugging | tee output.txt - name: Store benchmark result uses: rhysd/github-action-benchmark@v1 with: From e0ed713bb6aa0e8bbb387f1f2e708b7203314ab7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 2 Feb 2022 14:57:30 +0800 Subject: [PATCH 09/22] Fix builds. --- src/func/call.rs | 8 ++++---- src/func/script.rs | 10 +++++----- tests/debugging.rs | 7 ++++++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/func/call.rs b/src/func/call.rs index a73bd7bb..cd0d9614 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -375,7 +375,7 @@ impl Engine { #[cfg(feature = "debugging")] let orig_call_stack_len = global.debugger.call_stack().len(); - let mut result = if let Some(FnResolutionCacheEntry { func, source }) = func { + let mut _result = if let Some(FnResolutionCacheEntry { func, source }) = func { assert!(func.is_native()); // Calling pure function but the first argument is a reference? @@ -432,14 +432,14 @@ impl Engine { let scope = &mut &mut Scope::new(); let node = crate::ast::Stmt::Noop(pos); let node = (&node).into(); - let event = match result { + let event = match _result { Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), }; if let Err(err) = self.run_debugger_raw( scope, global, state, lib, &mut None, node, event, level, ) { - result = Err(err); + _result = Err(err); } } _ => (), @@ -450,7 +450,7 @@ impl Engine { } // Check the return value (including data sizes) - let result = self.check_return_value(result, pos)?; + let result = self.check_return_value(_result, pos)?; // Check the data size of any `&mut` object, which may be changed. #[cfg(not(feature = "unchecked"))] diff --git a/src/func/script.rs b/src/func/script.rs index 5178ed2b..9c10e76e 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -137,7 +137,7 @@ impl Engine { }; // Evaluate the function - let mut result = self + let mut _result = self .eval_stmt_block( scope, global, @@ -176,19 +176,19 @@ impl Engine { crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { let node = crate::ast::Stmt::Noop(pos); let node = (&node).into(); - let event = match result { + let event = match _result { Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), }; if let Err(err) = self .run_debugger_raw(scope, global, state, lib, this_ptr, node, event, level) { - result = Err(err); + _result = Err(err); } } _ => (), } - + // Pop the call stack global.debugger.rewind_call_stack(orig_call_stack_len); } @@ -212,7 +212,7 @@ impl Engine { // Restore state state.rewind_fn_resolution_caches(orig_fn_resolution_caches_len); - result + _result } // Does a script-defined function exist? diff --git a/tests/debugging.rs b/tests/debugging.rs index b40c1016..aa797624 100644 --- a/tests/debugging.rs +++ b/tests/debugging.rs @@ -9,7 +9,12 @@ use rhai::Map; #[test] fn test_debugging() -> Result<(), Box> { - let engine = Engine::new(); + let mut engine = Engine::new(); + + engine.register_debugger( + || Dynamic::UNIT, + |_, _, _, _, _| Ok(rhai::debugger::DebuggerCommand::Continue), + ); #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_index"))] From db2f1a601c213ba9fc775b126194ea041c3f174e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 2 Feb 2022 15:07:22 +0800 Subject: [PATCH 10/22] Make call stack available also under no_function. --- src/eval/debugger.rs | 15 +-------------- src/eval/mod.rs | 7 ++----- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index d2d2234d..d8a29d9e 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -56,7 +56,7 @@ pub enum DebuggerCommand { pub enum DebuggerStatus { // Stop at the next statement or expression. Next(bool, bool), - // Run to the end of the current function call. + // Run to the end of the current level of function call. FunctionExit(usize), } @@ -193,7 +193,6 @@ impl BreakPoint { } /// A function call. -#[cfg(not(feature = "no_function"))] #[derive(Debug, Clone, Hash)] pub struct CallStackFrame { /// Function name. @@ -206,7 +205,6 @@ pub struct CallStackFrame { pub pos: Position, } -#[cfg(not(feature = "no_function"))] impl fmt::Display for CallStackFrame { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut fp = f.debug_tuple(&self.fn_name); @@ -239,7 +237,6 @@ pub struct Debugger { /// The current set of break-points. break_points: Vec, /// The current function call stack. - #[cfg(not(feature = "no_function"))] call_stack: Vec, } @@ -260,7 +257,6 @@ impl Debugger { Dynamic::UNIT }, break_points: Vec::new(), - #[cfg(not(feature = "no_function"))] call_stack: Vec::new(), } } @@ -277,26 +273,17 @@ impl Debugger { &mut self.state } /// Get the current call stack. - /// - /// Not available under `no_function`. - #[cfg(not(feature = "no_function"))] #[inline(always)] #[must_use] pub fn call_stack(&self) -> &[CallStackFrame] { &self.call_stack } /// Rewind the function call stack to a particular depth. - /// - /// Not available under `no_function`. - #[cfg(not(feature = "no_function"))] #[inline(always)] pub(crate) fn rewind_call_stack(&mut self, len: usize) { self.call_stack.truncate(len); } /// Add a new frame to the function call stack. - /// - /// Not available under `no_function`. - #[cfg(not(feature = "no_function"))] #[inline(always)] pub(crate) fn push_call_stack_frame( &mut self, diff --git a/src/eval/mod.rs b/src/eval/mod.rs index 7550c3ab..0b87a2dd 100644 --- a/src/eval/mod.rs +++ b/src/eval/mod.rs @@ -11,12 +11,9 @@ mod target; #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] pub use chaining::{ChainArgument, ChainType}; #[cfg(feature = "debugging")] -#[cfg(not(feature = "no_function"))] -pub use debugger::CallStackFrame; -#[cfg(feature = "debugging")] pub use debugger::{ - BreakPoint, Debugger, DebuggerCommand, DebuggerEvent, DebuggerStatus, OnDebuggerCallback, - OnDebuggingInit, + BreakPoint, CallStackFrame, Debugger, DebuggerCommand, DebuggerEvent, DebuggerStatus, + OnDebuggerCallback, OnDebuggingInit, }; pub use eval_context::EvalContext; pub use eval_state::EvalState; From 8322e62c181783184e4ef12c945a4d7ba486fd33 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 2 Feb 2022 22:42:33 +0800 Subject: [PATCH 11/22] Fix function exit trigger and add function enter trigger. --- CHANGELOG.md | 2 +- src/api/call_fn.rs | 2 +- src/eval/chaining.rs | 38 +++++++++++------------ src/eval/debugger.rs | 45 ++++++++++++++++++--------- src/eval/expr.rs | 4 +-- src/eval/stmt.rs | 1 + src/func/call.rs | 73 ++++++++++++++++++++++++++++---------------- src/func/native.rs | 2 +- src/func/script.rs | 16 ++++++++-- 9 files changed, 115 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0de82a1d..42f989ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -576,7 +576,7 @@ Breaking changes New features ------------ -* Line continuation (via `\`) and multi-line literal strings (wrapped with \`) support are added. +* Line continuation (via `\`) and multi-line literal strings (wrapped with `` ` ``) support are added. * Rhai scripts can now start with a shebang `#!` which is ignored. Enhancements diff --git a/src/api/call_fn.rs b/src/api/call_fn.rs index e501c68d..ca41552d 100644 --- a/src/api/call_fn.rs +++ b/src/api/call_fn.rs @@ -190,8 +190,8 @@ impl Engine { &mut this_ptr, fn_def, &mut args, - Position::NONE, rewind_scope, + Position::NONE, 0, ); diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 16daa9e5..8cd99b15 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -191,8 +191,8 @@ impl Engine { let fn_name = crate::engine::FN_IDX_SET; if let Err(err) = self.exec_fn_call( - global, state, lib, fn_name, hash_set, args, is_ref_mut, true, - root_pos, None, level, + None, global, state, lib, fn_name, hash_set, args, is_ref_mut, + true, root_pos, level, ) { // Just ignore if there is no index setter if !matches!(*err, ERR::ErrorFunctionNotFound(_, _)) { @@ -244,8 +244,8 @@ impl Engine { let fn_name = crate::engine::FN_IDX_SET; self.exec_fn_call( - global, state, lib, fn_name, hash_set, args, is_ref_mut, true, - root_pos, None, level, + None, global, state, lib, fn_name, hash_set, args, is_ref_mut, + true, root_pos, level, )?; } @@ -351,8 +351,8 @@ impl Engine { let args = &mut [target.as_mut()]; let (mut orig_val, _) = self .exec_fn_call( - global, state, lib, getter, hash, args, is_ref_mut, true, *pos, - None, level, + None, global, state, lib, getter, hash, args, is_ref_mut, true, + *pos, level, ) .or_else(|err| match *err { // Try an indexer if property does not exist @@ -392,7 +392,7 @@ impl Engine { let hash = crate::ast::FnCallHashes::from_native(*hash_set); let args = &mut [target.as_mut(), &mut new_val]; self.exec_fn_call( - global, state, lib, setter, hash, args, is_ref_mut, true, *pos, None, + None, global, state, lib, setter, hash, args, is_ref_mut, true, *pos, level, ) .or_else(|err| match *err { @@ -405,8 +405,8 @@ impl Engine { let pos = Position::NONE; self.exec_fn_call( - global, state, lib, fn_name, hash_set, args, is_ref_mut, true, - pos, None, level, + None, global, state, lib, fn_name, hash_set, args, is_ref_mut, + true, pos, level, ) .map_err( |idx_err| match *idx_err { @@ -429,7 +429,7 @@ impl Engine { let hash = crate::ast::FnCallHashes::from_native(*hash_get); let args = &mut [target.as_mut()]; self.exec_fn_call( - global, state, lib, getter, hash, args, is_ref_mut, true, *pos, None, + None, global, state, lib, getter, hash, args, is_ref_mut, true, *pos, level, ) .map_or_else( @@ -537,8 +537,8 @@ impl Engine { // Assume getters are always pure let (mut val, _) = self .exec_fn_call( - global, state, lib, getter, hash_get, args, is_ref_mut, - true, pos, None, level, + None, global, state, lib, getter, hash_get, args, + is_ref_mut, true, pos, level, ) .or_else(|err| match *err { // Try an indexer if property does not exist @@ -585,8 +585,8 @@ impl Engine { let mut arg_values = [target.as_mut(), val]; let args = &mut arg_values; self.exec_fn_call( - global, state, lib, setter, hash_set, args, is_ref_mut, - true, pos, None, level, + None, global, state, lib, setter, hash_set, args, + is_ref_mut, true, pos, level, ) .or_else( |err| match *err { @@ -600,8 +600,8 @@ impl Engine { global.hash_idx_set(), ); self.exec_fn_call( - global, state, lib, fn_name, hash_set, args, - is_ref_mut, true, pos, None, level, + None, global, state, lib, fn_name, hash_set, + args, is_ref_mut, true, pos, level, ) .or_else(|idx_err| match *idx_err { ERR::ErrorIndexingType(_, _) => { @@ -767,7 +767,7 @@ impl Engine { (crate::FnArgsVec::with_capacity(args.len()), Position::NONE), |(mut values, mut pos), expr| -> RhaiResultOf<_> { let (value, arg_pos) = self.get_arg_value( - scope, global, state, lib, this_ptr, level, expr, constants, + scope, global, state, lib, this_ptr, expr, constants, level, )?; if values.is_empty() { pos = arg_pos; @@ -813,7 +813,7 @@ impl Engine { (crate::FnArgsVec::with_capacity(args.len()), Position::NONE), |(mut values, mut pos), expr| -> RhaiResultOf<_> { let (value, arg_pos) = self.get_arg_value( - scope, global, state, lib, this_ptr, level, expr, constants, + scope, global, state, lib, this_ptr, expr, constants, level, )?; if values.is_empty() { pos = arg_pos @@ -1079,7 +1079,7 @@ impl Engine { let pos = Position::NONE; self.exec_fn_call( - global, state, lib, fn_name, hash_get, args, true, true, pos, None, level, + None, global, state, lib, fn_name, hash_get, args, true, true, pos, level, ) .map(|(v, _)| v.into()) } diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index d8a29d9e..f6889c17 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -51,6 +51,13 @@ pub enum DebuggerCommand { FunctionExit, } +impl Default for DebuggerCommand { + #[inline(always)] + fn default() -> Self { + Self::Continue + } +} + /// The debugger status. #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub enum DebuggerStatus { @@ -60,6 +67,19 @@ pub enum DebuggerStatus { FunctionExit(usize), } +impl Default for DebuggerStatus { + #[inline(always)] + fn default() -> Self { + Self::CONTINUE + } +} + +impl DebuggerStatus { + pub const CONTINUE: Self = Self::Next(false, false); + pub const STEP: Self = Self::Next(true, true); + pub const NEXT: Self = Self::Next(true, false); +} + /// A event that triggers the debugger. #[derive(Debug, Clone, Copy)] pub enum DebuggerEvent<'a> { @@ -231,7 +251,7 @@ impl fmt::Display for CallStackFrame { #[derive(Debug, Clone, Hash)] pub struct Debugger { /// The current status command. - status: DebuggerStatus, + pub(crate) status: DebuggerStatus, /// The current state. state: Dynamic, /// The current set of break-points. @@ -247,9 +267,9 @@ impl Debugger { pub fn new(engine: &Engine) -> Self { Self { status: if engine.debugger.is_some() { - DebuggerStatus::Next(true, true) + DebuggerStatus::STEP } else { - DebuggerStatus::Next(false, false) + DebuggerStatus::CONTINUE }, state: if let Some((ref init, _)) = engine.debugger { init() @@ -299,12 +319,6 @@ impl Debugger { pos, }); } - /// Get the current status of this [`Debugger`]. - #[inline(always)] - #[must_use] - pub(crate) fn status(&self) -> DebuggerStatus { - self.status - } /// Set the status of this [`Debugger`]. #[inline(always)] pub(crate) fn reset_status(&mut self, status: Option) { @@ -476,19 +490,19 @@ impl Engine { match command { DebuggerCommand::Continue => { - global.debugger.status = DebuggerStatus::Next(false, false); + global.debugger.status = DebuggerStatus::CONTINUE; Ok(None) } DebuggerCommand::Next => { - global.debugger.status = DebuggerStatus::Next(false, false); - Ok(Some(DebuggerStatus::Next(true, false))) + global.debugger.status = DebuggerStatus::CONTINUE; + Ok(Some(DebuggerStatus::NEXT)) } DebuggerCommand::StepOver => { - global.debugger.status = DebuggerStatus::Next(false, false); - Ok(Some(DebuggerStatus::Next(true, true))) + global.debugger.status = DebuggerStatus::CONTINUE; + Ok(Some(DebuggerStatus::STEP)) } DebuggerCommand::StepInto => { - global.debugger.status = DebuggerStatus::Next(true, true); + global.debugger.status = DebuggerStatus::STEP; Ok(None) } DebuggerCommand::FunctionExit => { @@ -499,6 +513,7 @@ impl Engine { | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(_, _))) => context.call_level() + 1, _ => context.call_level(), }; + println!("Set FunctionExit to {}", level); global.debugger.status = DebuggerStatus::FunctionExit(level); Ok(None) } diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 3b6015b7..79f27d92 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -236,8 +236,8 @@ impl Engine { ); self.make_function_call( - scope, global, state, lib, this_ptr, name, first_arg, args, constants, *hashes, pos, - *capture, level, + scope, global, state, lib, this_ptr, name, first_arg, args, constants, *hashes, + *capture, pos, level, ) } diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 2adb7829..bb4645da 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -153,6 +153,7 @@ impl Engine { let hash = hash_op_assign; let args = &mut [lhs_ptr_inner, &mut new_val]; + let level = level + 1; match self.call_native_fn( global, state, lib, op_assign, hash, args, true, true, op_pos, level, diff --git a/src/func/call.rs b/src/func/call.rs index cd0d9614..240b050c 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -427,7 +427,7 @@ impl Engine { #[cfg(feature = "debugging")] if self.debugger.is_some() { - match global.debugger.status() { + match global.debugger.status { crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { let scope = &mut &mut Scope::new(); let node = crate::ast::Stmt::Noop(pos); @@ -591,6 +591,7 @@ impl Engine { /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! pub(crate) fn exec_fn_call( &self, + scope: Option<&mut Scope>, global: &mut GlobalRuntimeState, state: &mut EvalState, lib: &[&Module], @@ -600,7 +601,6 @@ impl Engine { is_ref_mut: bool, is_method_call: bool, pos: Position, - scope: Option<&mut Scope>, level: usize, ) -> RhaiResultOf<(Dynamic, bool)> { fn no_method_err(name: &str, pos: Position) -> RhaiResultOf<(Dynamic, bool)> { @@ -711,8 +711,8 @@ impl Engine { &mut Some(*first_arg), func, rest_args, - pos, true, + pos, level, ); @@ -730,7 +730,7 @@ impl Engine { } let result = self.call_script_fn( - scope, global, state, lib, &mut None, func, args, pos, true, level, + scope, global, state, lib, &mut None, func, args, true, pos, level, ); // Restore the original reference @@ -812,7 +812,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - global, state, lib, fn_name, new_hash, &mut args, false, false, pos, None, + None, global, state, lib, fn_name, new_hash, &mut args, false, false, pos, level, ) } @@ -852,7 +852,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - global, state, lib, fn_name, new_hash, &mut args, is_ref_mut, true, pos, None, + None, global, state, lib, fn_name, new_hash, &mut args, is_ref_mut, true, pos, level, ) } @@ -924,7 +924,7 @@ impl Engine { args.extend(call_args.iter_mut()); self.exec_fn_call( - global, state, lib, fn_name, hash, &mut args, is_ref_mut, true, pos, None, + None, global, state, lib, fn_name, hash, &mut args, is_ref_mut, true, pos, level, ) } @@ -949,9 +949,9 @@ impl Engine { state: &mut EvalState, lib: &[&Module], this_ptr: &mut Option<&mut Dynamic>, - level: usize, arg_expr: &Expr, constants: &[Dynamic], + level: usize, ) -> RhaiResultOf<(Dynamic, Position)> { Ok(( if let Expr::Stack(slot, _) = arg_expr { @@ -967,7 +967,26 @@ impl Engine { } value } else { - self.eval_expr(scope, global, state, lib, this_ptr, arg_expr, level)? + // Do not match function exit for arguments + #[cfg(feature = "debugging")] + let reset_debugger = match global.debugger.status { + crate::eval::DebuggerStatus::FunctionExit(_) => { + Some(std::mem::take(&mut global.debugger.status)) + } + _ => None, + }; + + let result = self.eval_expr(scope, global, state, lib, this_ptr, arg_expr, level); + + // Restore function exit if status is not active + #[cfg(feature = "debugging")] + if self.debugger.is_some() + && global.debugger.status == crate::eval::DebuggerStatus::CONTINUE + { + global.debugger.reset_status(reset_debugger); + } + + result? }, arg_expr.position(), )) @@ -986,8 +1005,8 @@ impl Engine { args_expr: &[Expr], constants: &[Dynamic], hashes: FnCallHashes, - pos: Position, capture_scope: bool, + pos: Position, level: usize, ) -> RhaiResult { let mut first_arg = first_arg; @@ -1003,7 +1022,7 @@ impl Engine { KEYWORD_FN_PTR_CALL if total_args >= 1 => { let arg = first_arg.unwrap(); let (arg_value, arg_pos) = - self.get_arg_value(scope, global, state, lib, this_ptr, level, arg, constants)?; + self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?; if !arg_value.is::() { return Err(self.make_type_mismatch_err::( @@ -1038,7 +1057,7 @@ impl Engine { KEYWORD_FN_PTR if total_args == 1 => { let arg = first_arg.unwrap(); let (arg_value, arg_pos) = - self.get_arg_value(scope, global, state, lib, this_ptr, level, arg, constants)?; + self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?; // Fn - only in function call style return arg_value @@ -1053,7 +1072,7 @@ impl Engine { KEYWORD_FN_PTR_CURRY if total_args > 1 => { let first = first_arg.unwrap(); let (arg_value, arg_pos) = self - .get_arg_value(scope, global, state, lib, this_ptr, level, first, constants)?; + .get_arg_value(scope, global, state, lib, this_ptr, first, constants, level)?; if !arg_value.is::() { return Err(self.make_type_mismatch_err::( @@ -1070,7 +1089,7 @@ impl Engine { .iter() .try_fold(fn_curry, |mut curried, expr| -> RhaiResultOf<_> { let (value, _) = self.get_arg_value( - scope, global, state, lib, this_ptr, level, expr, constants, + scope, global, state, lib, this_ptr, expr, constants, level, )?; curried.push(value); Ok(curried) @@ -1084,7 +1103,7 @@ impl Engine { crate::engine::KEYWORD_IS_SHARED if total_args == 1 => { let arg = first_arg.unwrap(); let (arg_value, _) = - self.get_arg_value(scope, global, state, lib, this_ptr, level, arg, constants)?; + self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?; return Ok(arg_value.is_shared().into()); } @@ -1093,14 +1112,14 @@ impl Engine { crate::engine::KEYWORD_IS_DEF_FN if total_args == 2 => { let first = first_arg.unwrap(); let (arg_value, arg_pos) = self - .get_arg_value(scope, global, state, lib, this_ptr, level, first, constants)?; + .get_arg_value(scope, global, state, lib, this_ptr, first, constants, level)?; let fn_name = arg_value .into_immutable_string() .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos))?; let (arg_value, arg_pos) = self.get_arg_value( - scope, global, state, lib, this_ptr, level, &a_expr[0], constants, + scope, global, state, lib, this_ptr, &a_expr[0], constants, level, )?; let num_params = arg_value @@ -1120,7 +1139,7 @@ impl Engine { KEYWORD_IS_DEF_VAR if total_args == 1 => { let arg = first_arg.unwrap(); let (arg_value, arg_pos) = - self.get_arg_value(scope, global, state, lib, this_ptr, level, arg, constants)?; + self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?; let var_name = arg_value .into_immutable_string() .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos))?; @@ -1133,7 +1152,7 @@ impl Engine { let orig_scope_len = scope.len(); let arg = first_arg.unwrap(); let (arg_value, pos) = - self.get_arg_value(scope, global, state, lib, this_ptr, level, arg, constants)?; + self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?; let script = &arg_value .into_immutable_string() .map_err(|typ| self.make_type_mismatch_err::(typ, pos))?; @@ -1182,7 +1201,7 @@ impl Engine { .map(|&v| v) .chain(a_expr.iter()) .try_for_each(|expr| { - self.get_arg_value(scope, global, state, lib, this_ptr, level, expr, constants) + self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level) .map(|(value, _)| arg_values.push(value.flatten())) })?; args.extend(curry.iter_mut()); @@ -1193,7 +1212,7 @@ impl Engine { return self .exec_fn_call( - global, state, lib, name, hashes, &mut args, is_ref_mut, false, pos, scope, + scope, global, state, lib, name, hashes, &mut args, is_ref_mut, false, pos, level, ) .map(|(v, _)| v); @@ -1216,7 +1235,7 @@ impl Engine { // func(x, ...) -> x.func(...) a_expr.iter().try_for_each(|expr| { - self.get_arg_value(scope, global, state, lib, this_ptr, level, expr, constants) + self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level) .map(|(value, _)| arg_values.push(value.flatten())) })?; @@ -1252,7 +1271,7 @@ impl Engine { .chain(a_expr.iter()) .try_for_each(|expr| { self.get_arg_value( - scope, global, state, lib, this_ptr, level, expr, constants, + scope, global, state, lib, this_ptr, expr, constants, level, ) .map(|(value, _)| arg_values.push(value.flatten())) })?; @@ -1262,7 +1281,7 @@ impl Engine { } self.exec_fn_call( - global, state, lib, name, hashes, &mut args, is_ref_mut, false, pos, None, level, + None, global, state, lib, name, hashes, &mut args, is_ref_mut, false, pos, level, ) .map(|(v, _)| v) } @@ -1305,7 +1324,7 @@ impl Engine { arg_values.push(Dynamic::UNIT); args_expr.iter().skip(1).try_for_each(|expr| { - self.get_arg_value(scope, global, state, lib, this_ptr, level, expr, constants) + self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level) .map(|(value, _)| arg_values.push(value.flatten())) })?; @@ -1335,7 +1354,7 @@ impl Engine { } else { // func(..., ...) or func(mod::x, ...) args_expr.iter().try_for_each(|expr| { - self.get_arg_value(scope, global, state, lib, this_ptr, level, expr, constants) + self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level) .map(|(value, _)| arg_values.push(value.flatten())) })?; args.extend(arg_values.iter_mut()); @@ -1385,7 +1404,7 @@ impl Engine { mem::swap(&mut global.source, &mut source); let result = self.call_script_fn( - new_scope, global, state, lib, &mut None, fn_def, &mut args, pos, true, + new_scope, global, state, lib, &mut None, fn_def, &mut args, true, pos, level, ); diff --git a/src/func/native.rs b/src/func/native.rs index c5787386..43de5bcb 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -321,6 +321,7 @@ impl<'a> NativeCallContext<'a> { self.engine() .exec_fn_call( + None, &mut global, &mut state, self.lib, @@ -330,7 +331,6 @@ impl<'a> NativeCallContext<'a> { is_ref_mut, is_method_call, Position::NONE, - None, self.level + 1, ) .map(|(r, _)| r) diff --git a/src/func/script.rs b/src/func/script.rs index 9c10e76e..66132cc3 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -31,8 +31,8 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, fn_def: &ScriptFnDef, args: &mut FnCallArgs, - pos: Position, rewind_scope: bool, + pos: Position, level: usize, ) -> RhaiResult { #[inline(never)] @@ -73,6 +73,11 @@ impl Engine { return Err(ERR::ErrorStackOverflow(pos).into()); } + #[cfg(feature = "debugging")] + if self.debugger.is_none() && fn_def.body.is_empty() { + return Ok(Dynamic::UNIT); + } + #[cfg(not(feature = "debugging"))] if fn_def.body.is_empty() { return Ok(Dynamic::UNIT); } @@ -136,6 +141,13 @@ impl Engine { (lib, None) }; + #[cfg(feature = "debugging")] + if self.debugger.is_some() { + println!("Level = {}", level); + let node = crate::ast::Stmt::Noop(fn_def.body.position()); + self.run_debugger(scope, global, state, lib, this_ptr, &node, level)?; + } + // Evaluate the function let mut _result = self .eval_stmt_block( @@ -172,7 +184,7 @@ impl Engine { #[cfg(feature = "debugging")] if self.debugger.is_some() { - match global.debugger.status() { + match global.debugger.status { crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { let node = crate::ast::Stmt::Noop(pos); let node = (&node).into(); From 9fa683938089747f8dc622f6dea1fe60c35c112a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 3 Feb 2022 11:56:08 +0800 Subject: [PATCH 12/22] Simplify debugger calls. --- src/eval/chaining.rs | 78 +++++++++++++------------------------------- src/eval/debugger.rs | 61 +++++++++++++++++++++++++++------- src/eval/expr.rs | 18 +++------- src/eval/stmt.rs | 7 ++-- src/func/call.rs | 34 +++++-------------- src/func/script.rs | 3 +- 6 files changed, 90 insertions(+), 111 deletions(-) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 8cd99b15..aada9039 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -156,9 +156,7 @@ impl Engine { if !_terminate_chaining => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; let mut idx_val_for_setter = idx_val.clone(); let idx_pos = x.lhs.position(); @@ -206,9 +204,7 @@ impl Engine { // xxx[rhs] op= new_val _ if new_val.is_some() => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; let ((new_val, new_pos), (op_info, op_pos)) = new_val.expect("`Some`"); let mut idx_val_for_setter = idx_val.clone(); @@ -254,9 +250,7 @@ impl Engine { // xxx[rhs] _ => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; self.get_indexed_mut( global, state, lib, target, idx_val, pos, false, true, level, @@ -275,13 +269,9 @@ impl Engine { let call_args = &mut idx_val.into_fn_call_args(); #[cfg(feature = "debugging")] - let reset_debugger = if self.debugger.is_some() { - self.run_debugger_with_reset( - scope, global, state, lib, this_ptr, rhs, level, - )? - } else { - None - }; + let reset_debugger = self.run_debugger_with_reset( + scope, global, state, lib, this_ptr, rhs, level, + )?; let result = self.make_method_call( global, state, lib, name, *hashes, target, call_args, *pos, level, @@ -303,9 +293,7 @@ impl Engine { // {xxx:map}.id op= ??? Expr::Property(x, pos) if target.is::() && new_val.is_some() => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; let index = x.2.clone().into(); let ((new_val, new_pos), (op_info, op_pos)) = new_val.expect("`Some`"); @@ -326,9 +314,7 @@ impl Engine { // {xxx:map}.id Expr::Property(x, pos) if target.is::() => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; let index = x.2.clone().into(); let val = self.get_indexed_mut( @@ -339,9 +325,7 @@ impl Engine { // xxx.id op= ??? Expr::Property(x, pos) if new_val.is_some() => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; let ((getter, hash_get), (setter, hash_set), name) = x.as_ref(); let ((mut new_val, new_pos), (op_info, op_pos)) = new_val.expect("`Some`"); @@ -421,9 +405,7 @@ impl Engine { // xxx.id Expr::Property(x, pos) => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, rhs, level)?; let ((getter, hash_get), _, name) = x.as_ref(); let hash = crate::ast::FnCallHashes::from_native(*hash_get); @@ -463,11 +445,9 @@ impl Engine { let val_target = &mut match x.lhs { Expr::Property(ref p, pos) => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger( - scope, global, state, lib, this_ptr, _node, level, - )?; - } + self.run_debugger( + scope, global, state, lib, this_ptr, _node, level, + )?; let index = p.2.clone().into(); self.get_indexed_mut( @@ -480,13 +460,9 @@ impl Engine { let call_args = &mut idx_val.into_fn_call_args(); #[cfg(feature = "debugging")] - let reset_debugger = if self.debugger.is_some() { - self.run_debugger_with_reset( - scope, global, state, lib, this_ptr, _node, level, - )? - } else { - None - }; + let reset_debugger = self.run_debugger_with_reset( + scope, global, state, lib, this_ptr, _node, level, + )?; let result = self.make_method_call( global, state, lib, name, *hashes, target, call_args, pos, @@ -521,11 +497,9 @@ impl Engine { // xxx.prop[expr] | xxx.prop.expr Expr::Property(ref p, pos) => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger( - scope, global, state, lib, this_ptr, _node, level, - )?; - } + self.run_debugger( + scope, global, state, lib, this_ptr, _node, level, + )?; let ((getter, hash_get), (setter, hash_set), name) = p.as_ref(); let rhs_chain = rhs.into(); @@ -626,13 +600,9 @@ impl Engine { let args = &mut idx_val.into_fn_call_args(); #[cfg(feature = "debugging")] - let reset_debugger = if self.debugger.is_some() { - self.run_debugger_with_reset( - scope, global, state, lib, this_ptr, _node, level, - )? - } else { - None - }; + let reset_debugger = self.run_debugger_with_reset( + scope, global, state, lib, this_ptr, _node, level, + )?; let result = self.make_method_call( global, state, lib, name, *hashes, target, args, pos, level, @@ -697,9 +667,7 @@ impl Engine { // id.??? or id[???] Expr::Variable(_, var_pos, x) => { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, lhs, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, lhs, level)?; #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, *var_pos)?; diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index f6889c17..d585439b 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -4,9 +4,9 @@ use super::{EvalContext, EvalState, GlobalRuntimeState}; use crate::ast::{ASTNode, Expr, Stmt}; use crate::{Dynamic, Engine, EvalAltResult, Identifier, Module, Position, RhaiResultOf, Scope}; -use std::fmt; #[cfg(feature = "no_std")] use std::prelude::v1::*; +use std::{fmt, mem}; /// Callback function to initialize the debugger. #[cfg(not(feature = "sync"))] @@ -319,11 +319,25 @@ impl Debugger { pos, }); } - /// Set the status of this [`Debugger`]. + /// Change the current status to [`CONTINUE`][DebuggerStatus::CONTINUE] and return the previous status. + pub(crate) fn clear_status_if( + &mut self, + filter: impl Fn(&DebuggerStatus) -> bool, + ) -> Option { + if filter(&self.status) { + Some(mem::replace(&mut self.status, DebuggerStatus::CONTINUE)) + } else { + None + } + } + /// Override the status of this [`Debugger`] if it is [`Some`] the current status is + /// [`CONTINUE`][DebuggerStatus::CONTINUE]. #[inline(always)] pub(crate) fn reset_status(&mut self, status: Option) { - if let Some(cmd) = status { - self.status = cmd; + if self.status == DebuggerStatus::CONTINUE { + if let Some(cmd) = status { + self.status = cmd; + } } } /// Returns the first break-point triggered by a particular [`AST` Node][ASTNode]. @@ -383,7 +397,7 @@ impl Debugger { } impl Engine { - /// Run the debugger callback. + /// Run the debugger callback if there is a debugging interface registered. #[inline(always)] pub(crate) fn run_debugger<'a>( &self, @@ -395,14 +409,40 @@ impl Engine { node: impl Into>, level: usize, ) -> RhaiResultOf<()> { - if let Some(cmd) = - self.run_debugger_with_reset(scope, global, state, lib, this_ptr, node, level)? - { - global.debugger.status = cmd; + if self.debugger.is_some() { + if let Some(cmd) = + self.run_debugger_with_reset_raw(scope, global, state, lib, this_ptr, node, level)? + { + global.debugger.status = cmd; + } } Ok(()) } + /// Run the debugger callback if there is a debugging interface registered. + /// + /// Returns `Some` if the debugger needs to be reactivated at the end of the block, statement or + /// function call. + /// + /// It is up to the [`Engine`] to reactivate the debugger. + #[inline(always)] + #[must_use] + pub(crate) fn run_debugger_with_reset<'a>( + &self, + scope: &mut Scope, + global: &mut GlobalRuntimeState, + state: &mut EvalState, + lib: &[&Module], + this_ptr: &mut Option<&mut Dynamic>, + node: impl Into>, + level: usize, + ) -> RhaiResultOf> { + if self.debugger.is_some() { + self.run_debugger_with_reset_raw(scope, global, state, lib, this_ptr, node, level) + } else { + Ok(None) + } + } /// Run the debugger callback. /// /// Returns `Some` if the debugger needs to be reactivated at the end of the block, statement or @@ -411,7 +451,7 @@ impl Engine { /// It is up to the [`Engine`] to reactivate the debugger. #[inline] #[must_use] - pub(crate) fn run_debugger_with_reset<'a>( + pub(crate) fn run_debugger_with_reset_raw<'a>( &self, scope: &mut Scope, global: &mut GlobalRuntimeState, @@ -513,7 +553,6 @@ impl Engine { | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(_, _))) => context.call_level() + 1, _ => context.call_level(), }; - println!("Set FunctionExit to {}", level); global.debugger.status = DebuggerStatus::FunctionExit(level); Ok(None) } diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 79f27d92..520f8283 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -266,11 +266,8 @@ impl Engine { // binary operators are also function calls. if let Expr::FnCall(x, pos) = expr { #[cfg(feature = "debugging")] - let reset_debugger = if self.debugger.is_some() { - self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)? - } else { - None - }; + let reset_debugger = + self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)?; #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, expr.position())?; @@ -289,9 +286,7 @@ impl Engine { // will cost more than the mis-predicted `match` branch. if let Expr::Variable(index, var_pos, x) = expr { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, expr, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, expr, level)?; #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, expr.position())?; @@ -308,11 +303,8 @@ impl Engine { } #[cfg(feature = "debugging")] - let reset_debugger = if self.debugger.is_some() { - self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)? - } else { - None - }; + let reset_debugger = + self.run_debugger_with_reset(scope, global, state, lib, this_ptr, expr, level)?; #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, expr.position())?; diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index bb4645da..065e251a 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -204,11 +204,8 @@ impl Engine { level: usize, ) -> RhaiResult { #[cfg(feature = "debugging")] - let reset_debugger = if self.debugger.is_some() { - self.run_debugger_with_reset(scope, global, state, lib, this_ptr, stmt, level)? - } else { - None - }; + let reset_debugger = + self.run_debugger_with_reset(scope, global, state, lib, this_ptr, stmt, level)?; // Coded this way for better branch prediction. // Popular branches are lifted out of the `match` statement into their own branches. diff --git a/src/func/call.rs b/src/func/call.rs index 240b050c..ab719c14 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -956,35 +956,24 @@ impl Engine { Ok(( if let Expr::Stack(slot, _) = arg_expr { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; constants[*slot].clone() } else if let Some(value) = arg_expr.get_literal_value() { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?; value } else { // Do not match function exit for arguments #[cfg(feature = "debugging")] - let reset_debugger = match global.debugger.status { - crate::eval::DebuggerStatus::FunctionExit(_) => { - Some(std::mem::take(&mut global.debugger.status)) - } - _ => None, - }; + let reset_debugger = global.debugger.clear_status_if(|status| { + matches!(status, crate::eval::DebuggerStatus::FunctionExit(_)) + }); let result = self.eval_expr(scope, global, state, lib, this_ptr, arg_expr, level); - // Restore function exit if status is not active + // Restore function exit status #[cfg(feature = "debugging")] - if self.debugger.is_some() - && global.debugger.status == crate::eval::DebuggerStatus::CONTINUE - { - global.debugger.reset_status(reset_debugger); - } + global.debugger.reset_status(reset_debugger); result? }, @@ -1229,9 +1218,7 @@ impl Engine { let first_expr = first_arg.unwrap(); #[cfg(feature = "debugging")] - if self.debugger.is_some() { - self.run_debugger(scope, global, state, lib, this_ptr, first_expr, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, first_expr, level)?; // func(x, ...) -> x.func(...) a_expr.iter().try_for_each(|expr| { @@ -1315,10 +1302,7 @@ impl Engine { // &mut first argument and avoid cloning the value if !args_expr.is_empty() && args_expr[0].is_variable_access(true) { #[cfg(feature = "debugging")] - if self.debugger.is_some() { - let node = &args_expr[0]; - self.run_debugger(scope, global, state, lib, this_ptr, node, level)?; - } + self.run_debugger(scope, global, state, lib, this_ptr, &args_expr[0], level)?; // func(x, ...) -> x.func(...) arg_values.push(Dynamic::UNIT); diff --git a/src/func/script.rs b/src/func/script.rs index 66132cc3..77397f1e 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -142,8 +142,7 @@ impl Engine { }; #[cfg(feature = "debugging")] - if self.debugger.is_some() { - println!("Level = {}", level); + { let node = crate::ast::Stmt::Noop(fn_def.body.position()); self.run_debugger(scope, global, state, lib, this_ptr, &node, level)?; } From 419ee450432c64d643441095078b1415a1141ff4 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 3 Feb 2022 21:17:47 +0800 Subject: [PATCH 13/22] Add bin-features to pull in all features for bin tools. --- CHANGELOG.md | 3 ++- Cargo.toml | 4 ++++ src/bin/README.md | 17 +++++++---------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42f989ef..5ca2686b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Bug fixes Script-breaking changes ----------------------- -* For consistency, the `export` statement no longer exports multiple variables. +* For consistency with the `import` statement, the `export` statement no longer exports multiple variables. New features ------------ @@ -33,6 +33,7 @@ Enhancements * The `no_module` feature now eliminates large sections of code via feature gates. * Debug display of `AST` is improved. * `NativeCallContext::call_level()` is added to give the current nesting level of function calls. +* A new feature, `bin-features`, pulls in all the required features for `bin` tools. REPL tool changes ----------------- diff --git a/Cargo.toml b/Cargo.toml index 2d687d30..c702f73f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,12 +55,16 @@ unicode-xid-ident = ["unicode-xid"] # allow Unicode Standard Annex #31 for ident metadata = ["serde", "serde_json", "rhai_codegen/metadata", "smartstring/serde"] # enable exporting functions metadata debugging = ["internals"] # enable debugging +# compiling for no-std no_std = ["no-std-compat", "num-traits/libm", "core-error", "libm", "ahash/compile-time-rng"] # compiling for WASM wasm-bindgen = ["instant/wasm-bindgen"] stdweb = ["instant/stdweb"] +# compiling bin tools +bin-features = ["decimal", "metadata", "serde", "debugging", "rustyline"] + [[bin]] name = "rhai-repl" required-features = ["rustyline"] diff --git a/src/bin/README.md b/src/bin/README.md index e0648866..32ba31ae 100644 --- a/src/bin/README.md +++ b/src/bin/README.md @@ -9,32 +9,29 @@ Tools for running Rhai scripts. | [`rhai-repl`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-repl.rs) | `rustyline` | simple REPL that interactively evaluates statements | | [`rhai-dbg`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-dbg.rs) | `debugging` | the _Rhai Debugger_ | +There is a feature called `bin-features` which automatically includes all the necessary features +required for building these tools. + How to Run ---------- ```sh -cargo run --bin sample_app_to_run -``` - -or with required features - -```sh -cargo run --bin sample_app_to_run --features feature1,feature2,feature3 +cargo run --features bin-features --bin sample_app_to_run ``` How to Install -------------- -To install these all tools (with [`decimal`] and [`metadata`] support), use the following command: +To install these all tools (with full features), use the following command: ```sh -cargo install --path . --bins --features decimal,metadata,debugging,rustyline +cargo install --path . --bins --features bin-features ``` or specifically: ```sh -cargo install --path . --bin rhai-run --features decimal,metadata,debugging,rustyline +cargo install --path . --bin rhai-run --features bin-features ``` From 345a060672808d179f4218ec8caeb098dd206e2b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 3 Feb 2022 23:54:53 +0800 Subject: [PATCH 14/22] Fix type name display. --- CHANGELOG.md | 1 + src/api/mod.rs | 2 + src/api/type_names.rs | 132 ++++++++++++++++++++++++++++++++++++++++++ src/engine.rs | 62 +------------------- src/lib.rs | 2 + src/types/dynamic.rs | 72 +---------------------- src/types/mod.rs | 2 + 7 files changed, 144 insertions(+), 129 deletions(-) create mode 100644 src/api/type_names.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ca2686b..436e20df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Bug fixes * Variables introduced inside `try` blocks are now properly cleaned up upon an exception. * Off-by-one error in character positions after a comment line is now fixed. * Globally-defined constants are now encapsulated correctly inside a loaded module and no longer spill across call boundaries. +* Type names display is fixed. Script-breaking changes ----------------------- diff --git a/src/api/mod.rs b/src/api/mod.rs index 39045da7..b7d9b8f2 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -1,5 +1,7 @@ //! Module defining the public API of the Rhai engine. +pub mod type_names; + pub mod eval; pub mod run; diff --git a/src/api/type_names.rs b/src/api/type_names.rs new file mode 100644 index 00000000..8e7162a2 --- /dev/null +++ b/src/api/type_names.rs @@ -0,0 +1,132 @@ +use crate::{ + Engine, ExclusiveRange, FnPtr, ImmutableString, InclusiveRange, Instant, Position, RhaiError, + ERR, +}; +use std::any::type_name; +#[cfg(feature = "no_std")] +use std::prelude::v1::*; + +/// Map the name of a standard type into a friendly form. +#[inline] +#[must_use] +fn map_std_type_name(name: &str, shorthands: bool) -> &str { + let name = name.trim(); + + if name == type_name::() { + return if shorthands { "string" } else { "String" }; + } + if name == type_name::() || name == "ImmutableString" { + return if shorthands { + "string" + } else { + "ImmutableString" + }; + } + if name == type_name::<&str>() { + return if shorthands { "string" } else { "&str" }; + } + #[cfg(feature = "decimal")] + if name == type_name::() { + return if shorthands { "decimal" } else { "Decimal" }; + } + if name == type_name::() || name == "FnPtr" { + return if shorthands { "Fn" } else { "FnPtr" }; + } + #[cfg(not(feature = "no_index"))] + if name == type_name::() || name == "Array" { + return if shorthands { "array" } else { "Array" }; + } + #[cfg(not(feature = "no_index"))] + if name == type_name::() || name == "Blob" { + return if shorthands { "blob" } else { "Blob" }; + } + #[cfg(not(feature = "no_object"))] + if name == type_name::() || name == "Map" { + return if shorthands { "map" } else { "Map" }; + } + #[cfg(not(feature = "no_std"))] + if name == type_name::() || name == "Instant" { + return if shorthands { "timestamp" } else { "Instant" }; + } + if name == type_name::() || name == "ExclusiveRange" { + return if shorthands { + "range" + } else if cfg!(feature = "only_i32") { + "Range" + } else { + "Range" + }; + } + if name == type_name::() || name == "InclusiveRange" { + return if shorthands { + "range=" + } else if cfg!(feature = "only_i32") { + "RangeInclusive" + } else { + "RangeInclusive" + }; + } + + if name.starts_with("rhai::") { + map_std_type_name(&name[6..], shorthands) + } else { + name + } +} + +impl Engine { + /// Pretty-print a type name. + /// + /// If a type is registered via [`register_type_with_name`][Engine::register_type_with_name], + /// the type name provided for the registration will be used. + /// + /// # Panics + /// + /// Panics if the type name is `&mut`. + #[inline] + #[must_use] + pub fn map_type_name<'a>(&'a self, name: &'a str) -> &'a str { + self.type_names + .get(name) + .map(|s| s.as_str()) + .unwrap_or_else(|| map_std_type_name(name, true)) + } + + /// Format a type name. + /// + /// If a type is registered via [`register_type_with_name`][Engine::register_type_with_name], + /// the type name provided for the registration will be used. + #[cfg(feature = "metadata")] + #[inline] + #[must_use] + pub(crate) fn format_type_name<'a>(&'a self, name: &'a str) -> std::borrow::Cow<'a, str> { + if name.starts_with("&mut ") { + let x = &name[5..]; + let r = self.format_type_name(x); + return if x != r { + format!("&mut {}", r).into() + } else { + name.into() + }; + } + + self.type_names + .get(name) + .map(|s| s.as_str()) + .unwrap_or_else(|| match name { + "INT" => return type_name::(), + #[cfg(not(feature = "no_float"))] + "FLOAT" => return type_name::(), + _ => map_std_type_name(name, false), + }) + .into() + } + + /// Make a `Box<`[`EvalAltResult`][ERR::ErrorMismatchDataType]`>`. + #[inline(never)] + #[must_use] + pub(crate) fn make_type_mismatch_err(&self, typ: &str, pos: Position) -> RhaiError { + ERR::ErrorMismatchDataType(self.map_type_name(type_name::()).into(), typ.into(), pos) + .into() + } +} diff --git a/src/engine.rs b/src/engine.rs index 14206d6f..aa5a36d1 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -4,15 +4,14 @@ use crate::api::custom_syntax::CustomSyntax; use crate::func::native::{OnDebugCallback, OnParseTokenCallback, OnPrintCallback, OnVarCallback}; use crate::packages::{Package, StandardPackage}; use crate::tokenizer::Token; -use crate::types::dynamic::{map_std_type_name, Union}; +use crate::types::dynamic::{ Union}; use crate::{ - Dynamic, Identifier, ImmutableString, Module, Position, RhaiError, RhaiResult, Shared, - StaticVec, ERR, + Dynamic, Identifier, ImmutableString, Module, Position, RhaiResult, Shared, + StaticVec, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ - any::type_name, collections::{BTreeMap, BTreeSet}, fmt, num::NonZeroU8, @@ -337,59 +336,4 @@ impl Engine { result } - - /// Pretty-print a type name. - /// - /// If a type is registered via [`register_type_with_name`][Engine::register_type_with_name], - /// the type name provided for the registration will be used. - /// - /// # Panics - /// - /// Panics if the type name is `&mut`. - #[inline] - #[must_use] - pub fn map_type_name<'a>(&'a self, name: &'a str) -> &'a str { - self.type_names - .get(name) - .map(|s| s.as_str()) - .unwrap_or_else(|| map_std_type_name(name, true)) - } - - /// Format a type name. - /// - /// If a type is registered via [`register_type_with_name`][Engine::register_type_with_name], - /// the type name provided for the registration will be used. - #[cfg(feature = "metadata")] - #[inline] - #[must_use] - pub(crate) fn format_type_name<'a>(&'a self, name: &'a str) -> std::borrow::Cow<'a, str> { - if name.starts_with("&mut ") { - let x = &name[5..]; - let r = self.format_type_name(x); - return if x != r { - format!("&mut {}", r).into() - } else { - name.into() - }; - } - - self.type_names - .get(name) - .map(|s| s.as_str()) - .unwrap_or_else(|| match name { - "INT" => return type_name::(), - #[cfg(not(feature = "no_float"))] - "FLOAT" => return type_name::(), - _ => map_std_type_name(name, false), - }) - .into() - } - - /// Make a `Box<`[`EvalAltResult`][ERR::ErrorMismatchDataType]`>`. - #[inline] - #[must_use] - pub(crate) fn make_type_mismatch_err(&self, typ: &str, pos: Position) -> RhaiError { - ERR::ErrorMismatchDataType(self.map_type_name(type_name::()).into(), typ.into(), pos) - .into() - } } diff --git a/src/lib.rs b/src/lib.rs index a94f146e..ea94ed65 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -154,6 +154,8 @@ pub use eval::EvalContext; pub use func::{NativeCallContext, RegisterNativeFunction}; pub use module::{FnNamespace, Module}; pub use tokenizer::Position; +#[cfg(not(feature = "no_std"))] +pub use types::Instant; pub use types::{ Dynamic, EvalAltResult, FnPtr, ImmutableString, LexError, ParseError, ParseErrorType, Scope, }; diff --git a/src/types/dynamic.rs b/src/types/dynamic.rs index 0b8fdf80..48d0d124 100644 --- a/src/types/dynamic.rs +++ b/src/types/dynamic.rs @@ -16,11 +16,11 @@ use std::{ #[cfg(not(feature = "no_std"))] #[cfg(not(target_family = "wasm"))] -use std::time::Instant; +pub use std::time::Instant; #[cfg(not(feature = "no_std"))] #[cfg(target_family = "wasm")] -use instant::Instant; +pub use instant::Instant; /// The message: data type was checked const CHECKED: &str = "data type was checked"; @@ -543,74 +543,6 @@ impl Hash for Dynamic { } } -/// Map the name of a standard type into a friendly form. -#[inline] -#[must_use] -pub(crate) fn map_std_type_name(name: &str, shorthands: bool) -> &str { - let name = name.trim(); - - if name.starts_with("rhai::") { - return map_std_type_name(&name[6..], shorthands); - } - - if name == type_name::() { - return if shorthands { "string" } else { "String" }; - } - if name == type_name::() { - return if shorthands { - "string" - } else { - "ImmutableString" - }; - } - if name == type_name::<&str>() { - return if shorthands { "string" } else { "&str" }; - } - #[cfg(feature = "decimal")] - if name == type_name::() { - return if shorthands { "decimal" } else { "Decimal" }; - } - if name == type_name::() { - return if shorthands { "Fn" } else { "FnPtr" }; - } - #[cfg(not(feature = "no_index"))] - if name == type_name::() { - return if shorthands { "array" } else { "Array" }; - } - #[cfg(not(feature = "no_index"))] - if name == type_name::() { - return if shorthands { "blob" } else { "Blob" }; - } - #[cfg(not(feature = "no_object"))] - if name == type_name::() { - return if shorthands { "map" } else { "Map" }; - } - #[cfg(not(feature = "no_std"))] - if name == type_name::() { - return if shorthands { "timestamp" } else { "Instant" }; - } - if name == type_name::() || name == "ExclusiveRange" { - return if shorthands { - "range" - } else if cfg!(feature = "only_i32") { - "Range" - } else { - "Range" - }; - } - if name == type_name::() || name == "InclusiveRange" { - return if shorthands { - "range=" - } else if cfg!(feature = "only_i32") { - "RangeInclusive" - } else { - "RangeInclusive" - }; - } - - name -} - impl fmt::Display for Dynamic { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.0 { diff --git a/src/types/mod.rs b/src/types/mod.rs index 1012f8b1..66da654c 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -9,6 +9,8 @@ pub mod parse_error; pub mod scope; pub use dynamic::Dynamic; +#[cfg(not(feature = "no_std"))] +pub use dynamic::Instant; pub use error::EvalAltResult; pub use fn_ptr::FnPtr; pub use immutable_string::ImmutableString; From 6c1c8bc53875acd6cdc0cab77e87d0bdefa7bcba Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 12:04:33 +0800 Subject: [PATCH 15/22] Improve position display. --- CHANGELOG.md | 3 ++ src/ast/ast.rs | 2 +- src/ast/expr.rs | 29 ++++++++++++--- src/ast/stmt.rs | 65 +++++++++++++++++++++++++-------- src/eval/chaining.rs | 15 ++++---- src/eval/expr.rs | 8 ++--- src/eval/stmt.rs | 11 +++--- src/func/call.rs | 60 +++++++++++++++---------------- src/func/script.rs | 32 +++++++++-------- src/optimizer.rs | 39 +++++++++++--------- src/parser.rs | 86 ++++++++++++++++++++++++-------------------- 11 files changed, 212 insertions(+), 138 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 436e20df..3f7a2997 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ Enhancements * Debug display of `AST` is improved. * `NativeCallContext::call_level()` is added to give the current nesting level of function calls. * A new feature, `bin-features`, pulls in all the required features for `bin` tools. +* `AST` position display is improved: + * `Expr::start_position` is added to give the beginning of the expression (not the operator's position). + * `StmtBlock` and `Stmt::Block` now keep the position of the closing `}` as well. REPL tool changes ----------------- diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 18b02cff..6fb96e85 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -91,7 +91,7 @@ impl AST { ) -> Self { Self { source: Identifier::new_const(), - body: StmtBlock::new(statements, Position::NONE), + body: StmtBlock::new(statements, Position::NONE, Position::NONE), #[cfg(not(feature = "no_function"))] lib: functions.into(), #[cfg(not(feature = "no_module"))] diff --git a/src/ast/expr.rs b/src/ast/expr.rs index a9881804..dffacf42 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -191,6 +191,8 @@ pub struct FnCallExpr { pub constants: StaticVec, /// Does this function call capture the parent scope? pub capture_parent_scope: bool, + /// [Position] of the function name. + pub pos: Position, } impl fmt::Debug for FnCallExpr { @@ -207,6 +209,7 @@ impl fmt::Debug for FnCallExpr { if self.capture_parent_scope { ff.field("capture_parent_scope", &self.capture_parent_scope); } + ff.field("pos", &self.pos); ff.finish() } } @@ -437,7 +440,7 @@ impl Default for Expr { impl fmt::Debug for Expr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut display_pos = self.position(); + let mut display_pos = self.start_position(); match self { Self::DynamicConstant(value, _) => write!(f, "{:?}", value), @@ -629,6 +632,7 @@ impl Expr { args: once(Self::Stack(0, pos)).collect(), constants: once(f.fn_name().into()).collect(), capture_parent_scope: false, + pos, } .into(), pos, @@ -685,15 +689,30 @@ impl Expr { | Self::Map(_, pos) | Self::Variable(_, pos, _) | Self::Stack(_, pos) - | Self::FnCall(_, pos) + | Self::And(_, pos) + | Self::Or(_, pos) | Self::Index(_, _, pos) + | Self::Dot(_, _, pos) | Self::Custom(_, pos) | Self::InterpolatedString(_, pos) | Self::Property(_, pos) => *pos, - Self::Stmt(x) => x.position(), + Self::FnCall(x, _) => x.pos, - Self::And(x, _) | Self::Or(x, _) | Self::Dot(x, _, _) => x.lhs.position(), + Self::Stmt(x) => x.position(), + } + } + /// Get the starting [position][Position] of the expression. + /// For a binary expression, this will be the left-most LHS instead of the operator. + #[inline] + #[must_use] + pub const fn start_position(&self) -> Position { + match self { + Self::And(x, _) | Self::Or(x, _) | Self::Index(x, _, _) | Self::Dot(x, _, _) => { + x.lhs.start_position() + } + Self::FnCall(_, pos) => *pos, + _ => self.position(), } } /// Override the [position][Position] of the expression. @@ -722,7 +741,7 @@ impl Expr { | Self::InterpolatedString(_, pos) | Self::Property(_, pos) => *pos = new_pos, - Self::Stmt(x) => x.set_position(new_pos), + Self::Stmt(x) => x.set_position(new_pos, Position::NONE), } self diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index f9f6717c..ca82fb8f 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -134,7 +134,7 @@ pub struct TryCatchBlock { /// _(internals)_ A scoped block of statements. /// Exported under the `internals` feature only. #[derive(Clone, Hash, Default)] -pub struct StmtBlock(StaticVec, Position); +pub struct StmtBlock(StaticVec, (Position, Position)); impl StmtBlock { /// A [`StmtBlock`] that does not exist. @@ -142,16 +142,20 @@ impl StmtBlock { /// Create a new [`StmtBlock`]. #[must_use] - pub fn new(statements: impl IntoIterator, pos: Position) -> Self { + pub fn new( + statements: impl IntoIterator, + start_pos: Position, + end_pos: Position, + ) -> Self { let mut statements: StaticVec<_> = statements.into_iter().collect(); statements.shrink_to_fit(); - Self(statements, pos) + Self(statements, (start_pos, end_pos)) } /// Create an empty [`StmtBlock`]. #[inline(always)] #[must_use] pub const fn empty(pos: Position) -> Self { - Self(StaticVec::new_const(), pos) + Self(StaticVec::new_const(), (pos, pos)) } /// Is this statements block empty? #[inline(always)] @@ -183,16 +187,42 @@ impl StmtBlock { pub fn iter(&self) -> impl Iterator { self.0.iter() } - /// Get the position (location of the beginning `{`) of this statements block. + /// Get the start position (location of the beginning `{`) of this statements block. #[inline(always)] #[must_use] pub const fn position(&self) -> Position { + (self.1).0 + } + /// Get the end position (location of the ending `}`) of this statements block. + #[inline(always)] + #[must_use] + pub const fn end_position(&self) -> Position { + (self.1).1 + } + /// Get the positions (locations of the beginning `{` and ending `}`) of this statements block. + #[inline(always)] + #[must_use] + pub const fn positions(&self) -> (Position, Position) { self.1 } - /// Set the position (location of the beginning `{`) of this statements block. + /// Get the positions (locations of the beginning `{` and ending `}`) of this statements block + /// or a default. #[inline(always)] - pub fn set_position(&mut self, pos: Position) { - self.1 = pos; + #[must_use] + pub const fn positions_or_else( + &self, + def_start_pos: Position, + def_end_pos: Position, + ) -> (Position, Position) { + ( + (self.1).0.or_else(def_start_pos), + (self.1).1.or_else(def_end_pos), + ) + } + /// Set the positions of this statements block. + #[inline(always)] + pub fn set_position(&mut self, start_pos: Position, end_pos: Position) { + self.1 = (start_pos, end_pos); } } @@ -230,7 +260,12 @@ impl fmt::Debug for StmtBlock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("Block")?; fmt::Debug::fmt(&self.0, f)?; - self.1.debug_print(f) + (self.1).0.debug_print(f)?; + #[cfg(not(feature = "no_position"))] + if !(self.1).1.is_none() { + write!(f, "-{:?}", (self.1).1)?; + } + Ok(()) } } @@ -239,10 +274,10 @@ impl From for StmtBlock { fn from(stmt: Stmt) -> Self { match stmt { Stmt::Block(mut block, pos) => Self(block.iter_mut().map(mem::take).collect(), pos), - Stmt::Noop(pos) => Self(StaticVec::new_const(), pos), + Stmt::Noop(pos) => Self(StaticVec::new_const(), (pos, pos)), _ => { let pos = stmt.position(); - Self(vec![stmt].into(), pos) + Self(vec![stmt].into(), (pos, Position::NONE)) } } } @@ -309,7 +344,7 @@ pub enum Stmt { /// function call forming one statement. FnCall(Box, Position), /// `{` stmt`;` ... `}` - Block(Box<[Stmt]>, Position), + Block(Box<[Stmt]>, (Position, Position)), /// `try` `{` stmt; ... `}` `catch` `(` var `)` `{` stmt; ... `}` TryCatch(Box, Position), /// [expression][Expr] @@ -377,7 +412,7 @@ impl Stmt { match self { Self::Noop(pos) | Self::BreakLoop(_, pos) - | Self::Block(_, pos) + | Self::Block(_, (pos, _)) | Self::Assignment(_, pos) | Self::FnCall(_, pos) | Self::If(_, _, pos) @@ -389,7 +424,7 @@ impl Stmt { | Self::Var(_, _, _, pos) | Self::TryCatch(_, pos) => *pos, - Self::Expr(x) => x.position(), + Self::Expr(x) => x.start_position(), #[cfg(not(feature = "no_module"))] Self::Import(_, _, pos) => *pos, @@ -405,7 +440,7 @@ impl Stmt { match self { Self::Noop(pos) | Self::BreakLoop(_, pos) - | Self::Block(_, pos) + | Self::Block(_, (pos, _)) | Self::Assignment(_, pos) | Self::FnCall(_, pos) | Self::If(_, _, pos) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index aada9039..c681ce5a 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -146,7 +146,7 @@ impl Engine { match chain_type { #[cfg(not(feature = "no_index"))] ChainType::Indexing => { - let pos = rhs.position(); + let pos = rhs.start_position(); let root_pos = idx_val.position(); let idx_val = idx_val.into_index_value().expect("`ChainType::Index`"); @@ -159,7 +159,7 @@ impl Engine { self.run_debugger(scope, global, state, lib, this_ptr, _parent, level)?; let mut idx_val_for_setter = idx_val.clone(); - let idx_pos = x.lhs.position(); + let idx_pos = x.lhs.start_position(); let rhs_chain = rhs.into(); let (try_setter, result) = { @@ -629,7 +629,7 @@ impl Engine { } } // Syntax error - _ => Err(ERR::ErrorDotExpr("".into(), rhs.position()).into()), + _ => Err(ERR::ErrorDotExpr("".into(), rhs.start_position()).into()), } } } @@ -691,7 +691,7 @@ impl Engine { expr => { let value = self.eval_expr(scope, global, state, lib, this_ptr, expr, level)?; let obj_ptr = &mut value.into(); - let root = ("", expr.position()); + let root = ("", expr.start_position()); self.eval_dot_index_chain_helper( global, state, lib, this_ptr, obj_ptr, root, expr, rhs, term, idx_values, chain_type, level, new_val, @@ -804,7 +804,10 @@ impl Engine { _ if _parent_chain_type == ChainType::Indexing => self .eval_expr(scope, global, state, lib, this_ptr, lhs, level) .map(|v| { - super::ChainArgument::from_index_value(v.flatten(), lhs.position()) + super::ChainArgument::from_index_value( + v.flatten(), + lhs.start_position(), + ) })?, expr => unreachable!("unknown chained expression: {:?}", expr), }; @@ -828,7 +831,7 @@ impl Engine { _ if _parent_chain_type == ChainType::Indexing => idx_values.push( self.eval_expr(scope, global, state, lib, this_ptr, expr, level) .map(|v| { - super::ChainArgument::from_index_value(v.flatten(), expr.position()) + super::ChainArgument::from_index_value(v.flatten(), expr.start_position()) })?, ), _ => unreachable!("unknown chained expression: {:?}", expr), diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 520f8283..dcbb830b 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -148,7 +148,7 @@ impl Engine { Err(ERR::ErrorUnboundThis(*pos).into()) } } - _ if state.always_search_scope => (0, expr.position()), + _ if state.always_search_scope => (0, expr.start_position()), Expr::Variable(Some(i), pos, _) => (i.get() as usize, *pos), Expr::Variable(None, pos, v) => (v.0.map(NonZeroUsize::get).unwrap_or(0), *pos), _ => unreachable!("Expr::Variable expected but gets {:?}", expr), @@ -347,11 +347,11 @@ impl Engine { item, level, ) { - result = Err(err.fill_position(expr.position())); + result = Err(err.fill_position(expr.start_position())); break; } - pos = expr.position(); + pos = expr.start_position(); } result.map(|_| concat) @@ -504,7 +504,7 @@ impl Engine { let result = (custom_def.func)(&mut context, &expressions); - self.check_return_value(result, expr.position()) + self.check_return_value(result, expr.start_position()) } Expr::Stmt(x) if x.is_empty() => Ok(Dynamic::UNIT), diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 065e251a..8e1e7c5c 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -267,7 +267,7 @@ impl Engine { rhs_val, level, ) - .map_err(|err| err.fill_position(rhs.position())) + .map_err(|err| err.fill_position(rhs.start_position())) .map(|_| Dynamic::UNIT) } else { search_result.map(|_| Dynamic::UNIT) @@ -283,7 +283,7 @@ impl Engine { .map(Dynamic::flatten); if let Ok(rhs_val) = rhs_result { - let _new_val = Some(((rhs_val, rhs.position()), (*op_info, *op_pos))); + let _new_val = Some(((rhs_val, rhs.start_position()), (*op_info, *op_pos))); // Must be either `var[index] op= val` or `var.prop op= val` match lhs { @@ -686,7 +686,7 @@ impl Engine { loop_result } else { - Err(ERR::ErrorFor(expr.position()).into()) + Err(ERR::ErrorFor(expr.start_position()).into()) } } else { iter_result @@ -869,9 +869,10 @@ impl Engine { let path_result = self .eval_expr(scope, global, state, lib, this_ptr, &expr, level) .and_then(|v| { + let typ = v.type_name(); v.try_cast::().ok_or_else(|| { self.make_type_mismatch_err::( - "", + typ, expr.position(), ) }) @@ -880,7 +881,7 @@ impl Engine { if let Ok(path) = path_result { use crate::ModuleResolver; - let path_pos = expr.position(); + let path_pos = expr.start_position(); let resolver = global.embedded_module_resolver.clone(); diff --git a/src/func/call.rs b/src/func/call.rs index ab719c14..292bd33f 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -426,23 +426,26 @@ impl Engine { }; #[cfg(feature = "debugging")] - if self.debugger.is_some() { - match global.debugger.status { - crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { - let scope = &mut &mut Scope::new(); - let node = crate::ast::Stmt::Noop(pos); - let node = (&node).into(); - let event = match _result { - Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), - Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), - }; - if let Err(err) = self.run_debugger_raw( - scope, global, state, lib, &mut None, node, event, level, - ) { - _result = Err(err); - } + { + let trigger = match global.debugger.status { + crate::eval::DebuggerStatus::FunctionExit(n) => n >= level, + crate::eval::DebuggerStatus::Next(_, true) => true, + _ => false, + }; + if trigger { + let scope = &mut &mut Scope::new(); + let node = crate::ast::Stmt::Noop(pos); + let node = (&node).into(); + let event = match _result { + Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), + Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), + }; + match self + .run_debugger_raw(scope, global, state, lib, &mut None, node, event, level) + { + Ok(_) => (), + Err(err) => _result = Err(err), } - _ => (), } // Pop the call stack @@ -977,7 +980,7 @@ impl Engine { result? }, - arg_expr.position(), + arg_expr.start_position(), )) } @@ -1378,24 +1381,17 @@ impl Engine { #[cfg(not(feature = "no_function"))] 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); - if fn_def.body.is_empty() { - Ok(Dynamic::UNIT) - } else { - let new_scope = &mut Scope::new(); + let result = self.call_script_fn( + new_scope, global, state, lib, &mut None, fn_def, &mut args, true, pos, level, + ); - let mut source = module.id_raw().clone(); - mem::swap(&mut global.source, &mut source); + global.source = source; - let result = self.call_script_fn( - new_scope, global, state, lib, &mut None, fn_def, &mut args, true, pos, - level, - ); - - global.source = source; - - result - } + result } Some(f) if f.is_plugin_fn() => { diff --git a/src/func/script.rs b/src/func/script.rs index 77397f1e..7b4236a8 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -182,22 +182,24 @@ impl Engine { }); #[cfg(feature = "debugging")] - if self.debugger.is_some() { - match global.debugger.status { - crate::eval::DebuggerStatus::FunctionExit(n) if n >= level => { - let node = crate::ast::Stmt::Noop(pos); - let node = (&node).into(); - let event = match _result { - Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), - Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), - }; - if let Err(err) = self - .run_debugger_raw(scope, global, state, lib, this_ptr, node, event, level) - { - _result = Err(err); - } + { + let trigger = match global.debugger.status { + crate::eval::DebuggerStatus::FunctionExit(n) => n >= level, + crate::eval::DebuggerStatus::Next(_, true) => true, + _ => false, + }; + if trigger { + let node = crate::ast::Stmt::Noop(fn_def.body.end_position().or_else(pos)); + let node = (&node).into(); + let event = match _result { + Ok(ref r) => crate::eval::DebuggerEvent::FunctionExitWithValue(r), + Err(ref err) => crate::eval::DebuggerEvent::FunctionExitWithError(err), + }; + match self.run_debugger_raw(scope, global, state, lib, this_ptr, node, event, level) + { + Ok(_) => (), + Err(err) => _result = Err(err), } - _ => (), } // Pop the call stack diff --git a/src/optimizer.rs b/src/optimizer.rs index acae357e..df9bc26d 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -463,13 +463,16 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b Stmt::If(condition, x, _) if x.0.is_empty() && x.1.is_empty() => { state.set_dirty(); - let pos = condition.position(); + let pos = condition.start_position(); let mut expr = mem::take(condition); optimize_expr(&mut expr, state, false); *stmt = if preserve_result { // -> { expr, Noop } - Stmt::Block([Stmt::Expr(expr), Stmt::Noop(pos)].into(), pos) + Stmt::Block( + [Stmt::Expr(expr), Stmt::Noop(pos)].into(), + (pos, Position::NONE), + ) } else { // -> expr Stmt::Expr(expr) @@ -487,7 +490,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b match optimize_stmt_block(mem::take(&mut *x.1), state, preserve_result, true, false) { statements if statements.is_empty() => Stmt::Noop(x.1.position()), - statements => Stmt::Block(statements.into_boxed_slice(), x.1.position()), + statements => Stmt::Block(statements.into_boxed_slice(), x.1.positions()), } } // if true { if_block } else { else_block } -> if_block @@ -497,7 +500,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b match optimize_stmt_block(mem::take(&mut *x.0), state, preserve_result, true, false) { statements if statements.is_empty() => Stmt::Noop(x.0.position()), - statements => Stmt::Block(statements.into_boxed_slice(), x.0.position()), + statements => Stmt::Block(statements.into_boxed_slice(), x.0.positions()), } } // if expr { if_block } else { else_block } @@ -531,11 +534,11 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b mem::take(&mut block.statements), Stmt::Block( def_stmt.into_boxed_slice(), - x.def_case.position().or_else(*pos), + x.def_case.positions_or_else(*pos, Position::NONE), ) .into(), )), - match_expr.position(), + match_expr.start_position(), ); } else { // Promote the matched case @@ -546,7 +549,8 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b true, false, ); - *stmt = Stmt::Block(statements.into_boxed_slice(), block.statements.position()); + *stmt = + Stmt::Block(statements.into_boxed_slice(), block.statements.positions()); } state.set_dirty(); @@ -586,11 +590,11 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b mem::take(&mut block.statements), Stmt::Block( def_stmt.into_boxed_slice(), - x.def_case.position().or_else(*pos), + x.def_case.positions_or_else(*pos, Position::NONE), ) .into(), )), - match_expr.position(), + match_expr.start_position(), ); } else { // Promote the matched case @@ -599,7 +603,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b optimize_stmt_block(statements, state, true, true, false); *stmt = Stmt::Block( statements.into_boxed_slice(), - block.statements.position(), + block.statements.positions(), ); } @@ -647,7 +651,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b optimize_stmt_block(mem::take(&mut x.def_case), state, true, true, false); *stmt = Stmt::Block( def_stmt.into_boxed_slice(), - x.def_case.position().or_else(*pos), + x.def_case.positions_or_else(*pos, Position::NONE), ); } // switch @@ -704,7 +708,8 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b if preserve_result { statements.push(Stmt::Noop(pos)) } - *stmt = Stmt::Block(statements.into_boxed_slice(), pos); + *stmt = + Stmt::Block(statements.into_boxed_slice(), (pos, Position::NONE)); } else { *stmt = Stmt::Noop(pos); }; @@ -721,7 +726,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b *stmt = Stmt::Block( optimize_stmt_block(mem::take(&mut **body), state, false, true, false) .into_boxed_slice(), - body.position(), + body.positions(), ); } // do { block } while|until expr @@ -749,7 +754,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b match block.as_mut_slice() { [] => { state.set_dirty(); - *stmt = Stmt::Noop(*pos); + *stmt = Stmt::Noop(pos.0); } // Only one statement which is not block-dependent - promote [s] if !s.is_block_dependent() => { @@ -766,7 +771,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b *stmt = Stmt::Block( optimize_stmt_block(mem::take(&mut *x.try_block), state, false, true, false) .into_boxed_slice(), - x.try_block.position(), + x.try_block.positions(), ); } // try { try_block } catch ( var ) { catch_block } @@ -1073,7 +1078,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) { if let Some(value) = arg.get_literal_value() { state.set_dirty(); constants.push(value); - *arg = Expr::Stack(constants.len()-1, arg.position()); + *arg = Expr::Stack(constants.len()-1, arg.start_position()); } }); } @@ -1121,7 +1126,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) { if let Some(value) = arg.get_literal_value() { state.set_dirty(); x.constants.push(value); - *arg = Expr::Stack(x.constants.len()-1, arg.position()); + *arg = Expr::Stack(x.constants.len()-1, arg.start_position()); } }, diff --git a/src/parser.rs b/src/parser.rs index 2efdccee..878e7ddd 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -306,7 +306,7 @@ impl Expr { Err( PERR::MismatchedType("a boolean expression".to_string(), type_name.to_string()) - .into_err(self.position()), + .into_err(self.start_position()), ) } /// Raise an error if the expression can never yield an iterable value. @@ -326,7 +326,7 @@ impl Expr { Err( PERR::MismatchedType("an iterable value".to_string(), type_name.to_string()) - .into_err(self.position()), + .into_err(self.start_position()), ) } } @@ -521,6 +521,7 @@ fn parse_fn_call( namespace, hashes, args, + pos: settings.pos, ..Default::default() } .into_fn_call_expr(settings.pos)); @@ -585,6 +586,7 @@ fn parse_fn_call( namespace, hashes, args, + pos: settings.pos, ..Default::default() } .into_fn_call_expr(settings.pos)); @@ -652,7 +654,7 @@ fn parse_index_chain( return Err(PERR::MalformedIndexExpr( "Only arrays, object maps and strings can be indexed".into(), ) - .into_err(lhs.position())) + .into_err(lhs.start_position())) } Expr::CharConstant(_, _) @@ -663,7 +665,7 @@ fn parse_index_chain( return Err(PERR::MalformedIndexExpr( "Only arrays, object maps and strings can be indexed".into(), ) - .into_err(lhs.position())) + .into_err(lhs.start_position())) } _ => (), @@ -677,7 +679,7 @@ fn parse_index_chain( return Err(PERR::MalformedIndexExpr( "Array or string expects numeric index, not a string".into(), ) - .into_err(idx_expr.position())) + .into_err(idx_expr.start_position())) } #[cfg(not(feature = "no_float"))] @@ -685,7 +687,7 @@ fn parse_index_chain( return Err(PERR::MalformedIndexExpr( "Only arrays, object maps and strings can be indexed".into(), ) - .into_err(lhs.position())) + .into_err(lhs.start_position())) } Expr::CharConstant(_, _) @@ -696,7 +698,7 @@ fn parse_index_chain( return Err(PERR::MalformedIndexExpr( "Only arrays, object maps and strings can be indexed".into(), ) - .into_err(lhs.position())) + .into_err(lhs.start_position())) } _ => (), @@ -708,35 +710,35 @@ fn parse_index_chain( return Err(PERR::MalformedIndexExpr( "Array access expects integer index, not a float".into(), ) - .into_err(x.position())) + .into_err(x.start_position())) } // lhs[char] x @ Expr::CharConstant(_, _) => { return Err(PERR::MalformedIndexExpr( "Array access expects integer index, not a character".into(), ) - .into_err(x.position())) + .into_err(x.start_position())) } // lhs[()] x @ Expr::Unit(_) => { return Err(PERR::MalformedIndexExpr( "Array access expects integer index, not ()".into(), ) - .into_err(x.position())) + .into_err(x.start_position())) } // lhs[??? && ???], lhs[??? || ???] x @ Expr::And(_, _) | x @ Expr::Or(_, _) => { return Err(PERR::MalformedIndexExpr( "Array access expects integer index, not a boolean".into(), ) - .into_err(x.position())) + .into_err(x.start_position())) } // lhs[true], lhs[false] x @ Expr::BoolConstant(_, _) => { return Err(PERR::MalformedIndexExpr( "Array access expects integer index, not a boolean".into(), ) - .into_err(x.position())) + .into_err(x.start_position())) } // All other expressions _ => (), @@ -1048,7 +1050,7 @@ fn parse_switch( let (hash, range) = if let Some(expr) = expr { let value = expr.get_literal_value().ok_or_else(|| { - PERR::ExprExpected("a literal".to_string()).into_err(expr.position()) + PERR::ExprExpected("a literal".to_string()).into_err(expr.start_position()) })?; let guard = value.read_lock::(); @@ -1058,14 +1060,14 @@ fn parse_switch( } else if let Some(range) = value.read_lock::() { (None, Some((*range.start(), *range.end(), true))) } else if value.is::() && !ranges.is_empty() { - return Err(PERR::WrongSwitchIntegerCase.into_err(expr.position())); + return Err(PERR::WrongSwitchIntegerCase.into_err(expr.start_position())); } else { let hasher = &mut get_hasher(); value.hash(hasher); let hash = hasher.finish(); if cases.contains_key(&hash) { - return Err(PERR::DuplicatedSwitchCase.into_err(expr.position())); + return Err(PERR::DuplicatedSwitchCase.into_err(expr.start_position())); } (Some(hash), None) } @@ -1682,6 +1684,7 @@ fn parse_unary( name: state.get_identifier("", "-"), hashes: FnCallHashes::from_native(calc_fn_hash("-", 1)), args, + pos, ..Default::default() } .into_fn_call_expr(pos)) @@ -1708,6 +1711,7 @@ fn parse_unary( name: state.get_identifier("", "+"), hashes: FnCallHashes::from_native(calc_fn_hash("+", 1)), args, + pos, ..Default::default() } .into_fn_call_expr(pos)) @@ -1725,6 +1729,7 @@ fn parse_unary( name: state.get_identifier("", "!"), hashes: FnCallHashes::from_native(calc_fn_hash("!", 1)), args, + pos, ..Default::default() } .into_fn_call_expr(pos)) @@ -1753,7 +1758,7 @@ fn make_assignment_stmt( } Expr::Property(_, _) => None, // Anything other than a property after dotting (e.g. a method call) is not an l-value - ref e => Some(e.position()), + ref e => Some(e.start_position()), }, Expr::Index(x, term, _) | Expr::Dot(x, term, _) => match x.lhs { Expr::Property(_, _) => unreachable!("unexpected Expr::Property in indexing"), @@ -1762,7 +1767,7 @@ fn make_assignment_stmt( }, Expr::Property(_, _) if parent_is_dot => None, Expr::Property(_, _) => unreachable!("unexpected Expr::Property in indexing"), - e if parent_is_dot => Some(e.position()), + e if parent_is_dot => Some(e.start_position()), _ => None, } } @@ -1772,7 +1777,7 @@ fn make_assignment_stmt( match lhs { // const_expr = rhs ref expr if expr.is_constant() => { - Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) + Err(PERR::AssignmentToConstant("".into()).into_err(lhs.start_position())) } // var (non-indexed) = rhs Expr::Variable(None, _, ref x) if x.0.is_none() => Ok(Stmt::Assignment( @@ -1814,10 +1819,8 @@ fn make_assignment_stmt( op_pos, )), // expr[???] = rhs, expr.??? = rhs - ref expr => { - Err(PERR::AssignmentToInvalidLHS("".to_string()) - .into_err(expr.position())) - } + ref expr => Err(PERR::AssignmentToInvalidLHS("".to_string()) + .into_err(expr.start_position())), } } Some(err_pos) => { @@ -1832,7 +1835,7 @@ fn make_assignment_stmt( ) .into_err(op_pos)), // expr = rhs - _ => Err(PERR::AssignmentToInvalidLHS("".to_string()).into_err(lhs.position())), + _ => Err(PERR::AssignmentToInvalidLHS("".to_string()).into_err(lhs.start_position())), } } @@ -1983,7 +1986,7 @@ fn make_dot_expr( Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), false, op_pos)) } // lhs.rhs - (_, rhs) => Err(PERR::PropertyExpected.into_err(rhs.position())), + (_, rhs) => Err(PERR::PropertyExpected.into_err(rhs.start_position())), } } @@ -2065,6 +2068,7 @@ fn parse_binary_op( let op_base = FnCallExpr { name: state.get_identifier("", op), hashes: FnCallHashes::from_native(hash), + pos, ..Default::default() }; @@ -2082,7 +2086,10 @@ fn parse_binary_op( | Token::LessThan | Token::LessThanEqualsTo | Token::GreaterThan - | Token::GreaterThanEqualsTo => FnCallExpr { args, ..op_base }.into_fn_call_expr(pos), + | Token::GreaterThanEqualsTo => { + let pos = args[0].start_position(); + FnCallExpr { args, ..op_base }.into_fn_call_expr(pos) + } Token::Or => { let rhs = args.pop().unwrap(); @@ -2111,6 +2118,7 @@ fn parse_binary_op( Token::In => { // Swap the arguments let current_lhs = args.remove(0); + let pos = current_lhs.start_position(); args.push(current_lhs); args.shrink_to_fit(); @@ -2132,6 +2140,7 @@ fn parse_binary_op( .map_or(false, Option::is_some) => { let hash = calc_fn_hash(&s, 2); + let pos = args[0].start_position(); FnCallExpr { hashes: if is_valid_function_name(&s) { @@ -2145,7 +2154,10 @@ fn parse_binary_op( .into_fn_call_expr(pos) } - _ => FnCallExpr { args, ..op_base }.into_fn_call_expr(pos), + _ => { + let pos = args[0].start_position(); + FnCallExpr { args, ..op_base }.into_fn_call_expr(pos) + } }; } } @@ -2734,13 +2746,10 @@ fn parse_block( #[cfg(not(feature = "no_module"))] let orig_imports_len = state.imports.len(); - loop { + let end_pos = loop { // Terminated? match input.peek().expect(NEVER_ENDS) { - (Token::RightBrace, _) => { - eat_token(input, Token::RightBrace); - break; - } + (Token::RightBrace, _) => break eat_token(input, Token::RightBrace), (Token::EOF, pos) => { return Err(PERR::MissingToken( Token::RightBrace.into(), @@ -2767,10 +2776,7 @@ fn parse_block( match input.peek().expect(NEVER_ENDS) { // { ... stmt } - (Token::RightBrace, _) => { - eat_token(input, Token::RightBrace); - break; - } + (Token::RightBrace, _) => break eat_token(input, Token::RightBrace), // { ... stmt; (Token::SemiColon, _) if need_semicolon => { eat_token(input, Token::SemiColon); @@ -2793,7 +2799,7 @@ fn parse_block( .into_err(*pos)); } } - } + }; state.stack.truncate(state.entry_stack_len); state.entry_stack_len = prev_entry_stack_len; @@ -2801,7 +2807,10 @@ fn parse_block( #[cfg(not(feature = "no_module"))] state.imports.truncate(orig_imports_len); - Ok(Stmt::Block(statements.into_boxed_slice(), settings.pos)) + Ok(Stmt::Block( + statements.into_boxed_slice(), + (settings.pos, end_pos), + )) } /// Parse an expression as a statement. @@ -3244,6 +3253,7 @@ fn make_curry_from_externals( num_externals + 1, )), args, + pos, ..Default::default() } .into_fn_call_expr(pos); @@ -3253,7 +3263,7 @@ fn make_curry_from_externals( let mut statements = StaticVec::with_capacity(externals.len() + 1); statements.extend(externals.into_iter().map(Stmt::Share)); statements.push(Stmt::Expr(expr)); - Expr::Stmt(crate::ast::StmtBlock::new(statements, pos).into()) + Expr::Stmt(crate::ast::StmtBlock::new(statements, pos, Position::NONE).into()) } /// Parse an anonymous function definition. From 3be27746e00a0374e33ba208d84361e85a8a62a8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 13:20:47 +0800 Subject: [PATCH 16/22] Add allow_shadowing. --- CHANGELOG.md | 1 + src/api/options.rs | 21 +++++++++++++++++---- src/eval/stmt.rs | 5 +++++ src/parser.rs | 20 +++++++++++++------- src/types/error.rs | 11 +++++++++-- src/types/parse_error.rs | 5 +++++ tests/options.rs | 16 +++++++++++++++- 7 files changed, 65 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f7a2997..fcc67f65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ New features * A debugging interface is added. * A new bin tool, `rhai-dbg` (aka _The Rhai Debugger_), is added to showcase the debugging interface. * A new package, `DebuggingPackage`, is added which contains the `back_trace` function to get the current call stack anywhere in a script. +* `Engine::set_allow_shadowing` is added to allow/disallow variables _shadowing_, with new errors `EvalAltResult::ErrorVariableExists` and `ParseErrorType::VariableExists`. Enhancements ------------ diff --git a/src/api/options.rs b/src/api/options.rs index a6e1f59b..4fe6a4d7 100644 --- a/src/api/options.rs +++ b/src/api/options.rs @@ -17,9 +17,11 @@ pub struct LanguageOptions { #[cfg(not(feature = "no_function"))] pub allow_anonymous_fn: bool, /// Is looping allowed? - pub allow_loop: bool, + pub allow_looping: bool, /// Strict variables mode? pub strict_var: bool, + /// Is variables shadowing allowed? + pub allow_shadowing: bool, } impl LanguageOptions { @@ -32,8 +34,9 @@ impl LanguageOptions { allow_stmt_expr: true, #[cfg(not(feature = "no_function"))] allow_anonymous_fn: true, - allow_loop: true, + allow_looping: true, strict_var: false, + allow_shadowing: true, } } } @@ -94,12 +97,12 @@ impl Engine { /// Is looping allowed? #[inline(always)] pub fn allow_looping(&self) -> bool { - self.options.allow_loop + self.options.allow_looping } /// Set whether looping is allowed. #[inline(always)] pub fn set_allow_looping(&mut self, enable: bool) { - self.options.allow_loop = enable; + self.options.allow_looping = enable; } /// Is strict variables mode enabled? #[inline(always)] @@ -111,4 +114,14 @@ impl Engine { pub fn set_strict_variables(&mut self, enable: bool) { self.options.strict_var = enable; } + /// Is variables shadowing allowed? + #[inline(always)] + pub fn allow_shadowing(&self) -> bool { + self.options.allow_shadowing + } + /// Set whether variables shadowing is allowed. + #[inline(always)] + pub fn set_allow_shadowing(&mut self, enable: bool) { + self.options.allow_shadowing = enable; + } } diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 8e1e7c5c..f37cd574 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -802,9 +802,14 @@ impl Engine { // Empty return Stmt::Return(_, None, pos) => Err(ERR::Return(Dynamic::UNIT, *pos).into()), + // Let/const statement - shadowing disallowed + Stmt::Var(_, x, _, pos) if !self.allow_shadowing() && scope.contains(&x.name) => { + Err(ERR::ErrorVariableExists(x.name.to_string(), *pos).into()) + } // Let/const statement Stmt::Var(expr, x, options, _) => { let var_name = &x.name; + let entry_type = if options.contains(AST_OPTION_CONSTANT) { AccessMode::ReadOnly } else { diff --git a/src/parser.rs b/src/parser.rs index 878e7ddd..85046218 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -16,7 +16,7 @@ use crate::types::dynamic::AccessMode; use crate::types::StringsInterner; use crate::{ calc_fn_hash, Dynamic, Engine, ExclusiveRange, Identifier, ImmutableString, InclusiveRange, - LexError, ParseError, Position, Scope, Shared, StaticVec, AST, INT, PERR, + LexError, ParseError, Position, Scope, Shared, StaticVec, Variant, AST, INT, PERR, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -2591,6 +2591,12 @@ fn parse_let( // let name ... let (name, pos) = parse_var_name(input)?; + if !settings.default_options.allow_shadowing + && state.stack.iter().any(|(v, _)| v == name.as_ref()) + { + return Err(PERR::VariableExists(name.to_string()).into_err(pos)); + } + let name = state.get_identifier("", name); let var_def = Ident { name: name.clone(), @@ -2973,25 +2979,25 @@ fn parse_stmt( Token::If => parse_if(input, state, lib, settings.level_up()), Token::Switch => parse_switch(input, state, lib, settings.level_up()), - Token::While | Token::Loop if settings.default_options.allow_loop => { + Token::While | Token::Loop if settings.default_options.allow_looping => { parse_while_loop(input, state, lib, settings.level_up()) } - Token::Do if settings.default_options.allow_loop => { + Token::Do if settings.default_options.allow_looping => { parse_do(input, state, lib, settings.level_up()) } - Token::For if settings.default_options.allow_loop => { + Token::For if settings.default_options.allow_looping => { parse_for(input, state, lib, settings.level_up()) } - Token::Continue if settings.default_options.allow_loop && settings.is_breakable => { + Token::Continue if settings.default_options.allow_looping && settings.is_breakable => { let pos = eat_token(input, Token::Continue); Ok(Stmt::BreakLoop(AST_OPTION_NONE, pos)) } - Token::Break if settings.default_options.allow_loop && settings.is_breakable => { + Token::Break if settings.default_options.allow_looping && settings.is_breakable => { let pos = eat_token(input, Token::Break); Ok(Stmt::BreakLoop(AST_OPTION_BREAK, pos)) } - Token::Continue | Token::Break if settings.default_options.allow_loop => { + Token::Continue | Token::Break if settings.default_options.allow_looping => { Err(PERR::LoopBreak.into_err(token_pos)) } diff --git a/src/types/error.rs b/src/types/error.rs index 8131b7b0..60fc5d41 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -30,6 +30,8 @@ pub enum EvalAltResult { /// Syntax error. ErrorParsing(ParseErrorType, Position), + /// Shadowing of an existing variable disallowed. Wrapped value is the variable name. + ErrorVariableExists(String, Position), /// Usage of an unknown variable. Wrapped value is the variable name. ErrorVariableNotFound(String, Position), /// Call to an unknown function. Wrapped value is the function signature. @@ -139,8 +141,9 @@ impl fmt::Display for EvalAltResult { } Self::ErrorInModule(s, err, _) => write!(f, "Error in module {}: {}", s, err)?, - Self::ErrorFunctionNotFound(s, _) => write!(f, "Function not found: {}", s)?, + Self::ErrorVariableExists(s, _) => write!(f, "Variable is already defined: {}", s)?, Self::ErrorVariableNotFound(s, _) => write!(f, "Variable not found: {}", s)?, + Self::ErrorFunctionNotFound(s, _) => write!(f, "Function not found: {}", s)?, Self::ErrorModuleNotFound(s, _) => write!(f, "Module not found: {}", s)?, Self::ErrorDataRace(s, _) => { write!(f, "Data race detected when accessing variable: {}", s)? @@ -274,6 +277,7 @@ impl EvalAltResult { | Self::ErrorBitFieldBounds(_, _, _) | Self::ErrorIndexingType(_, _) | Self::ErrorFor(_) + | Self::ErrorVariableExists(_, _) | Self::ErrorVariableNotFound(_, _) | Self::ErrorModuleNotFound(_, _) | Self::ErrorDataRace(_, _) @@ -364,7 +368,8 @@ impl EvalAltResult { Self::ErrorIndexingType(t, _) => { map.insert("type".into(), t.into()); } - Self::ErrorVariableNotFound(v, _) + Self::ErrorVariableExists(v, _) + | Self::ErrorVariableNotFound(v, _) | Self::ErrorDataRace(v, _) | Self::ErrorAssignmentToConstant(v, _) => { map.insert("variable".into(), v.into()); @@ -415,6 +420,7 @@ impl EvalAltResult { | Self::ErrorBitFieldBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) | Self::ErrorFor(pos) + | Self::ErrorVariableExists(_, pos) | Self::ErrorVariableNotFound(_, pos) | Self::ErrorModuleNotFound(_, pos) | Self::ErrorDataRace(_, pos) @@ -463,6 +469,7 @@ impl EvalAltResult { | Self::ErrorBitFieldBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) | Self::ErrorFor(pos) + | Self::ErrorVariableExists(_, pos) | Self::ErrorVariableNotFound(_, pos) | Self::ErrorModuleNotFound(_, pos) | Self::ErrorDataRace(_, pos) diff --git a/src/types/parse_error.rs b/src/types/parse_error.rs index 50ab93da..d416043b 100644 --- a/src/types/parse_error.rs +++ b/src/types/parse_error.rs @@ -167,6 +167,10 @@ pub enum ParseErrorType { /// Assignment to an inappropriate LHS (left-hand-side) expression. /// Wrapped value is the error message (if any). AssignmentToInvalidLHS(String), + /// A variable is already defined. + /// + /// Only appears when variables shadowing is disabled. + VariableExists(String), /// A variable is not found. /// /// Only appears when strict variables mode is enabled. @@ -241,6 +245,7 @@ impl fmt::Display for ParseErrorType { Self::DuplicatedSwitchCase => f.write_str("Duplicated switch case"), Self::DuplicatedVariable(s) => write!(f, "Duplicated variable name: {}", s), + Self::VariableExists(s) => write!(f, "Variable already defined: {}", s), Self::VariableUndefined(s) => write!(f, "Undefined variable: {}", s), Self::ModuleUndefined(s) => write!(f, "Undefined module: {}", s), diff --git a/tests/options.rs b/tests/options.rs index 336a2856..d7e45780 100644 --- a/tests/options.rs +++ b/tests/options.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult}; +use rhai::{Engine, EvalAltResult, Scope, INT}; #[test] fn test_options_allow() -> Result<(), Box> { @@ -31,6 +31,20 @@ fn test_options_allow() -> Result<(), Box> { assert!(engine.compile("while x > y { foo(z); }").is_err()); + engine.compile("let x = 42; let x = 123;")?; + + engine.set_allow_shadowing(false); + + assert!(engine.compile("let x = 42; let x = 123;").is_err()); + assert!(engine.compile("const x = 42; let x = 123;").is_err()); + assert!(engine.compile("let x = 42; const x = 123;").is_err()); + assert!(engine.compile("const x = 42; const x = 123;").is_err()); + + let mut scope = Scope::new(); + scope.push("x", 42 as INT); + + assert!(engine.run_with_scope(&mut scope, "let x = 42;").is_err()); + Ok(()) } From f09abd7ab31c9f7f6d945c478085ef830101de3a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 13:31:33 +0800 Subject: [PATCH 17/22] Fix builds. --- src/ast/ast.rs | 2 +- src/engine.rs | 5 ++--- src/parser.rs | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 6fb96e85..7077bad6 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -73,7 +73,7 @@ impl AST { ) -> Self { Self { source: Identifier::new_const(), - body: StmtBlock::new(statements, Position::NONE), + body: StmtBlock::new(statements, Position::NONE, Position::NONE), #[cfg(not(feature = "no_function"))] lib: functions.into(), #[cfg(not(feature = "no_module"))] diff --git a/src/engine.rs b/src/engine.rs index aa5a36d1..a08e13f4 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -4,10 +4,9 @@ use crate::api::custom_syntax::CustomSyntax; use crate::func::native::{OnDebugCallback, OnParseTokenCallback, OnPrintCallback, OnVarCallback}; use crate::packages::{Package, StandardPackage}; use crate::tokenizer::Token; -use crate::types::dynamic::{ Union}; +use crate::types::dynamic::Union; use crate::{ - Dynamic, Identifier, ImmutableString, Module, Position, RhaiResult, Shared, - StaticVec, + Dynamic, Identifier, ImmutableString, Module, Position, RhaiResult, Shared, StaticVec, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; diff --git a/src/parser.rs b/src/parser.rs index 85046218..ae8de862 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -16,7 +16,7 @@ use crate::types::dynamic::AccessMode; use crate::types::StringsInterner; use crate::{ calc_fn_hash, Dynamic, Engine, ExclusiveRange, Identifier, ImmutableString, InclusiveRange, - LexError, ParseError, Position, Scope, Shared, StaticVec, Variant, AST, INT, PERR, + LexError, ParseError, Position, Scope, Shared, StaticVec, AST, INT, PERR, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; From 5aee06674feb7b180a6180f2af0c85e1cfc7ae2f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 16:38:01 +0800 Subject: [PATCH 18/22] Fix no-std build. --- src/api/type_names.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/api/type_names.rs b/src/api/type_names.rs index 8e7162a2..faf1fa44 100644 --- a/src/api/type_names.rs +++ b/src/api/type_names.rs @@ -1,6 +1,5 @@ use crate::{ - Engine, ExclusiveRange, FnPtr, ImmutableString, InclusiveRange, Instant, Position, RhaiError, - ERR, + Engine, ExclusiveRange, FnPtr, ImmutableString, InclusiveRange, Position, RhaiError, ERR, }; use std::any::type_name; #[cfg(feature = "no_std")] @@ -45,7 +44,7 @@ fn map_std_type_name(name: &str, shorthands: bool) -> &str { return if shorthands { "map" } else { "Map" }; } #[cfg(not(feature = "no_std"))] - if name == type_name::() || name == "Instant" { + if name == type_name::() || name == "Instant" { return if shorthands { "timestamp" } else { "Instant" }; } if name == type_name::() || name == "ExclusiveRange" { From 936dc01e391dc8c80c0afcc439acf6e20b075b98 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 22:16:12 +0800 Subject: [PATCH 19/22] Pass level to var resolver. --- src/eval/chaining.rs | 2 +- src/eval/expr.rs | 12 ++++++++---- src/func/call.rs | 5 +++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index c681ce5a..a9a34adf 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -673,7 +673,7 @@ impl Engine { self.inc_operations(&mut global.num_operations, *var_pos)?; let (mut target, _) = - self.search_namespace(scope, global, state, lib, this_ptr, lhs)?; + self.search_namespace(scope, global, state, lib, this_ptr, lhs, level)?; let obj_ptr = &mut target; let root = (x.2.as_str(), *var_pos); diff --git a/src/eval/expr.rs b/src/eval/expr.rs index dcbb830b..9793bf0c 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -50,15 +50,18 @@ impl Engine { lib: &[&Module], this_ptr: &'s mut Option<&mut Dynamic>, expr: &Expr, + level: usize, ) -> RhaiResultOf<(Target<'s>, Position)> { match expr { Expr::Variable(Some(_), _, _) => { - self.search_scope_only(scope, global, state, lib, this_ptr, expr) + self.search_scope_only(scope, global, state, lib, this_ptr, expr, level) } Expr::Variable(None, _var_pos, v) => match v.as_ref() { // Normal variable access #[cfg(not(feature = "no_module"))] - (_, None, _) => self.search_scope_only(scope, global, state, lib, this_ptr, expr), + (_, None, _) => { + self.search_scope_only(scope, global, state, lib, this_ptr, expr, level) + } #[cfg(feature = "no_module")] (_, (), _) => self.search_scope_only(scope, global, state, lib, this_ptr, expr), @@ -136,6 +139,7 @@ impl Engine { lib: &[&Module], this_ptr: &'s mut Option<&mut Dynamic>, expr: &Expr, + level: usize, ) -> RhaiResultOf<(Target<'s>, Position)> { // Make sure that the pointer indirection is taken only when absolutely necessary. @@ -163,7 +167,7 @@ impl Engine { state, lib, this_ptr, - level: 0, + level, }; match resolve_var( expr.get_variable_name(true).expect("`Expr::Variable`"), @@ -297,7 +301,7 @@ impl Engine { .cloned() .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into()) } else { - self.search_namespace(scope, global, state, lib, this_ptr, expr) + self.search_namespace(scope, global, state, lib, this_ptr, expr, level) .map(|(val, _)| val.take_or_clone()) }; } diff --git a/src/func/call.rs b/src/func/call.rs index 292bd33f..e688178a 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -1230,7 +1230,7 @@ impl Engine { })?; let (mut target, _pos) = - self.search_namespace(scope, global, state, lib, this_ptr, first_expr)?; + self.search_namespace(scope, global, state, lib, this_ptr, first_expr, level)?; if target.as_ref().is_read_only() { target = target.into_owned(); @@ -1316,8 +1316,9 @@ impl Engine { })?; // Get target reference to first argument + let first_arg = &args_expr[0]; let (target, _pos) = - self.search_scope_only(scope, global, state, lib, this_ptr, &args_expr[0])?; + self.search_scope_only(scope, global, state, lib, this_ptr, first_arg, level)?; #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, _pos)?; From be9356727fc11bf3367f81263472f9329ee26f41 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 22:59:41 +0800 Subject: [PATCH 20/22] Add variable definition filter. --- CHANGELOG.md | 1 + src/api/events.rs | 63 ++++++++++++++++++++++++- src/engine.rs | 8 +++- src/eval/stmt.rs | 113 +++++++++++++++++++++++++++++---------------- src/func/native.rs | 8 ++++ tests/var_scope.rs | 23 +++++++++ 6 files changed, 173 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcc67f65..571ec17e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ New features * A new bin tool, `rhai-dbg` (aka _The Rhai Debugger_), is added to showcase the debugging interface. * A new package, `DebuggingPackage`, is added which contains the `back_trace` function to get the current call stack anywhere in a script. * `Engine::set_allow_shadowing` is added to allow/disallow variables _shadowing_, with new errors `EvalAltResult::ErrorVariableExists` and `ParseErrorType::VariableExists`. +* `Engine::on_def_var` allows registering a closure which can decide whether a variable definition is allow to continue, or should fail with an error. Enhancements ------------ diff --git a/src/api/events.rs b/src/api/events.rs index 53de16f5..0a7b5eaf 100644 --- a/src/api/events.rs +++ b/src/api/events.rs @@ -15,12 +15,12 @@ impl Engine { /// > `Fn(name: &str, index: usize, context: &EvalContext) -> Result, Box>` /// /// where: + /// * `name`: name of the variable. /// * `index`: an offset from the bottom of the current [`Scope`][crate::Scope] that the /// variable is supposed to reside. Offsets start from 1, with 1 meaning the last variable in /// the current [`Scope`][crate::Scope]. Essentially the correct variable is at position /// `scope.len() - index`. If `index` is zero, then there is no pre-calculated offset position /// and a search through the current [`Scope`][crate::Scope] must be performed. - /// /// * `context`: the current [evaluation context][`EvalContext`]. /// /// ## Return value @@ -63,6 +63,67 @@ impl Engine { self.resolve_var = Some(Box::new(callback)); self } + /// Provide a callback that will be invoked before the definition of each variable . + /// + /// # Callback Function Signature + /// + /// The callback function signature takes the following form: + /// + /// > `Fn(name: &str, is_const: bool, block_level: usize, will_shadow: bool, context: &EvalContext) -> Result>` + /// + /// where: + /// * `name`: name of the variable to be defined. + /// * `is_const`: `true` if the statement is `const`, otherwise it is `let`. + /// * `block_level`: the current nesting level of statement blocks, with zero being the global level + /// * `will_shadow`: will the variable _shadow_ an existing variable? + /// * `context`: the current [evaluation context][`EvalContext`]. + /// + /// ## Return value + /// + /// * `Ok(true)`: continue with normal variable definition. + /// * `Ok(false)`: deny the variable definition with an [runtime error][EvalAltResult::ErrorRuntime]. + /// + /// ## Raising errors + /// + /// Return `Err(...)` if there is an error. + /// + /// # Example + /// + /// ```should_panic + /// # fn main() -> Result<(), Box> { + /// use rhai::Engine; + /// + /// let mut engine = Engine::new(); + /// + /// // Register a variable definition filter. + /// engine.on_def_var(|name, is_const, _, _, _| { + /// // Disallow defining MYSTIC_NUMBER as a constant + /// if name == "MYSTIC_NUMBER" && is_const { + /// Ok(false) + /// } else { + /// Ok(true) + /// } + /// }); + /// + /// // The following runs fine: + /// engine.eval::("let MYSTIC_NUMBER = 42;")?; + /// + /// // The following will cause an error: + /// engine.eval::("const MYSTIC_NUMBER = 42;")?; + /// + /// # Ok(()) + /// # } + /// ``` + #[inline(always)] + pub fn on_def_var( + &mut self, + callback: impl Fn(&str, bool, usize, bool, &EvalContext) -> RhaiResultOf + + SendSync + + 'static, + ) -> &mut Self { + self.def_var_filter = Some(Box::new(callback)); + self + } /// _(internals)_ Register a callback that will be invoked during parsing to remap certain tokens. /// Exported under the `internals` feature only. /// diff --git a/src/engine.rs b/src/engine.rs index a08e13f4..86fb6859 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,7 +1,9 @@ //! Main module defining the script evaluation [`Engine`]. use crate::api::custom_syntax::CustomSyntax; -use crate::func::native::{OnDebugCallback, OnParseTokenCallback, OnPrintCallback, OnVarCallback}; +use crate::func::native::{ + OnDebugCallback, OnDefVarCallback, OnParseTokenCallback, OnPrintCallback, OnVarCallback, +}; use crate::packages::{Package, StandardPackage}; use crate::tokenizer::Token; use crate::types::dynamic::Union; @@ -113,6 +115,8 @@ pub struct Engine { pub(crate) custom_keywords: BTreeMap>, /// Custom syntax. pub(crate) custom_syntax: BTreeMap>, + /// Callback closure for filtering variable definition. + pub(crate) def_var_filter: Option>, /// Callback closure for resolving variable access. pub(crate) resolve_var: Option>, /// Callback closure to remap tokens during parsing. @@ -160,6 +164,7 @@ impl fmt::Debug for Engine { .field("disabled_symbols", &self.disabled_symbols) .field("custom_keywords", &self.custom_keywords) .field("custom_syntax", &(!self.custom_syntax.is_empty())) + .field("def_var_filter", &self.def_var_filter.is_some()) .field("resolve_var", &self.resolve_var.is_some()) .field("token_mapper", &self.token_mapper.is_some()) .field("print", &self.print.is_some()) @@ -272,6 +277,7 @@ impl Engine { custom_keywords: BTreeMap::new(), custom_syntax: BTreeMap::new(), + def_var_filter: None, resolve_var: None, token_mapper: None, diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index f37cd574..1c4a06dc 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -1,6 +1,6 @@ //! Module defining functions for evaluating a statement. -use super::{EvalState, GlobalRuntimeState, Target}; +use super::{EvalContext, EvalState, GlobalRuntimeState, Target}; use crate::ast::{ BinaryExpr, Expr, Ident, OpAssignment, Stmt, SwitchCases, TryCatchBlock, AST_OPTION_FLAGS::*, }; @@ -240,7 +240,7 @@ impl Engine { if let Ok(rhs_val) = rhs_result { let search_result = - self.search_namespace(scope, global, state, lib, this_ptr, lhs); + self.search_namespace(scope, global, state, lib, this_ptr, lhs, level); if let Ok(search_val) = search_result { let (mut lhs_ptr, pos) = search_val; @@ -807,7 +807,7 @@ impl Engine { Err(ERR::ErrorVariableExists(x.name.to_string(), *pos).into()) } // Let/const statement - Stmt::Var(expr, x, options, _) => { + Stmt::Var(expr, x, options, pos) => { let var_name = &x.name; let entry_type = if options.contains(AST_OPTION_CONSTANT) { @@ -817,48 +817,79 @@ impl Engine { }; let export = options.contains(AST_OPTION_EXPORTED); - let value_result = self - .eval_expr(scope, global, state, lib, this_ptr, expr, level) - .map(Dynamic::flatten); - - if let Ok(value) = value_result { - let _alias = if !rewind_scope { - #[cfg(not(feature = "no_function"))] - #[cfg(not(feature = "no_module"))] - if state.scope_level == 0 - && entry_type == AccessMode::ReadOnly - && lib.iter().any(|&m| !m.is_empty()) - { - if global.constants.is_none() { - global.constants = Some(crate::Shared::new(crate::Locked::new( - std::collections::BTreeMap::new(), - ))); - } - crate::func::locked_write(global.constants.as_ref().unwrap()) - .insert(var_name.clone(), value.clone()); - } - - if export { - Some(var_name) - } else { - None - } - } else if export { - unreachable!("exported variable not on global level"); - } else { - None + let result = if let Some(ref filter) = self.def_var_filter { + let shadowing = scope.contains(var_name); + let scope_level = state.scope_level; + let is_const = entry_type == AccessMode::ReadOnly; + let context = EvalContext { + engine: self, + scope, + global, + state, + lib, + this_ptr, + level: level, }; - scope.push_dynamic_value(var_name.clone(), entry_type, value); - - #[cfg(not(feature = "no_module"))] - if let Some(alias) = _alias { - scope.add_entry_alias(scope.len() - 1, alias.clone()); + match filter(var_name, is_const, scope_level, shadowing, &context) { + Ok(true) => None, + Ok(false) => Some(Err(ERR::ErrorRuntime( + format!("Variable cannot be defined: {}", var_name).into(), + *pos, + ) + .into())), + err @ Err(_) => Some(err), } - - Ok(Dynamic::UNIT) } else { - value_result + None + }; + + if let Some(result) = result { + result.map(|_| Dynamic::UNIT) + } else { + let value_result = self + .eval_expr(scope, global, state, lib, this_ptr, expr, level) + .map(Dynamic::flatten); + + if let Ok(value) = value_result { + let _alias = if !rewind_scope { + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_module"))] + if state.scope_level == 0 + && entry_type == AccessMode::ReadOnly + && lib.iter().any(|&m| !m.is_empty()) + { + if global.constants.is_none() { + global.constants = Some(crate::Shared::new( + crate::Locked::new(std::collections::BTreeMap::new()), + )); + } + crate::func::locked_write(global.constants.as_ref().unwrap()) + .insert(var_name.clone(), value.clone()); + } + + if export { + Some(var_name) + } else { + None + } + } else if export { + unreachable!("exported variable not on global level"); + } else { + None + }; + + scope.push_dynamic_value(var_name.clone(), entry_type, value); + + #[cfg(not(feature = "no_module"))] + if let Some(alias) = _alias { + scope.add_entry_alias(scope.len() - 1, alias.clone()); + } + + Ok(Dynamic::UNIT) + } else { + value_result + } } } diff --git a/src/func/native.rs b/src/func/native.rs index 43de5bcb..380098fd 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -444,3 +444,11 @@ pub type OnVarCallback = dyn Fn(&str, usize, &EvalContext) -> RhaiResultOf RhaiResultOf> + Send + Sync; + +/// Callback function for variable definition. +#[cfg(not(feature = "sync"))] +pub type OnDefVarCallback = dyn Fn(&str, bool, usize, bool, &EvalContext) -> RhaiResultOf; +/// Callback function for variable definition. +#[cfg(feature = "sync")] +pub type OnDefVarCallback = + dyn Fn(&str, bool, usize, bool, &EvalContext) -> RhaiResultOf + Send + Sync; diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 8efbc5f4..7948833b 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -120,3 +120,26 @@ fn test_var_resolver() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_var_def_filter() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine.on_def_var(|name, scope_level, _, _| match (name, scope_level) { + ("x", 0 | 1) => Ok(false), + _ => Ok(true), + }); + + assert_eq!( + engine.eval::("let y = 42; let y = 123; let z = y + 1; z")?, + 124 + ); + + assert!(engine.run("let x = 42;").is_err()); + assert!(engine.run("const x = 42;").is_err()); + assert!(engine.run("let y = 42; { let x = y + 1; }").is_err()); + assert!(engine.run("let y = 42; { let x = y + 1; }").is_err()); + engine.run("let y = 42; { let z = y + 1; { let x = z + 1; } }")?; + + Ok(()) +} From 40bec9f017c36c853525dd3da89e49666041f1b7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 23:02:00 +0800 Subject: [PATCH 21/22] Fix tests. --- tests/var_scope.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 7948833b..b8d4824d 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -125,7 +125,7 @@ fn test_var_resolver() -> Result<(), Box> { fn test_var_def_filter() -> Result<(), Box> { let mut engine = Engine::new(); - engine.on_def_var(|name, scope_level, _, _| match (name, scope_level) { + engine.on_def_var(|name, _, scope_level, _, _| match (name, scope_level) { ("x", 0 | 1) => Ok(false), _ => Ok(true), }); From 6a740a9fa10b00403dbc4baf8498d4b88ea57e17 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 4 Feb 2022 23:08:09 +0800 Subject: [PATCH 22/22] Fix no_module build. --- src/eval/expr.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 9793bf0c..b7e89972 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -63,7 +63,9 @@ impl Engine { self.search_scope_only(scope, global, state, lib, this_ptr, expr, level) } #[cfg(feature = "no_module")] - (_, (), _) => self.search_scope_only(scope, global, state, lib, this_ptr, expr), + (_, (), _) => { + self.search_scope_only(scope, global, state, lib, this_ptr, expr, level) + } // Qualified variable access #[cfg(not(feature = "no_module"))]