From 5ac83f0f4640d85f4119b50602dfb7efd39d7278 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 21 Dec 2020 22:04:46 +0800 Subject: [PATCH] Add context source to on_debug. --- RELEASES.md | 2 +- doc/src/language/print-debug.md | 29 ++++++++++++-- src/ast.rs | 44 ++++++++++++++++++++- src/engine.rs | 22 +++++++---- src/engine_api.rs | 34 +++++++++------- src/fn_call.rs | 69 +++++++++++++++++++++------------ src/fn_native.rs | 4 +- src/module/mod.rs | 58 +++++++++++++++++++++++++-- src/module/resolvers/file.rs | 3 +- src/packages/fn_basic.rs | 4 +- tests/optimizer.rs | 9 +++-- tests/print.rs | 22 +++++++---- 12 files changed, 232 insertions(+), 68 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index c82038ee..012706d0 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -27,7 +27,7 @@ Breaking changes ---------------- * `Engine::on_progress` now takes `u64` instead of `&u64`. -* The closure for `Engine::on_debug` now takes an additional `Position` parameter. +* The closure for `Engine::on_debug` now takes two additional parameters: `source: Option<&str>` and `pos: Position`. * `AST::iter_functions` now returns `ScriptFnMetadata`. * The parser function passed to `Engine::register_custom_syntax_raw` now takes an additional parameter containing the _look-ahead_ symbol. diff --git a/doc/src/language/print-debug.md b/doc/src/language/print-debug.md index 606452d6..996e6aac 100644 --- a/doc/src/language/print-debug.md +++ b/doc/src/language/print-debug.md @@ -27,7 +27,7 @@ engine.on_print(|x| println!("hello: {}", x)); // Any function or closure that takes a '&str' and a 'Position' argument can be used to // override 'debug'. -engine.on_debug(|x, pos| println!("DEBUG at {:?}: {}", pos, x)); +engine.on_debug(|x, src, pos| println!("DEBUG of {} at {:?}: {}", src.unwrap_or("unknown"), pos, x)); // Example: quick-'n-dirty logging let logbook = Arc::new(RwLock::new(Vec::::new())); @@ -37,9 +37,9 @@ let log = logbook.clone(); engine.on_print(move |s| log.write().unwrap().push(format!("entry: {}", s))); let log = logbook.clone(); -engine.on_debug(move |s, pos| log.write().unwrap().push( - format!("DEBUG at {:?}: {}", pos, s) - )); +engine.on_debug(move |s, src, pos| log.write().unwrap().push( + format!("DEBUG of {} at {:?}: {}", src.unwrap_or("unknown"), pos, s) + )); // Evaluate script engine.eval::<()>(script)?; @@ -49,3 +49,24 @@ for entry in logbook.read().unwrap().iter() { println!("{}", entry); } ``` + + +`on_debug` Callback Signature +----------------------------- + +The function signature passed to `Engine::on_debug` takes the following form: + +> `Fn(text: &str, source: Option<&str>, pos: Position) + 'static` + +where: + +| Parameter | Type | Description | +| --------- | :------------: | --------------------------------------------------------------- | +| `text` | `&str` | text to display | +| `source` | `Option<&str>` | source of the current evaluation, if any | +| `pos` | `Position` | position (line number and character offset) of the `debug` call | + +The _source_ of a script evaluation is any text string provided to an [`AST`] via the `AST::set_source` method. + +If a [module] is loaded via an [`import`] statement, then the _source_ of functions defined +within the module will be the module's _path_. diff --git a/src/ast.rs b/src/ast.rs index 926cf3a3..5b0262a1 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -163,6 +163,8 @@ impl<'a> Into> for &'a ScriptFnDef { /// Currently, [`AST`] is neither `Send` nor `Sync`. Turn on the `sync` feature to make it `Send + Sync`. #[derive(Debug, Clone)] pub struct AST { + /// Source of the [`AST`]. + source: Option, /// Global statements. statements: Vec, /// Script-defined functions. @@ -172,6 +174,7 @@ pub struct AST { impl Default for AST { fn default() -> Self { Self { + source: None, statements: Vec::with_capacity(16), functions: Default::default(), } @@ -186,10 +189,36 @@ impl AST { functions: impl Into>, ) -> Self { Self { + source: None, statements: statements.into_iter().collect(), functions: functions.into(), } } + /// Create a new [`AST`] with a source name. + #[inline(always)] + pub fn new_with_source( + statements: impl IntoIterator, + functions: impl Into>, + source: impl Into, + ) -> Self { + Self { + source: Some(source.into()), + statements: statements.into_iter().collect(), + functions: functions.into(), + } + } + /// Get the source. + pub fn source(&self) -> Option<&str> { + self.source.as_ref().map(|s| s.as_str()) + } + /// Clone the source. + pub(crate) fn clone_source(&self) -> Option { + self.source.clone() + } + /// Set the source. + pub fn set_source>(&mut self, source: Option) { + self.source = source.map(|s| s.into()) + } /// Get the statements. #[cfg(not(feature = "internals"))] #[inline(always)] @@ -253,6 +282,7 @@ impl AST { let mut functions: Module = Default::default(); functions.merge_filtered(&self.functions, &mut filter); Self { + source: self.source.clone(), statements: Default::default(), functions: functions.into(), } @@ -262,6 +292,7 @@ impl AST { #[inline(always)] pub fn clone_statements_only(&self) -> Self { Self { + source: self.source.clone(), statements: self.statements.clone(), functions: Default::default(), } @@ -432,6 +463,7 @@ impl AST { let Self { statements, functions, + .. } = self; let ast = match (statements.is_empty(), other.statements.is_empty()) { @@ -445,10 +477,20 @@ impl AST { (true, true) => vec![], }; + let source = if other.source.is_some() { + other.source.clone() + } else { + self.source.clone() + }; + let mut functions = functions.as_ref().clone(); functions.merge_filtered(&other.functions, &mut filter); - Self::new(ast, functions) + if let Some(source) = source { + Self::new_with_source(ast, functions, source) + } else { + Self::new(ast, functions) + } } /// Combine one [`AST`] with another. The second [`AST`] is consumed. /// diff --git a/src/engine.rs b/src/engine.rs index 202a0402..17937537 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -100,19 +100,21 @@ impl Imports { } /// Get an iterator to this stack of imported [modules][Module] in reverse order. #[allow(dead_code)] - pub fn iter<'a>(&'a self) -> impl Iterator)> + 'a { + pub fn iter<'a>(&'a self) -> impl Iterator + 'a { self.0.iter().flat_map(|lib| { lib.iter() .rev() - .map(|(name, module)| (name.clone(), module.clone())) + .map(|(name, module)| (name.as_str(), module.as_ref())) }) } /// Get an iterator to this stack of imported [modules][Module] in reverse order. #[allow(dead_code)] pub(crate) fn iter_raw<'a>( &'a self, - ) -> impl Iterator)> + 'a { - self.0.iter().flat_map(|lib| lib.iter().rev().cloned()) + ) -> impl Iterator)> + 'a { + self.0 + .iter() + .flat_map(|lib| lib.iter().rev().map(|(n, m)| (n, m))) } /// Get a consuming iterator to this stack of imported [modules][Module] in reverse order. pub fn into_iter(self) -> impl Iterator)> { @@ -478,6 +480,8 @@ impl> From for Target<'_> { /// This type is volatile and may change. #[derive(Debug, Clone, Default)] pub struct State { + /// Source of the current context. + pub source: Option, /// Normally, access to variables are parsed with a relative offset into the scope to avoid a lookup. /// In some situation, e.g. after running an `eval` statement, subsequent offsets become mis-aligned. /// When that happens, this flag is turned on to force a scope lookup by name. @@ -708,10 +712,14 @@ fn default_print(_s: &str) { /// Debug to stdout #[inline(always)] -fn default_debug(_s: &str, _pos: Position) { +fn default_debug(_s: &str, _source: Option<&str>, _pos: Position) { #[cfg(not(feature = "no_std"))] #[cfg(not(target_arch = "wasm32"))] - println!("{:?} | {}", _pos, _s); + if let Some(source) = _source { + println!("{} @ {:?} | {}", source, _pos, _s); + } else { + println!("{:?} | {}", _pos, _s); + } } /// Search for a module within an imports stack. @@ -828,7 +836,7 @@ impl Engine { resolve_var: None, print: Box::new(|_| {}), - debug: Box::new(|_, _| {}), + debug: Box::new(|_, _, _| {}), progress: None, optimization_level: if cfg!(feature = "no_optimize") { diff --git a/src/engine_api.rs b/src/engine_api.rs index a71fb34e..9f72a5df 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1,7 +1,7 @@ //! Module that defines the extern API of [`Engine`]. use crate::dynamic::Variant; -use crate::engine::{EvalContext, Imports}; +use crate::engine::{EvalContext, Imports, State}; use crate::fn_native::{FnCallArgs, SendSync}; use crate::optimize::OptimizationLevel; use crate::stdlib::{ @@ -1368,9 +1368,9 @@ impl Engine { scope: &mut Scope, ast: &AST, ) -> Result> { - let mut mods = self.global_sub_modules.clone(); + let mods = &mut self.global_sub_modules.clone(); - let result = self.eval_ast_with_scope_raw(scope, &mut mods, ast)?; + let result = self.eval_ast_with_scope_raw(scope, mods, ast)?; let typ = self.map_type_name(result.type_name()); @@ -1391,7 +1391,10 @@ impl Engine { mods: &mut Imports, ast: &'a AST, ) -> Result> { - let state = &mut Default::default(); + let state = &mut State { + source: ast.clone_source(), + ..Default::default() + }; self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()]) } /// Evaluate a file, but throw away the result and only return error (if any). @@ -1451,9 +1454,12 @@ impl Engine { scope: &mut Scope, ast: &AST, ) -> Result<(), Box> { - let mut mods = self.global_sub_modules.clone(); - let mut state = Default::default(); - self.eval_statements_raw(scope, &mut mods, &mut state, ast.statements(), &[ast.lib()])?; + let mods = &mut self.global_sub_modules.clone(); + let state = &mut State { + source: ast.clone_source(), + ..Default::default() + }; + self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()])?; Ok(()) } /// Call a script function defined in an [`AST`] with multiple arguments. @@ -1809,20 +1815,22 @@ impl Engine { /// /// // Override action of 'print' function /// let logger = result.clone(); - /// engine.on_debug(move |s, pos| logger.write().unwrap().push_str( - /// &format!("{:?} > {}", pos, s) - /// )); + /// engine.on_debug(move |s, src, pos| logger.write().unwrap().push_str( + /// &format!("{} @ {:?} > {}", src.unwrap_or("unknown"), pos, s) + /// )); /// - /// engine.consume(r#"let x = "hello"; debug(x);"#)?; + /// let mut ast = engine.compile(r#"let x = "hello"; debug(x);"#)?; + /// ast.set_source(Some("world")); + /// engine.consume_ast(&ast)?; /// - /// assert_eq!(*result.read().unwrap(), r#"1:18 > "hello""#); + /// assert_eq!(*result.read().unwrap(), r#"world @ 1:18 > "hello""#); /// # Ok(()) /// # } /// ``` #[inline(always)] pub fn on_debug( &mut self, - callback: impl Fn(&str, Position) + SendSync + 'static, + callback: impl Fn(&str, Option<&str>, Position) + SendSync + 'static, ) -> &mut Self { self.debug = Box::new(callback); self diff --git a/src/fn_call.rs b/src/fn_call.rs index 0adf5d2d..70258f03 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -221,20 +221,17 @@ impl Engine { .into(), false, ), - KEYWORD_DEBUG => ( - (self.debug)( - result.as_str().map_err(|typ| { - EvalAltResult::ErrorMismatchOutputType( - self.map_type_name(type_name::()).into(), - typ.into(), - pos, - ) - })?, - pos, - ) - .into(), - false, - ), + KEYWORD_DEBUG => { + let text = result.as_str().map_err(|typ| { + EvalAltResult::ErrorMismatchOutputType( + self.map_type_name(type_name::()).into(), + typ.into(), + pos, + ) + })?; + let source = state.source.as_ref().map(|s| s.as_str()); + ((self.debug)(text, source, pos).into(), false) + } _ => (result, func.is_method()), }); } @@ -394,7 +391,7 @@ impl Engine { #[cfg(not(feature = "no_module"))] if !fn_def.mods.is_empty() { - mods.extend(fn_def.mods.iter_raw()); + mods.extend(fn_def.mods.iter_raw().map(|(n, m)| (n.clone(), m.clone()))); } // Evaluate the function at one higher level of call depth @@ -531,14 +528,16 @@ impl Engine { // Script-like function found #[cfg(not(feature = "no_function"))] - _ if self.has_override(Some(mods), lib, 0, hash_script, pub_only) => { + _ if hash_script != 0 + && self.has_override(Some(mods), lib, 0, hash_script, pub_only) => + { // Get function - let func = lib + let (func, mut source) = lib .iter() - .find_map(|&m| m.get_fn(hash_script, pub_only)) + .find_map(|&m| m.get_fn(hash_script, pub_only).map(|f| (f, m.clone_id()))) //.or_else(|| self.global_namespace.get_fn(hash_script, pub_only)) - .or_else(|| self.packages.get_fn(hash_script)) - //.or_else(|| mods.get_fn(hash_script)) + .or_else(|| self.packages.get_fn(hash_script).map(|f| (f, None))) + //.or_else(|| mods.iter().find_map(|(_, m)| m.get_qualified_fn(hash_script).map(|f| (f, m.clone_id())))) .unwrap(); if func.is_script() { @@ -563,7 +562,10 @@ impl Engine { let result = if _is_method { // Method call of script function - map first argument to `this` let (first, rest) = args.split_first_mut().unwrap(); - self.call_script_fn( + + mem::swap(&mut state.source, &mut source); + + let result = self.call_script_fn( scope, mods, state, @@ -573,17 +575,27 @@ impl Engine { rest, pos, _level, - )? + ); + + // Restore the original source + state.source = source; + + result? } else { // Normal call of script function // The first argument is a reference? let mut backup: ArgBackup = Default::default(); backup.change_first_arg_to_copy(is_ref, args); + mem::swap(&mut state.source, &mut source); + let result = self.call_script_fn( scope, mods, state, lib, &mut None, func, args, pos, _level, ); + // Restore the original source + state.source = source; + // Restore the original reference backup.restore_first_arg(args); @@ -678,6 +690,7 @@ impl Engine { // Evaluate the AST let mut new_state = State { + source: state.source.clone(), operations: state.operations, ..Default::default() }; @@ -1162,9 +1175,17 @@ impl Engine { let args = args.as_mut(); let new_scope = &mut Default::default(); let fn_def = f.get_fn_def().clone(); - self.call_script_fn( + + let mut source = module.clone_id(); + mem::swap(&mut state.source, &mut source); + + let result = self.call_script_fn( new_scope, mods, state, lib, &mut None, &fn_def, args, pos, level, - ) + ); + + state.source = source; + + result } Some(f) if f.is_plugin_fn() => f .get_plugin_fn() diff --git a/src/fn_native.rs b/src/fn_native.rs index c9c82300..6cbe7542 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -360,10 +360,10 @@ pub type OnPrintCallback = Box; /// A standard callback function for debugging. #[cfg(not(feature = "sync"))] -pub type OnDebugCallback = Box; +pub type OnDebugCallback = Box, Position) + 'static>; /// A standard callback function for debugging. #[cfg(feature = "sync")] -pub type OnDebugCallback = Box; +pub type OnDebugCallback = Box, Position) + Send + Sync + 'static>; /// A standard callback function for variable access. #[cfg(not(feature = "sync"))] diff --git a/src/module/mod.rs b/src/module/mod.rs index 6db9889d..e94d3297 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -119,6 +119,8 @@ impl FuncInfo { /// Not available under the `no_module` feature. #[derive(Clone)] pub struct Module { + /// ID identifying the module. + id: Option, /// Sub-modules. modules: HashMap>, /// Module variables. @@ -141,6 +143,7 @@ pub struct Module { impl Default for Module { fn default() -> Self { Self { + id: None, modules: Default::default(), variables: Default::default(), all_variables: Default::default(), @@ -157,7 +160,12 @@ impl fmt::Debug for Module { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "Module(\n modules: {}\n vars: {}\n functions: {}\n)", + "Module({}\n modules: {}\n vars: {}\n functions: {}\n)", + if let Some(ref id) = self.id { + format!("id: {:?}", id) + } else { + "".to_string() + }, self.modules .keys() .map(|m| m.as_str()) @@ -220,6 +228,41 @@ impl Module { } } + /// Get the ID of the module, if any. + /// + /// # Example + /// + /// ``` + /// use rhai::Module; + /// + /// let mut module = Module::new(); + /// module.set_id(Some("hello")); + /// assert_eq!(module.id(), Some("hello")); + /// ``` + pub fn id(&self) -> Option<&str> { + self.id.as_ref().map(|s| s.as_str()) + } + + /// Get the ID of the module, if any. + pub(crate) fn clone_id(&self) -> Option { + self.id.clone() + } + + /// Set the ID of the module. + /// + /// # Example + /// + /// ``` + /// use rhai::Module; + /// + /// let mut module = Module::new(); + /// module.set_id(Some("hello")); + /// assert_eq!(module.id(), Some("hello")); + /// ``` + pub fn set_id>(&mut self, id: Option) { + self.id = id.map(|s| s.into()); + } + /// Is the module empty? /// /// # Example @@ -1338,7 +1381,11 @@ impl Module { /// the hash calculated by [`build_index`][Module::build_index]. #[inline] pub fn contains_qualified_fn(&self, hash_fn: u64) -> bool { - self.all_functions.contains_key(&hash_fn) + if hash_fn == 0 { + false + } else { + self.all_functions.contains_key(&hash_fn) + } } /// Get a namespace-qualified function. @@ -1348,7 +1395,11 @@ impl Module { /// the hash calculated by [`build_index`][Module::build_index]. #[inline(always)] pub(crate) fn get_qualified_fn(&self, hash_qualified_fn: u64) -> Option<&CallableFunction> { - self.all_functions.get(&hash_qualified_fn) + if hash_qualified_fn == 0 { + None + } else { + self.all_functions.get(&hash_qualified_fn) + } } /// Combine another module into this module. @@ -1679,6 +1730,7 @@ impl Module { }); } + module.set_id(ast.clone_source()); module.build_index(); Ok(module) diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index 3c509e96..0831ff25 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -163,10 +163,11 @@ impl ModuleResolver for FileModuleResolver { _ => Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)), })?; - let m = Module::eval_ast_as_new(scope, &ast, engine).map_err(|err| { + let mut m = Module::eval_ast_as_new(scope, &ast, engine).map_err(|err| { Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) })?; + m.set_id(Some(path)); module = Some(m.into()); module_ref = module.clone(); }; diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs index e4592c33..4b241f55 100644 --- a/src/packages/fn_basic.rs +++ b/src/packages/fn_basic.rs @@ -128,8 +128,8 @@ fn collect_fn_metadata(ctx: NativeCallContext) -> Array { .for_each(|(_, _, _, _, f)| list.push(make_metadata(&dict, None, f).into())); if let Some(mods) = ctx.mods { - mods.iter() - .for_each(|(ns, m)| scan_module(&mut list, &dict, ns, m.as_ref())); + mods.iter_raw() + .for_each(|(ns, m)| scan_module(&mut list, &dict, ns.clone(), m.as_ref())); } list diff --git a/tests/optimizer.rs b/tests/optimizer.rs index bffc2633..b6468d03 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -55,17 +55,20 @@ fn test_optimizer_parse() -> Result<(), Box> { let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert!(format!("{:?}", ast).starts_with(r#"AST { statements: [Block([Const(IdentX { name: "DECISION", pos: 1:9 }, Some(Unit(0:0)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); + assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, statements: [Block([Const(IdentX { name: "DECISION", pos: 1:9 }, Some(Unit(0:0)), false, 1:3), Expr(IntegerConstant(123, 1:53))], 1:1)]"#)); let ast = engine.compile("if 1 == 2 { 42 }")?; - assert!(format!("{:?}", ast).starts_with("AST { statements: [], functions: Module(")); + assert!( + format!("{:?}", ast).starts_with("AST { source: None, statements: [], functions: Module(") + ); engine.set_optimization_level(OptimizationLevel::Full); let ast = engine.compile("abs(-42)")?; - assert!(format!("{:?}", ast).starts_with(r"AST { statements: [Expr(IntegerConstant(42, 1:1))]")); + assert!(format!("{:?}", ast) + .starts_with(r"AST { source: None, statements: [Expr(IntegerConstant(42, 1:1))]")); Ok(()) } diff --git a/tests/print.rs b/tests/print.rs index 57471323..c61cae49 100644 --- a/tests/print.rs +++ b/tests/print.rs @@ -13,20 +13,28 @@ fn test_print_debug() -> Result<(), Box> { engine .on_print(move |s| log1.write().unwrap().push(format!("entry: {}", s))) - .on_debug(move |s, pos| { - log2.write() - .unwrap() - .push(format!("DEBUG at {:?}: {}", pos, s)) + .on_debug(move |s, src, pos| { + log2.write().unwrap().push(format!( + "DEBUG of {} at {:?}: {}", + src.unwrap_or("unknown"), + pos, + s + )) }); // Evaluate script - engine.eval::<()>("print(40 + 2)")?; - engine.eval::<()>(r#"let x = "hello!"; debug(x)"#)?; + engine.consume("print(40 + 2)")?; + let mut ast = engine.compile(r#"let x = "hello!"; debug(x)"#)?; + ast.set_source(Some("world")); + engine.consume_ast(&ast)?; // 'logbook' captures all the 'print' and 'debug' output assert_eq!(logbook.read().unwrap().len(), 2); assert_eq!(logbook.read().unwrap()[0], "entry: 42"); - assert_eq!(logbook.read().unwrap()[1], r#"DEBUG at 1:19: "hello!""#); + assert_eq!( + logbook.read().unwrap()[1], + r#"DEBUG of world at 1:19: "hello!""# + ); for entry in logbook.read().unwrap().iter() { println!("{}", entry);