From a13fcc5cc2ac1075953a7cdd692416a72cb2b7a1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 1 Oct 2020 16:47:02 +0800 Subject: [PATCH 01/29] Refine plugin module docs. --- doc/src/plugins/module.md | 108 ++++++++++++++++++++++++++++++++++---- 1 file changed, 98 insertions(+), 10 deletions(-) diff --git a/doc/src/plugins/module.md b/doc/src/plugins/module.md index a083fb08..87a11087 100644 --- a/doc/src/plugins/module.md +++ b/doc/src/plugins/module.md @@ -15,8 +15,8 @@ use rhai::plugin::*; ``` -`#[export_module]` and `exported_module!` ----------------------------------------- +`#[export_module]` +------------------ When applied to a Rust module, the `#[export_module]` attribute generates the necessary code and metadata to allow Rhai access to its public (i.e. marked `pub`) functions, constants @@ -25,12 +25,17 @@ and sub-modules. This code is exactly what would need to be written by hand to achieve the same goal, and is custom fit to each exported item. -This Rust module can then either be loaded into an [`Engine`] as a normal [module] or -registered as a [custom package]. This is done by using the `exported_module!` macro. - All `pub` functions become registered functions, all `pub` constants become [module] constant variables, and all sub-modules become Rhai sub-modules. +This Rust module can then either be loaded into an [`Engine`] as a normal [module] or +registered as a [package]. This is done by using the `exported_module!` macro. + +The macro `combine_with_exported_module!` can also be used to _combine_ all the functions +and variables into an existing module, _flattening_ the namespace - i.e. all sub-modules +are eliminated and their contents promoted to the top level. This is typical for +developing [custom packages]. + ```rust use rhai::plugin::*; // a "prelude" import for macros @@ -57,7 +62,7 @@ mod my_module { 42 } - // This sub-module is ignored when loaded as a package. + // Sub-modules are ignored when the Module is loaded as a package. pub mod my_sub_module { // This function is ignored when loaded as a package. // Otherwise it is a valid registered function under a sub-module. @@ -65,16 +70,32 @@ mod my_module { "hello".to_string() } } + + // Sub-modules are commonly used to put feature gates on a group of + // functions because feature gates cannot be put on function definitions. + // This is currently a limitation of the plugin procedural macros. + #[cfg(feature = "advanced_functions")] + pub mod advanced { + // This function is ignored when loaded as a package. + // Otherwise it is a valid registered function under a sub-module + // which only exists when the 'advanced_functions' feature is used. + pub fn advanced_calc(input: i64) -> i64 { + input * 2 + } + } } ``` -The simplest way to load this into an [`Engine`] is to use the `load_package` method on the exported module: +### Use `Engine::load_package` + +The simplest way to load this into an [`Engine`] is to first use the `exported_module!` macro +to turn it into a normal Rhai [module], then use the `Engine::load_package` method on it: ```rust fn main() { let mut engine = Engine::new(); - // The macro call creates the Rhai module. + // The macro call creates a Rhai module from the plugin module. let module = exported_module!(my_module); // A module can simply be loaded, registering all public functions. @@ -102,10 +123,77 @@ x == 43; Notice that, when using a [module] as a [package], only functions registered at the _top level_ can be accessed. Variables as well as sub-modules are ignored. -Using this directly as a Rhai module is almost the same, except that a [module resolver] must -be used to serve the module, and the module is loaded via `import` statements. +### Use as loadable `Module` + +Using this directly as a dynamically-loadable Rhai [module] is almost the same, except that a +[module resolver] must be used to serve the module, and the module is loaded via `import` statements. + See the [module] section for more information. +### Use as custom package + +Finally the plugin module can also be used to develop a [custom package], +using `combine_with_exported_module!`: + +```rust +def_package!(rhai:MyPackage:"My own personal super package", module, { + combine_with_exported_module!(module, "my_module_ID", my_module)); +}); +``` + +`combine_with_exported_module!` automatically _flattens_ the module namespace so that all +functions in sub-modules are promoted to the top level. This is convenient for [custom packages]. + + +Sub-Modules and Feature Gates +---------------------------- + +Sub-modules in a plugin module definition are turned into valid sub-modules in the resultant +Rhai `Module`. + +They are also commonly used to put _feature gates_ or _compile-time gates_ on a group of functions, +because currently attributes do not work on individual function definitions due to a limitation of +the procedural macros system. + +This is especially convenient when using the `combine_with_exported_module!` macro to develop +[custom packages] because selected groups of functions can easily be included or excluded based on +different combinations of feature flags instead of having to manually include/exclude every +single function. + +```rust +#[export_module] +mod my_module { + // Always available + pub fn func0() {} + + // The following sub-module is only available under 'feature1' + #[cfg(feature = "feature1")] + pub mod feature1 { + fn func1() {} + fn func2() {} + fn func3() {} + } + + // The following sub-module is only available under 'feature2' + #[cfg(feature = "feature2")] + pub mod feature2 { + fn func4() {} + fn func5() {} + fn func6() {} + } +} + +// Registered functions: +// func0 - always available +// func1 - available under 'feature1' +// func2 - available under 'feature1' +// func3 - available under 'feature1' +// func4 - available under 'feature2' +// func5 - available under 'feature2' +// func6 - available under 'feature2' +combine_with_exported_module!(module, "my_module_ID", my_module); +``` + Function Overloading and Operators --------------------------------- From e8d5f78f88847185fa94b0b4ccee3216cdf52f0a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 1 Oct 2020 23:31:27 +0800 Subject: [PATCH 02/29] Simplify code. --- RELEASES.md | 9 +++ codegen/src/function.rs | 4 +- codegen/src/module.rs | 4 +- src/module/mod.rs | 151 +++++++++++++++++++--------------------- src/parser.rs | 25 +++++-- 5 files changed, 104 insertions(+), 89 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 3503baf7..cdfbe815 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,6 +1,15 @@ Rhai Release Notes ================== +Version 0.20.0 +============== + +Breaking changes +---------------- + +* `AST::iter_functions` and `Module::iter_script_fn_info` now return an iterator instead of taking a closure. + + Version 0.19.0 ============== diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 173bb0b2..210e41df 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -403,9 +403,9 @@ impl ExportedFn { pub(crate) fn exported_name<'n>(&'n self) -> Cow<'n, str> { if let Some(ref name) = self.params.name { - Cow::Borrowed(name.last().unwrap().as_str()) + name.last().unwrap().as_str().into() } else { - Cow::Owned(self.signature.ident.to_string()) + self.signature.ident.to_string().into() } } diff --git a/codegen/src/module.rs b/codegen/src/module.rs index 85b1278c..eb3497bf 100644 --- a/codegen/src/module.rs +++ b/codegen/src/module.rs @@ -204,9 +204,9 @@ impl Module { pub fn exported_name(&self) -> Option> { if let Some(ref s) = self.params.name { - Some(Cow::Borrowed(s)) + Some(s.into()) } else { - self.module_name().map(|m| Cow::Owned(m.to_string())) + self.module_name().map(|m| m.to_string().into()) } } diff --git a/src/module/mod.rs b/src/module/mod.rs index 0a24de99..83a5ca24 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -5,7 +5,7 @@ use crate::calc_fn_hash; use crate::engine::Engine; use crate::fn_native::{CallableFunction, FnCallArgs, IteratorFn, SendSync}; use crate::fn_register::by_value as cast_arg; -use crate::parser::{FnAccess, FnAccess::Public}; +use crate::parser::FnAccess; use crate::result::EvalAltResult; use crate::token::{Position, Token}; use crate::utils::{ImmutableString, StaticVec, StraightHasherBuilder}; @@ -85,7 +85,7 @@ impl fmt::Debug for Module { "Module(\n modules: {}\n vars: {}\n functions: {}\n)", self.modules .keys() - .map(|k| k.as_str()) + .map(String::as_str) .collect::>() .join(", "), self.variables @@ -382,10 +382,7 @@ impl Module { } else if public_only { self.functions .get(&hash_fn) - .map(|(_, access, _, _, _)| match access { - FnAccess::Public => true, - FnAccess::Private => false, - }) + .map(|(_, access, _, _, _)| access.is_public()) .unwrap_or(false) } else { self.functions.contains_key(&hash_fn) @@ -506,7 +503,7 @@ impl Module { }; self.set_fn( name, - Public, + FnAccess::Public, arg_types, CallableFunction::from_method(Box::new(f)), ) @@ -562,7 +559,7 @@ impl Module { let arg_types = []; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_pure(Box::new(f)), ) @@ -592,7 +589,7 @@ impl Module { let arg_types = [TypeId::of::()]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_pure(Box::new(f)), ) @@ -622,7 +619,7 @@ impl Module { let arg_types = [TypeId::of::()]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_method(Box::new(f)), ) @@ -679,7 +676,7 @@ impl Module { let arg_types = [TypeId::of::(), TypeId::of::()]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_pure(Box::new(f)), ) @@ -715,7 +712,7 @@ impl Module { let arg_types = [TypeId::of::(), TypeId::of::()]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_method(Box::new(f)), ) @@ -825,7 +822,7 @@ impl Module { let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_pure(Box::new(f)), ) @@ -867,7 +864,7 @@ impl Module { let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_method(Box::new(f)), ) @@ -924,7 +921,7 @@ impl Module { let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; self.set_fn( FN_IDX_SET, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_method(Box::new(f)), ) @@ -1012,7 +1009,7 @@ impl Module { ]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_pure(Box::new(f)), ) @@ -1061,7 +1058,7 @@ impl Module { ]; self.set_fn( name, - Public, + FnAccess::Public, &arg_types, CallableFunction::from_method(Box::new(f)), ) @@ -1152,11 +1149,11 @@ impl Module { ) -> &mut Self { #[cfg(not(feature = "no_function"))] if !other.modules.is_empty() { - for (k, v) in &other.modules { + other.modules.iter().for_each(|(k, v)| { let mut m = Self::new(); m.merge_filtered(v, _filter); self.modules.insert(k.clone(), m); - } + }); } #[cfg(feature = "no_function")] if !other.modules.is_empty() { @@ -1244,13 +1241,14 @@ impl Module { } #[cfg(not(feature = "no_function"))] - pub fn iter_script_fn_info(&self, mut action: impl FnMut(FnAccess, &str, usize)) { + pub fn iter_script_fn_info(&self) -> impl Iterator { self.functions - .iter() - .for_each(|(_, (_, _, _, _, v))| match v { - CallableFunction::Script(f) => action(f.access, f.name.as_str(), f.params.len()), - _ => (), - }); + .values() + .filter(|(_, _, _, _, v)| v.is_script()) + .map(|(_, _, _, _, v)| { + let f = v.get_fn_def(); + (f.access, f.name.as_str(), f.params.len()) + }) } /// Create a new `Module` by evaluating an `AST`. @@ -1310,9 +1308,9 @@ impl Module { #[cfg(not(feature = "no_function"))] if merge_namespaces { - ast.iter_functions(|access, name, num_args| match access { - FnAccess::Private => (), - FnAccess::Public => { + ast.iter_functions() + .filter(|(access, _, _)| access.is_public()) + .for_each(|(_, name, num_args)| { let fn_name = name.to_string(); let ast_lib = ast.lib().clone(); @@ -1349,8 +1347,7 @@ impl Module { }) }, ); - } - }); + }); } else { module.merge(ast.lib()); } @@ -1369,71 +1366,63 @@ impl Module { variables: &mut Vec<(u64, Dynamic)>, functions: &mut Vec<(u64, CallableFunction)>, ) { - for (name, m) in &module.modules { + module.modules.iter().for_each(|(name, m)| { // Index all the sub-modules first. qualifiers.push(name); index_module(m, qualifiers, variables, functions); qualifiers.pop(); - } + }); // Index all variables - for (var_name, value) in &module.variables { + module.variables.iter().for_each(|(var_name, value)| { // Qualifiers + variable name let hash_var = calc_fn_hash(qualifiers.iter().map(|&v| v), var_name, 0, empty()); variables.push((hash_var, value.clone())); - } + }); // Index all Rust functions - for (&_hash, (name, access, _num_args, params, func)) in module.functions.iter() { - match access { - // Private functions are not exported - FnAccess::Private => continue, - FnAccess::Public => (), - } + module + .functions + .iter() + .filter(|(_, (_, access, _, _, _))| access.is_public()) + .for_each(|(&_hash, (name, _, _num_args, params, func))| { + if let Some(params) = params { + // Qualified Rust functions are indexed in two steps: + // 1) Calculate a hash in a similar manner to script-defined functions, + // i.e. qualifiers + function name + number of arguments. + let hash_qualified_script = + calc_fn_hash(qualifiers.iter().cloned(), name, params.len(), empty()); + // 2) Calculate a second hash with no qualifiers, empty function name, + // zero number of arguments, and the actual list of argument `TypeId`'.s + let hash_fn_args = calc_fn_hash(empty(), "", 0, params.iter().cloned()); + // 3) The final hash is the XOR of the two hashes. + let hash_qualified_fn = hash_qualified_script ^ hash_fn_args; - #[cfg(not(feature = "no_function"))] - if params.is_none() { - let hash_qualified_script = if qualifiers.is_empty() { - _hash - } else { - // Qualifiers + function name + number of arguments. - calc_fn_hash(qualifiers.iter().map(|&v| v), &name, *_num_args, empty()) - }; - functions.push((hash_qualified_script, func.clone())); - continue; - } - - if let Some(params) = params { - // Qualified Rust functions are indexed in two steps: - // 1) Calculate a hash in a similar manner to script-defined functions, - // i.e. qualifiers + function name + number of arguments. - let hash_qualified_script = - calc_fn_hash(qualifiers.iter().map(|&v| v), name, params.len(), empty()); - // 2) Calculate a second hash with no qualifiers, empty function name, - // zero number of arguments, and the actual list of argument `TypeId`'.s - let hash_fn_args = calc_fn_hash(empty(), "", 0, params.iter().cloned()); - // 3) The final hash is the XOR of the two hashes. - let hash_qualified_fn = hash_qualified_script ^ hash_fn_args; - - functions.push((hash_qualified_fn, func.clone())); - } - } + functions.push((hash_qualified_fn, func.clone())); + } else if cfg!(not(feature = "no_function")) { + let hash_qualified_script = if qualifiers.is_empty() { + _hash + } else { + // Qualifiers + function name + number of arguments. + calc_fn_hash(qualifiers.iter().map(|&v| v), &name, *_num_args, empty()) + }; + functions.push((hash_qualified_script, func.clone())); + } + }); } - if self.indexed { - return; + if !self.indexed { + let mut qualifiers: Vec<_> = Default::default(); + let mut variables: Vec<_> = Default::default(); + let mut functions: Vec<_> = Default::default(); + + qualifiers.push("root"); + + index_module(self, &mut qualifiers, &mut variables, &mut functions); + + self.all_variables = variables.into_iter().collect(); + self.all_functions = functions.into_iter().collect(); + self.indexed = true; } - - let mut qualifiers: Vec<_> = Default::default(); - let mut variables: Vec<_> = Default::default(); - let mut functions: Vec<_> = Default::default(); - - qualifiers.push("root"); - - index_module(self, &mut qualifiers, &mut variables, &mut functions); - - self.all_variables = variables.into_iter().collect(); - self.all_functions = functions.into_iter().collect(); - self.indexed = true; } /// Does a type iterator exist in the module? diff --git a/src/parser.rs b/src/parser.rs index 98de18fb..356429eb 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -301,8 +301,8 @@ impl AST { /// Iterate through all functions #[cfg(not(feature = "no_function"))] - pub fn iter_functions(&self, action: impl FnMut(FnAccess, &str, usize)) { - self.1.iter_script_fn_info(action); + pub fn iter_functions(&self) -> impl Iterator { + self.1.iter_script_fn_info() } /// Clear all function definitions in the `AST`. @@ -340,10 +340,10 @@ impl AsRef for AST { /// A type representing the access mode of a scripted function. #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub enum FnAccess { - /// Private function. - Private, /// Public function. Public, + /// Private function. + Private, } impl fmt::Display for FnAccess { @@ -355,6 +355,23 @@ impl fmt::Display for FnAccess { } } +impl FnAccess { + /// Is this access mode private? + pub fn is_private(self) -> bool { + match self { + Self::Public => false, + Self::Private => true, + } + } + /// Is this access mode public? + pub fn is_public(self) -> bool { + match self { + Self::Public => true, + Self::Private => false, + } + } +} + /// [INTERNALS] A type containing information on a scripted function. /// Exported under the `internals` feature only. /// From d2c94ba07c1b058faaa5c1554014ff7b8ca77f32 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 2 Oct 2020 14:55:02 +0800 Subject: [PATCH 03/29] Add more module tests. --- benches/eval_module.rs | 87 +++++++++++++++++++++++++++++++++++++ doc/src/rust/modules/ast.md | 10 ++++- src/fn_register.rs | 2 +- src/module/mod.rs | 8 +++- tests/modules.rs | 37 ++++++++++++++++ 5 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 benches/eval_module.rs diff --git a/benches/eval_module.rs b/benches/eval_module.rs new file mode 100644 index 00000000..98843eeb --- /dev/null +++ b/benches/eval_module.rs @@ -0,0 +1,87 @@ +#![feature(test)] + +///! Test evaluating with scope +extern crate test; + +use rhai::{module_resolvers::StaticModuleResolver, Engine, Module, OptimizationLevel}; +use test::Bencher; + +#[bench] +fn bench_eval_module_merged(bench: &mut Bencher) { + let script = r#" + fn foo(x) { x + 1 } + fn bar(x) { foo(x) } + "#; + + let mut engine = Engine::new(); + engine.set_optimization_level(OptimizationLevel::None); + + let ast = engine.compile(script).unwrap(); + + let module = Module::eval_ast_as_new(Default::default(), &ast, true, &engine).unwrap(); + + let mut resolver = StaticModuleResolver::new(); + resolver.insert("testing", module); + engine.set_module_resolver(Some(resolver)); + + let ast = engine + .compile( + r#" + fn foo(x) { x - 1 } + import "testing" as t; + t::bar(41) + "#, + ) + .unwrap(); + + bench.iter(|| engine.consume_ast(&ast).unwrap()); +} + +#[bench] +fn bench_eval_module_unmerged(bench: &mut Bencher) { + let script = r#" + fn foo(x) { x + 1 } + fn bar(x) { foo(x) } + "#; + + let mut engine = Engine::new(); + engine.set_optimization_level(OptimizationLevel::None); + + let ast = engine.compile(script).unwrap(); + + let module = Module::eval_ast_as_new(Default::default(), &ast, false, &engine).unwrap(); + + let mut resolver = StaticModuleResolver::new(); + resolver.insert("testing", module); + engine.set_module_resolver(Some(resolver)); + + let ast = engine + .compile( + r#" + fn foo(x) { x - 1 } + import "testing" as t; + t::bar(41) + "#, + ) + .unwrap(); + + bench.iter(|| engine.consume_ast(&ast).unwrap()); +} + +#[bench] +fn bench_eval_function_call(bench: &mut Bencher) { + let mut engine = Engine::new(); + engine.set_optimization_level(OptimizationLevel::None); + + let ast = engine + .compile( + r#" + fn foo(x) { x - 1 } + fn bar(x) { foo(x) } + bar(41) + "#, + ) + .unwrap(); + + bench.iter(|| engine.consume_ast(&ast).unwrap()); +} diff --git a/doc/src/rust/modules/ast.md b/doc/src/rust/modules/ast.md index 0348579a..fed0cb09 100644 --- a/doc/src/rust/modules/ast.md +++ b/doc/src/rust/modules/ast.md @@ -27,9 +27,13 @@ functions exposed by the module and the namespace that they can access: | `merge_namespaces` value | Description | Namespace | Performance | Call global functions | Call functions in same module | | :----------------------: | ------------------------------------------------ | :-----------------: | :---------: | :-------------------: | :---------------------------: | -| `true` | encapsulate entire `AST` into each function call | module, then global | slower | yes | yes | +| `true` | encapsulate entire `AST` into each function call | module, then global | 2x slower | yes | yes | | `false` | register each function independently | global only | fast | yes | no | +If the ultimate intention is to load the [module] directly into an [`Engine`] via `Engine::load_package`, +set `merge_namespaces` to `false` because there will not be any _module_ namespace as `Engine::load_package` +flattens everything into the _global_ namespace anyway. + Examples -------- @@ -78,7 +82,9 @@ let ast = engine.compile(r#" // a copy of the entire 'AST' into each function, allowing functions in the module script // to cross-call each other. // -// This incurs additional overhead, avoidable by setting 'merge_namespaces' to false. +// This incurs additional overhead, avoidable by setting 'merge_namespaces' to false +// which makes function calls 2x faster but at the expense of not being able to cross-call +// functions in the same module script. let module = Module::eval_ast_as_new(Scope::new(), &ast, true, &engine)?; // 'module' now contains: diff --git a/src/fn_register.rs b/src/fn_register.rs index 5c519d4c..3df06d5e 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -178,7 +178,7 @@ macro_rules! def_register { (imp $abi:ident : $($par:ident => $arg:expr => $mark:ty => $param:ty => $let:stmt => $clone:expr),*) => { // ^ function ABI type // ^ function parameter generic type name (A, B, C etc.) -// ^ call argument(like A, *B, &mut C etc) + // ^ call argument(like A, *B, &mut C etc) // ^ function parameter marker type (T, Ref or Mut) // ^ function parameter actual type (T, &T or &mut T) // ^ argument let statement diff --git a/src/module/mod.rs b/src/module/mod.rs index 83a5ca24..3981ead4 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1260,8 +1260,12 @@ impl Module { /// defined in the module, are _merged_ into a _unified_ namespace before each call. /// Therefore, all functions will be found, at the expense of some performance. /// - /// * If `false`, each function is registered independently and cannot cross-call - /// each other. Functions are searched in the global namespace. + /// * If `false`, each function is registered independently and cannot cross-call each other. + /// Functions are searched in the global namespace. + /// This is roughly 2x faster than the `true` case. + /// + /// Set `merge_namespaces` to `false` if the ultimate intention is to load the module + /// via `Engine::load_package` because it does not create any module namespace. /// /// # Examples /// diff --git a/tests/modules.rs b/tests/modules.rs index e1a8bae6..f14a8276 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -361,3 +361,40 @@ fn test_module_str() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_module_ast_namespace() -> Result<(), Box> { + let script = r#" + fn foo(x) { x + 1 } + fn bar(x) { foo(x) } + "#; + + let mut engine = Engine::new(); + + let ast = engine.compile(script)?; + + let module = Module::eval_ast_as_new(Default::default(), &ast, true, &engine)?; + + let mut resolver = StaticModuleResolver::new(); + resolver.insert("testing", module); + engine.set_module_resolver(Some(resolver)); + + assert_eq!( + engine.eval::(r#"import "testing" as t; t::foo(41)"#)?, + 42 + ); + assert_eq!( + engine.eval::(r#"import "testing" as t; t::bar(41)"#)?, + 42 + ); + assert_eq!( + engine.eval::(r#"fn foo(x) { x - 1 } import "testing" as t; t::foo(41)"#)?, + 42 + ); + assert_eq!( + engine.eval::(r#"fn foo(x) { x - 1 } import "testing" as t; t::bar(41)"#)?, + 42 + ); + + Ok(()) +} From 2fd30321a888e839cd4432aee4a9eea254bbbf72 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 2 Oct 2020 15:31:59 +0800 Subject: [PATCH 04/29] Fix test. --- tests/modules.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/modules.rs b/tests/modules.rs index f14a8276..db82d600 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -362,6 +362,7 @@ fn test_module_str() -> Result<(), Box> { Ok(()) } +#[cfg(not(feature = "no_function"))] #[test] fn test_module_ast_namespace() -> Result<(), Box> { let script = r#" From 038b058e635e03375827d3dfac5b1e5608795bc6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 2 Oct 2020 16:21:18 +0800 Subject: [PATCH 05/29] Use shared AST module. --- src/module/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/module/mod.rs b/src/module/mod.rs index 3981ead4..f9adfcb6 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1312,11 +1312,13 @@ impl Module { #[cfg(not(feature = "no_function"))] if merge_namespaces { + let ast_lib: Shared = ast.lib().clone().into(); + ast.iter_functions() .filter(|(access, _, _)| access.is_public()) .for_each(|(_, name, num_args)| { let fn_name = name.to_string(); - let ast_lib = ast.lib().clone(); + let ast_lib = ast_lib.clone(); module.set_raw_fn_as_scripted( name, From 08ca90a1368656421d5eac440bc27634aa7280d9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 2 Oct 2020 18:52:18 +0800 Subject: [PATCH 06/29] Use ImmutableString for import alias. --- src/engine.rs | 9 ++++++--- src/parser.rs | 4 ++-- src/syntax.rs | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index cceda886..0bde53c6 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -36,7 +36,6 @@ use crate::utils::ImmutableString; use crate::any::DynamicWriteLock; use crate::stdlib::{ - borrow::Cow, boxed::Box, collections::{HashMap, HashSet}, fmt, format, @@ -70,7 +69,11 @@ pub type Map = HashMap; /// ## WARNING /// /// This type is volatile and may change. -pub type Imports<'a> = Vec<(Cow<'a, str>, Module)>; +// +// Note - We cannot use &str or Cow here because `eval` may load a module +// and the module name will live beyond the AST of the eval script text. +// The best we can do is a shared reference. +pub type Imports = Vec<(ImmutableString, Module)>; #[cfg(not(feature = "unchecked"))] #[cfg(debug_assertions)] @@ -1826,7 +1829,7 @@ impl Engine { if let Some((name, _)) = alias { module.index_all_sub_modules(); - mods.push((name.clone().into(), module)); + mods.push((name.clone(), module)); } state.modules += 1; diff --git a/src/parser.rs b/src/parser.rs index 356429eb..4618f041 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -592,7 +592,7 @@ pub enum Stmt { ReturnWithVal(Box<((ReturnType, Position), Option, Position)>), /// import expr as module #[cfg(not(feature = "no_module"))] - Import(Box<(Expr, Option<(String, Position)>, Position)>), + Import(Box<(Expr, Option<(ImmutableString, Position)>, Position)>), /// expr id as name, ... #[cfg(not(feature = "no_module"))] Export( @@ -2753,7 +2753,7 @@ fn parse_import( Ok(Stmt::Import(Box::new(( expr, - Some((name, settings.pos)), + Some((name.into(), settings.pos)), token_pos, )))) } diff --git a/src/syntax.rs b/src/syntax.rs index cf9c9519..9f3f7f36 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -74,8 +74,8 @@ impl fmt::Debug for CustomSyntax { /// Context of a script evaluation process. #[derive(Debug)] -pub struct EvalContext<'a, 'b: 'a, 's, 'm, 't, 'd: 't> { - pub(crate) mods: &'a mut Imports<'b>, +pub struct EvalContext<'a, 's, 'm, 't, 'd: 't> { + pub(crate) mods: &'a mut Imports, pub(crate) state: &'s mut State, pub(crate) lib: &'m Module, pub(crate) this_ptr: &'t mut Option<&'d mut Dynamic>, From a72f70846f3ae709369095570820ee9e05870c07 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 2 Oct 2020 23:14:33 +0800 Subject: [PATCH 07/29] Make merged namespace more efficient. --- RELEASES.md | 3 +- src/api.rs | 8 ++---- src/engine.rs | 21 -------------- src/fn_native.rs | 4 +-- src/module/mod.rs | 71 ++++++++++++++++++++++++++++++----------------- src/parser.rs | 6 ++-- 6 files changed, 57 insertions(+), 56 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index cdfbe815..b8a45ed8 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -7,7 +7,8 @@ Version 0.20.0 Breaking changes ---------------- -* `AST::iter_functions` and `Module::iter_script_fn_info` now return an iterator instead of taking a closure. +* `AST::iter_functions` now returns an iterator instead of taking a closure. +* `Module::iter_script_fn_info` is removed and merged into `Module::iter_script_fn`. Version 0.19.0 diff --git a/src/api.rs b/src/api.rs index 0d5aa25d..b24b3b95 100644 --- a/src/api.rs +++ b/src/api.rs @@ -28,10 +28,7 @@ use crate::{ use crate::fn_register::{RegisterFn, RegisterResultFn}; #[cfg(not(feature = "no_function"))] -use crate::{ - engine::get_script_function_by_signature, fn_args::FuncArgs, fn_call::ensure_no_data_race, - utils::StaticVec, -}; +use crate::{fn_args::FuncArgs, fn_call::ensure_no_data_race, utils::StaticVec}; #[cfg(not(feature = "no_optimize"))] use crate::optimize::optimize_into_ast; @@ -1598,7 +1595,8 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, args: &mut [&mut Dynamic], ) -> FuncReturn { - let fn_def = get_script_function_by_signature(lib, name, args.len(), true) + let fn_def = lib + .get_script_function_by_signature(name, args.len(), true) .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), Position::none()))?; let mut state = State::new(); diff --git a/src/engine.rs b/src/engine.rs index 0bde53c6..29c742af 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -18,9 +18,6 @@ use crate::utils::StaticVec; #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] use crate::any::Variant; -#[cfg(not(feature = "no_function"))] -use crate::parser::ScriptFnDef; - #[cfg(not(feature = "no_module"))] use crate::module::ModuleResolver; @@ -318,24 +315,6 @@ impl State { } } -/// Get a script-defined function definition from a module. -#[cfg(not(feature = "no_function"))] -pub fn get_script_function_by_signature<'a>( - module: &'a Module, - name: &str, - params: usize, - pub_only: bool, -) -> Option<&'a ScriptFnDef> { - // Qualifiers (none) + function name + number of arguments. - let hash_script = calc_fn_hash(empty(), name, params, empty()); - let func = module.get_fn(hash_script, pub_only)?; - if func.is_script() { - Some(func.get_fn_def()) - } else { - None - } -} - /// [INTERNALS] A type containing all the limits imposed by the `Engine`. /// Exported under the `internals` feature only. /// diff --git a/src/fn_native.rs b/src/fn_native.rs index cf2ae263..df9e3948 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -351,10 +351,10 @@ impl CallableFunction { /// /// Panics if the `CallableFunction` is not `Script`. #[cfg(not(feature = "no_function"))] - pub fn get_shared_fn_def(&self) -> Shared { + pub fn get_shared_fn_def(&self) -> &Shared { match self { Self::Pure(_) | Self::Method(_) | Self::Iterator(_) | Self::Plugin(_) => unreachable!(), - Self::Script(f) => f.clone(), + Self::Script(f) => f, } } /// Get a reference to a script-defined function definition. diff --git a/src/module/mod.rs b/src/module/mod.rs index f9adfcb6..11153dae 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -516,19 +516,19 @@ impl Module { pub(crate) fn set_raw_fn_as_scripted( &mut self, name: impl Into, - num_args: usize, + num_params: usize, func: impl Fn(&Engine, &Module, &mut [&mut Dynamic]) -> FuncReturn + SendSync + 'static, ) -> u64 { // None + function name + number of arguments. let name = name.into(); - let hash_script = calc_fn_hash(empty(), &name, num_args, empty()); + let hash_script = calc_fn_hash(empty(), &name, num_params, empty()); let f = move |engine: &Engine, lib: &Module, args: &mut FnCallArgs| func(engine, lib, args); self.functions.insert( hash_script, ( name, FnAccess::Public, - num_args, + num_params, None, CallableFunction::from_pure(Box::new(f)), ), @@ -1082,6 +1082,24 @@ impl Module { } } + /// Get a script-defined function definition from a module. + #[cfg(not(feature = "no_function"))] + pub fn get_script_function_by_signature( + &self, + name: &str, + num_params: usize, + pub_only: bool, + ) -> Option<&ScriptFnDef> { + // Qualifiers (none) + function name + number of arguments. + let hash_script = calc_fn_hash(empty(), name, num_params, empty()); + let func = self.get_fn(hash_script, pub_only)?; + if func.is_script() { + Some(func.get_fn_def()) + } else { + None + } + } + /// Get a modules-qualified function. /// Name and Position in `EvalAltResult` are None and must be set afterwards. /// @@ -1232,22 +1250,17 @@ impl Module { /// Get an iterator over all script-defined functions in the module. #[cfg(not(feature = "no_function"))] - pub fn iter_script_fn<'a>(&'a self) -> impl Iterator> + 'a { + pub fn iter_script_fn<'a>( + &'a self, + ) -> impl Iterator)> + 'a { self.functions .values() .map(|(_, _, _, _, f)| f) .filter(|f| f.is_script()) - .map(|f| f.get_shared_fn_def()) - } - - #[cfg(not(feature = "no_function"))] - pub fn iter_script_fn_info(&self) -> impl Iterator { - self.functions - .values() - .filter(|(_, _, _, _, v)| v.is_script()) - .map(|(_, _, _, _, v)| { - let f = v.get_fn_def(); - (f.access, f.name.as_str(), f.params.len()) + .map(CallableFunction::get_shared_fn_def) + .map(|f| { + let func = f.clone(); + (f.access, f.name.as_str(), f.params.len(), func) }) } @@ -1315,14 +1328,13 @@ impl Module { let ast_lib: Shared = ast.lib().clone().into(); ast.iter_functions() - .filter(|(access, _, _)| access.is_public()) - .for_each(|(_, name, num_args)| { - let fn_name = name.to_string(); + .filter(|(access, _, _, _)| access.is_public()) + .for_each(|(_, name, num_params, func)| { let ast_lib = ast_lib.clone(); module.set_raw_fn_as_scripted( name, - num_args, + num_params, move |engine: &Engine, lib: &Module, args: &mut [&mut Dynamic]| { let mut lib_merged; @@ -1336,12 +1348,16 @@ impl Module { }; engine - .call_fn_dynamic_raw( - &mut Scope::new(), - &unified_lib, - &fn_name, + .call_script_fn( + &mut Default::default(), + &mut Default::default(), + &mut Default::default(), + unified_lib, &mut None, + &func.name, + func.as_ref(), args, + 0, ) .map_err(|err| { // Wrap the error in a module-error @@ -1390,7 +1406,7 @@ impl Module { .functions .iter() .filter(|(_, (_, access, _, _, _))| access.is_public()) - .for_each(|(&_hash, (name, _, _num_args, params, func))| { + .for_each(|(&_hash, (name, _, _num_params, params, func))| { if let Some(params) = params { // Qualified Rust functions are indexed in two steps: // 1) Calculate a hash in a similar manner to script-defined functions, @@ -1409,7 +1425,12 @@ impl Module { _hash } else { // Qualifiers + function name + number of arguments. - calc_fn_hash(qualifiers.iter().map(|&v| v), &name, *_num_args, empty()) + calc_fn_hash( + qualifiers.iter().map(|&v| v), + &name, + *_num_params, + empty(), + ) }; functions.push((hash_qualified_script, func.clone())); } diff --git a/src/parser.rs b/src/parser.rs index 4618f041..f197ebc2 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -301,8 +301,10 @@ impl AST { /// Iterate through all functions #[cfg(not(feature = "no_function"))] - pub fn iter_functions(&self) -> impl Iterator { - self.1.iter_script_fn_info() + pub fn iter_functions<'a>( + &'a self, + ) -> impl Iterator)> + 'a { + self.1.iter_script_fn() } /// Clear all function definitions in the `AST`. From eec3f4e1bfa2b5023acbaf4f7eb905108f9e53ca Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 3 Oct 2020 11:42:54 +0800 Subject: [PATCH 08/29] Module:;eval_ast_as_new defaults to merging namespaces. --- RELEASES.md | 2 + benches/eval_module.rs | 35 +----- doc/src/language/fn-namespaces.md | 81 ------------ doc/src/rust/modules/ast.md | 29 +---- doc/src/rust/modules/resolvers.md | 47 ------- src/module/mod.rs | 30 ++--- src/module/resolvers/file.rs | 4 +- src/module/resolvers/global_file.rs | 188 ---------------------------- src/module/resolvers/mod.rs | 8 -- tests/modules.rs | 4 +- tests/packages.rs | 5 +- tests/plugins.rs | 1 + 12 files changed, 22 insertions(+), 412 deletions(-) delete mode 100644 src/module/resolvers/global_file.rs diff --git a/RELEASES.md b/RELEASES.md index b8a45ed8..26ae0f99 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -9,6 +9,8 @@ Breaking changes * `AST::iter_functions` now returns an iterator instead of taking a closure. * `Module::iter_script_fn_info` is removed and merged into `Module::iter_script_fn`. +* The `merge_namespaces` parameter to `Module::eval_ast_as_new` is removed and now defaults to `true`. +* `GlobalFileModuleResolver` is removed because its performance gain over the `FileModuleResolver` is no longer very significant. Version 0.19.0 diff --git a/benches/eval_module.rs b/benches/eval_module.rs index 98843eeb..350877fe 100644 --- a/benches/eval_module.rs +++ b/benches/eval_module.rs @@ -7,7 +7,7 @@ use rhai::{module_resolvers::StaticModuleResolver, Engine, Module, OptimizationL use test::Bencher; #[bench] -fn bench_eval_module_merged(bench: &mut Bencher) { +fn bench_eval_module(bench: &mut Bencher) { let script = r#" fn foo(x) { x + 1 } fn bar(x) { foo(x) } @@ -18,38 +18,7 @@ fn bench_eval_module_merged(bench: &mut Bencher) { let ast = engine.compile(script).unwrap(); - let module = Module::eval_ast_as_new(Default::default(), &ast, true, &engine).unwrap(); - - let mut resolver = StaticModuleResolver::new(); - resolver.insert("testing", module); - engine.set_module_resolver(Some(resolver)); - - let ast = engine - .compile( - r#" - fn foo(x) { x - 1 } - import "testing" as t; - t::bar(41) - "#, - ) - .unwrap(); - - bench.iter(|| engine.consume_ast(&ast).unwrap()); -} - -#[bench] -fn bench_eval_module_unmerged(bench: &mut Bencher) { - let script = r#" - fn foo(x) { x + 1 } - fn bar(x) { foo(x) } - "#; - - let mut engine = Engine::new(); - engine.set_optimization_level(OptimizationLevel::None); - - let ast = engine.compile(script).unwrap(); - - let module = Module::eval_ast_as_new(Default::default(), &ast, false, &engine).unwrap(); + let module = Module::eval_ast_as_new(Default::default(), &ast, &engine).unwrap(); let mut resolver = StaticModuleResolver::new(); resolver.insert("testing", module); diff --git a/doc/src/language/fn-namespaces.md b/doc/src/language/fn-namespaces.md index 11ce1107..e1a4a55e 100644 --- a/doc/src/language/fn-namespaces.md +++ b/doc/src/language/fn-namespaces.md @@ -88,84 +88,3 @@ fn say_hello() { } say_hello(); ``` - - -Namespace Consideration When Not Using `FileModuleResolver` ---------------------------------------------------------- - -[Modules] can be dynamically loaded into a Rhai script using the [`import`] keyword. -When that happens, functions defined within the [module] can be called with a _qualified_ name. - -The [`FileModuleResolver`][module resolver] encapsulates the namespace inside the module itself, -so everything works as expected. A function defined in the module script cannot access functions -defined in the calling script, but it can freely call functions defined within the same module script. - -There is a catch, though. When using anything other than the [`FileModuleResolver`][module resolver], -functions in a module script refer to functions defined in the _global namespace_. -When called later, those functions may not be found. - -When using the [`GlobalFileModuleResolver`][module resolver], for example: - -```rust -Using GlobalFileModuleResolver -============================== - ------------------ -| greeting.rhai | ------------------ - -fn get_message() { "Hello!" }; - -fn say_hello() { - print(get_message()); // 'get_message' is looked up in the global namespace - // when exported -} - -say_hello(); // Here, 'get_message' is found in the module namespace - ---------------- -| script.rhai | ---------------- - -import "greeting" as g; -g::say_hello(); // <- error: function not found - 'get_message' - // because it does not exist in the global namespace -``` - -In the example above, although the module `greeting.rhai` loads fine (`"Hello!"` is printed), -the subsequent call using the _namespace-qualified_ function name fails to find the same function -'`message`' which now essentially becomes `g::message`. The call fails as there is no more -function named '`message`' in the global namespace. - -Therefore, when writing functions for a [module] intended for the [`GlobalFileModuleResolver`][module resolver], -make sure that those functions are as independent as possible and avoid cross-calling them from each other. - -A [function pointer] is a valid technique to call another function in an environment-independent manner: - -```rust ------------------ -| greeting.rhai | ------------------ - -fn get_message() { "Hello!" }; - -fn say_hello(msg_func) { // 'msg_func' is a function pointer - print(msg_func.call()); // call via the function pointer -} - -say_hello(Fn("get_message")); - - ---------------- -| script.rhai | ---------------- - -import "greeting" as g; - -fn my_msg() { - import "greeting" as g; // <- must import again here... - g::get_message() // <- ... otherwise will not find module 'g' -} - -g::say_hello(Fn("my_msg")); // prints 'Hello!' -``` diff --git a/doc/src/rust/modules/ast.md b/doc/src/rust/modules/ast.md index fed0cb09..51f1b187 100644 --- a/doc/src/rust/modules/ast.md +++ b/doc/src/rust/modules/ast.md @@ -18,21 +18,8 @@ When given an [`AST`], it is first evaluated, then the following items are expos * Global modules that remain in the [`Scope`] at the end of a script run. - -`merge_namespaces` Parameter ---------------------------- - -The parameter `merge_namespaces` in `Module::eval_ast_as_new` determines the exact behavior of -functions exposed by the module and the namespace that they can access: - -| `merge_namespaces` value | Description | Namespace | Performance | Call global functions | Call functions in same module | -| :----------------------: | ------------------------------------------------ | :-----------------: | :---------: | :-------------------: | :---------------------------: | -| `true` | encapsulate entire `AST` into each function call | module, then global | 2x slower | yes | yes | -| `false` | register each function independently | global only | fast | yes | no | - -If the ultimate intention is to load the [module] directly into an [`Engine`] via `Engine::load_package`, -set `merge_namespaces` to `false` because there will not be any _module_ namespace as `Engine::load_package` -flattens everything into the _global_ namespace anyway. +`Module::eval_ast_as_new` encapsulates the entire `AST` into each function call, merging the module namespace +with the global namespace. Therefore, functions defined within the same module script can cross-call each other. Examples @@ -77,15 +64,9 @@ let ast = engine.compile(r#" "#)?; // Convert the 'AST' into a module, using the 'Engine' to evaluate it first -// -// The second parameter ('merge_namespaces'), when set to true, will encapsulate -// a copy of the entire 'AST' into each function, allowing functions in the module script -// to cross-call each other. -// -// This incurs additional overhead, avoidable by setting 'merge_namespaces' to false -// which makes function calls 2x faster but at the expense of not being able to cross-call -// functions in the same module script. -let module = Module::eval_ast_as_new(Scope::new(), &ast, true, &engine)?; +// A copy of the entire 'AST' is encapsulated into each function, +// allowing functions in the module script to cross-call each other. +let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; // 'module' now contains: // - sub-module: 'foobar' (renamed from 'extra') diff --git a/doc/src/rust/modules/resolvers.md b/doc/src/rust/modules/resolvers.md index ebb73698..c042b9f1 100644 --- a/doc/src/rust/modules/resolvers.md +++ b/doc/src/rust/modules/resolvers.md @@ -56,53 +56,6 @@ The base directory can be changed via the `FileModuleResolver::new_with_path` co `FileModuleResolver::create_module` loads a script file and returns a module. -`GlobalFileModuleResolver` -------------------------- - -A simpler but more efficient version of `FileModuleResolver`, intended for short utility modules. -Not available for [`no_std`] or [WASM] builds. -Loads a script file (based off the current directory) with `.rhai` extension. - -All functions are assumed **independent** and _cannot_ cross-call each other. -Functions are searched _only_ in the _global_ namespace. - -```rust ------------------- -| my_module.rhai | ------------------- - -private fn inner_message() { "hello! from module!" } - -fn greet_inner() { - print(inner_message()); // cross-calling a module function! - // there will be trouble because each function - // in the module is supposed to be independent - // of each other -} - -fn greet() { - print(main_message()); // function is searched in global namespace -} - -------------- -| main.rhai | -------------- - -fn main_message() { "hi! from main!" } - -import "my_module" as m; - -m::greet_inner(); // <- function not found: 'inner_message' - -m::greet(); // works because 'main_message' exists in - // the global namespace -``` - -The base directory can be changed via the `FileModuleResolver::new_with_path` constructor function. - -`GlobalFileModuleResolver::create_module` loads a script file and returns a module. - - `StaticModuleResolver` --------------------- diff --git a/src/module/mod.rs b/src/module/mod.rs index 11153dae..f888f52b 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1266,19 +1266,10 @@ impl Module { /// Create a new `Module` by evaluating an `AST`. /// - /// ### `merge_namespaces` parameter - /// - /// * If `true`, the entire `AST` is encapsulated into each function, allowing functions - /// to cross-call each other. Functions in the global namespace, plus all functions - /// defined in the module, are _merged_ into a _unified_ namespace before each call. - /// Therefore, all functions will be found, at the expense of some performance. - /// - /// * If `false`, each function is registered independently and cannot cross-call each other. - /// Functions are searched in the global namespace. - /// This is roughly 2x faster than the `true` case. - /// - /// Set `merge_namespaces` to `false` if the ultimate intention is to load the module - /// via `Engine::load_package` because it does not create any module namespace. + /// The entire `AST` is encapsulated into each function, allowing functions + /// to cross-call each other. Functions in the global namespace, plus all functions + /// defined in the module, are _merged_ into a _unified_ namespace before each call. + /// Therefore, all functions will be found. /// /// # Examples /// @@ -1288,19 +1279,14 @@ impl Module { /// /// let engine = Engine::new(); /// let ast = engine.compile("let answer = 42; export answer;")?; - /// let module = Module::eval_ast_as_new(Scope::new(), &ast, true, &engine)?; + /// let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; /// assert!(module.contains_var("answer")); /// assert_eq!(module.get_var_value::("answer").unwrap(), 42); /// # Ok(()) /// # } /// ``` #[cfg(not(feature = "no_module"))] - pub fn eval_ast_as_new( - mut scope: Scope, - ast: &AST, - merge_namespaces: bool, - engine: &Engine, - ) -> FuncReturn { + pub fn eval_ast_as_new(mut scope: Scope, ast: &AST, engine: &Engine) -> FuncReturn { let mut mods = Imports::new(); // Run the script @@ -1324,7 +1310,7 @@ impl Module { }); #[cfg(not(feature = "no_function"))] - if merge_namespaces { + { let ast_lib: Shared = ast.lib().clone().into(); ast.iter_functions() @@ -1370,8 +1356,6 @@ impl Module { }, ); }); - } else { - module.merge(ast.lib()); } Ok(module) diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index bd49b429..11bfb5b1 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -153,7 +153,7 @@ impl ModuleResolver for FileModuleResolver { let c = self.cache.read().unwrap(); if let Some(ast) = c.get(&file_path) { - module = Module::eval_ast_as_new(scope, ast, true, engine).map_err(|err| { + module = Module::eval_ast_as_new(scope, ast, engine).map_err(|err| { Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) })?; None @@ -163,7 +163,7 @@ impl ModuleResolver for FileModuleResolver { Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) })?; - module = Module::eval_ast_as_new(scope, &ast, true, engine).map_err(|err| { + module = Module::eval_ast_as_new(scope, &ast, engine).map_err(|err| { Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) })?; Some(ast) diff --git a/src/module/resolvers/global_file.rs b/src/module/resolvers/global_file.rs deleted file mode 100644 index f9445d09..00000000 --- a/src/module/resolvers/global_file.rs +++ /dev/null @@ -1,188 +0,0 @@ -use crate::engine::Engine; -use crate::module::{Module, ModuleResolver}; -use crate::parser::AST; -use crate::result::EvalAltResult; -use crate::token::Position; - -use crate::stdlib::{boxed::Box, collections::HashMap, path::PathBuf, string::String}; - -#[cfg(not(feature = "sync"))] -use crate::stdlib::cell::RefCell; - -#[cfg(feature = "sync")] -use crate::stdlib::sync::RwLock; - -/// Module resolution service that loads module script files from the file system. -/// -/// All functions in each module are treated as strictly independent and cannot refer to -/// other functions within the same module. Functions are searched in the _global_ namespace. -/// -/// For simple utility libraries, this usually performs better than the full `FileModuleResolver`. -/// -/// Script files are cached so they are are not reloaded and recompiled in subsequent requests. -/// -/// The `new_with_path` and `new_with_path_and_extension` constructor functions -/// allow specification of a base directory with module path used as a relative path offset -/// to the base directory. The script file is then forced to be in a specified extension -/// (default `.rhai`). -/// -/// # Examples -/// -/// ``` -/// use rhai::Engine; -/// use rhai::module_resolvers::GlobalFileModuleResolver; -/// -/// // Create a new 'GlobalFileModuleResolver' loading scripts from the 'scripts' subdirectory -/// // with file extension '.x'. -/// let resolver = GlobalFileModuleResolver::new_with_path_and_extension("./scripts", "x"); -/// -/// let mut engine = Engine::new(); -/// -/// engine.set_module_resolver(Some(resolver)); -/// ``` -#[derive(Debug)] -pub struct GlobalFileModuleResolver { - path: PathBuf, - extension: String, - - #[cfg(not(feature = "sync"))] - cache: RefCell>, - - #[cfg(feature = "sync")] - cache: RwLock>, -} - -impl Default for GlobalFileModuleResolver { - fn default() -> Self { - Self::new_with_path(PathBuf::default()) - } -} - -impl GlobalFileModuleResolver { - /// Create a new `GlobalFileModuleResolver` with a specific base path. - /// - /// # Examples - /// - /// ``` - /// use rhai::Engine; - /// use rhai::module_resolvers::GlobalFileModuleResolver; - /// - /// // Create a new 'GlobalFileModuleResolver' loading scripts from the 'scripts' subdirectory - /// // with file extension '.rhai' (the default). - /// let resolver = GlobalFileModuleResolver::new_with_path("./scripts"); - /// - /// let mut engine = Engine::new(); - /// engine.set_module_resolver(Some(resolver)); - /// ``` - pub fn new_with_path>(path: P) -> Self { - Self::new_with_path_and_extension(path, "rhai") - } - - /// Create a new `GlobalFileModuleResolver` with a specific base path and file extension. - /// - /// The default extension is `.rhai`. - /// - /// # Examples - /// - /// ``` - /// use rhai::Engine; - /// use rhai::module_resolvers::GlobalFileModuleResolver; - /// - /// // Create a new 'GlobalFileModuleResolver' loading scripts from the 'scripts' subdirectory - /// // with file extension '.x'. - /// let resolver = GlobalFileModuleResolver::new_with_path_and_extension("./scripts", "x"); - /// - /// let mut engine = Engine::new(); - /// engine.set_module_resolver(Some(resolver)); - /// ``` - pub fn new_with_path_and_extension, E: Into>( - path: P, - extension: E, - ) -> Self { - Self { - path: path.into(), - extension: extension.into(), - cache: Default::default(), - } - } - - /// Create a new `GlobalFileModuleResolver` with the current directory as base path. - /// - /// # Examples - /// - /// ``` - /// use rhai::Engine; - /// use rhai::module_resolvers::GlobalFileModuleResolver; - /// - /// // Create a new 'GlobalFileModuleResolver' loading scripts from the current directory - /// // with file extension '.rhai' (the default). - /// let resolver = GlobalFileModuleResolver::new(); - /// - /// let mut engine = Engine::new(); - /// engine.set_module_resolver(Some(resolver)); - /// ``` - pub fn new() -> Self { - Default::default() - } - - /// Create a `Module` from a file path. - pub fn create_module>( - &self, - engine: &Engine, - path: &str, - ) -> Result> { - self.resolve(engine, path, Default::default()) - } -} - -impl ModuleResolver for GlobalFileModuleResolver { - fn resolve( - &self, - engine: &Engine, - path: &str, - pos: Position, - ) -> Result> { - // Construct the script file path - let mut file_path = self.path.clone(); - file_path.push(path); - file_path.set_extension(&self.extension); // Force extension - - let scope = Default::default(); - let module; - - // See if it is cached - let ast = { - #[cfg(not(feature = "sync"))] - let c = self.cache.borrow(); - #[cfg(feature = "sync")] - let c = self.cache.read().unwrap(); - - if let Some(ast) = c.get(&file_path) { - module = Module::eval_ast_as_new(scope, ast, false, engine).map_err(|err| { - Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) - })?; - None - } else { - // Load the file and compile it if not found - let ast = engine.compile_file(file_path.clone()).map_err(|err| { - Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) - })?; - - module = Module::eval_ast_as_new(scope, &ast, false, engine).map_err(|err| { - Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) - })?; - Some(ast) - } - }; - - if let Some(ast) = ast { - // Put it into the cache - #[cfg(not(feature = "sync"))] - self.cache.borrow_mut().insert(file_path, ast); - #[cfg(feature = "sync")] - self.cache.write().unwrap().insert(file_path, ast); - } - - Ok(module) - } -} diff --git a/src/module/resolvers/mod.rs b/src/module/resolvers/mod.rs index b9da048d..386d9e05 100644 --- a/src/module/resolvers/mod.rs +++ b/src/module/resolvers/mod.rs @@ -17,14 +17,6 @@ mod file; #[cfg(not(target_arch = "wasm32"))] pub use file::FileModuleResolver; -#[cfg(not(feature = "no_std"))] -#[cfg(not(target_arch = "wasm32"))] -mod global_file; - -#[cfg(not(feature = "no_std"))] -#[cfg(not(target_arch = "wasm32"))] -pub use global_file::GlobalFileModuleResolver; - mod stat; pub use stat::StaticModuleResolver; diff --git a/tests/modules.rs b/tests/modules.rs index db82d600..d2ee5e1c 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -265,7 +265,7 @@ fn test_module_from_ast() -> Result<(), Box> { engine.set_module_resolver(Some(resolver1)); - let module = Module::eval_ast_as_new(Scope::new(), &ast, true, &engine)?; + let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; let mut resolver2 = StaticModuleResolver::new(); resolver2.insert("testing", module); @@ -374,7 +374,7 @@ fn test_module_ast_namespace() -> Result<(), Box> { let ast = engine.compile(script)?; - let module = Module::eval_ast_as_new(Default::default(), &ast, true, &engine)?; + let module = Module::eval_ast_as_new(Default::default(), &ast, &engine)?; let mut resolver = StaticModuleResolver::new(); resolver.insert("testing", module); diff --git a/tests/packages.rs b/tests/packages.rs index b0235b6b..ae31e719 100644 --- a/tests/packages.rs +++ b/tests/packages.rs @@ -36,12 +36,9 @@ fn test_packages_with_script() -> Result<(), Box> { let mut engine = Engine::new(); let ast = engine.compile("fn foo(x) { x + 1 } fn bar(x) { foo(x) + 1 }")?; - let module = Module::eval_ast_as_new(Scope::new(), &ast, false, &engine)?; + let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; engine.load_package(module); assert_eq!(engine.eval::("foo(41)")?, 42); - - let module = Module::eval_ast_as_new(Scope::new(), &ast, true, &engine)?; - engine.load_package(module); assert_eq!(engine.eval::("bar(40)")?, 42); Ok(()) diff --git a/tests/plugins.rs b/tests/plugins.rs index 909ad51b..ce36d6bd 100644 --- a/tests/plugins.rs +++ b/tests/plugins.rs @@ -47,6 +47,7 @@ mod test { macro_rules! gen_unary_functions { ($op_name:ident = $op_fn:ident ( $($arg_type:ident),+ ) -> $return_type:ident) => { mod $op_name { $( + #[allow(non_snake_case)] pub mod $arg_type { use super::super::*; From fbfb7677c1df22bc33f0aa1321fb6485c8e9ae5d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 3 Oct 2020 16:25:58 +0800 Subject: [PATCH 09/29] Add is_def_var and is_def_fn. --- RELEASES.md | 5 ++ doc/src/about/index.md | 2 + doc/src/appendix/keywords.md | 62 ++++++++++++----------- doc/src/language/functions.md | 15 ++++++ doc/src/language/variables.md | 8 +++ doc/src/rust/modules/resolvers.md | 83 ++++++++++++++++++++++++++++--- src/engine.rs | 3 ++ src/fn_call.rs | 71 ++++++++++++++++++++++---- src/module/mod.rs | 7 ++- src/token.rs | 20 +++++--- 10 files changed, 220 insertions(+), 56 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 26ae0f99..5f082cb0 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -12,6 +12,11 @@ Breaking changes * The `merge_namespaces` parameter to `Module::eval_ast_as_new` is removed and now defaults to `true`. * `GlobalFileModuleResolver` is removed because its performance gain over the `FileModuleResolver` is no longer very significant. +New features +------------ + +* `is_def_var()` to detect if variable is defined and `is_def_fn()` to detect if script function is defined. + Version 0.19.0 ============== diff --git a/doc/src/about/index.md b/doc/src/about/index.md index ce966635..869016b9 100644 --- a/doc/src/about/index.md +++ b/doc/src/about/index.md @@ -14,7 +14,9 @@ Versions This Book is for version **{{version}}** of Rhai. +{% if rootUrl != "" and not rootUrl is ending_with("vnext") %} For the latest development version, see [here]({{rootUrl}}/vnext/). +{% endif %} Etymology of the name "Rhai" diff --git a/doc/src/appendix/keywords.md b/doc/src/appendix/keywords.md index 47a7b446..b403ac56 100644 --- a/doc/src/appendix/keywords.md +++ b/doc/src/appendix/keywords.md @@ -3,36 +3,38 @@ Keywords List {{#include ../links.md}} -| Keyword | Description | Inactive under | Overloadable | -| :-------------------: | ------------------------------------------- | :-------------: | :----------: | -| `true` | boolean true literal | | no | -| `false` | boolean false literal | | no | -| `let` | variable declaration | | no | -| `const` | constant declaration | | no | -| `is_shared` | is a value shared? | | no | -| `if` | if statement | | no | -| `else` | else block of if statement | | no | -| `while` | while loop | | no | -| `loop` | infinite loop | | no | -| `for` | for loop | | no | -| `in` | 1) containment test
2) part of for loop | | no | -| `continue` | continue a loop at the next iteration | | no | -| `break` | break out of loop iteration | | no | -| `return` | return value | | no | -| `throw` | throw exception | | no | -| `import` | import module | [`no_module`] | no | -| `export` | export variable | [`no_module`] | no | -| `as` | alias for variable export | [`no_module`] | no | -| `private` | mark function private | [`no_function`] | no | -| `fn` (lower-case `f`) | function definition | [`no_function`] | no | -| `Fn` (capital `F`) | create a [function pointer] | | yes | -| `call` | call a [function pointer] | | no | -| `curry` | curry a [function pointer] | | no | -| `this` | reference to base object for method call | [`no_function`] | no | -| `type_of` | get type name of value | | yes | -| `print` | print value | | yes | -| `debug` | print value in debug format | | yes | -| `eval` | evaluate script | | yes | +| Keyword | Description | Inactive under | Is function? | Overloadable | +| :-------------------: | ------------------------------------------- | :-------------: | :----------: | :----------: | +| `true` | boolean true literal | | no | | +| `false` | boolean false literal | | no | | +| `let` | variable declaration | | no | | +| `const` | constant declaration | | no | | +| `is_def_var` | is a variable declared? | | yes | yes | +| `is_shared` | is a value shared? | [`no_closure`] | yes | no | +| `if` | if statement | | no | | +| `else` | else block of if statement | | no | | +| `while` | while loop | | no | | +| `loop` | infinite loop | | no | | +| `for` | for loop | | no | | +| `in` | 1) containment test
2) part of for loop | | no | | +| `continue` | continue a loop at the next iteration | | no | | +| `break` | break out of loop iteration | | no | | +| `return` | return value | | no | | +| `throw` | throw exception | | no | | +| `import` | import module | [`no_module`] | no | | +| `export` | export variable | [`no_module`] | no | | +| `as` | alias for variable export | [`no_module`] | no | | +| `private` | mark function private | [`no_function`] | no | | +| `fn` (lower-case `f`) | function definition | [`no_function`] | no | | +| `Fn` (capital `F`) | create a [function pointer] | | yes | yes | +| `call` | call a [function pointer] | | yes | no | +| `curry` | curry a [function pointer] | | yes | no | +| `this` | reference to base object for method call | [`no_function`] | no | | +| `is_def_fn` | is a scripted function defined? | [`no_function`] | yes | yes | +| `type_of` | get type name of value | | yes | yes | +| `print` | print value | | yes | yes | +| `debug` | print value in debug format | | yes | yes | +| `eval` | evaluate script | | yes | yes | Reserved Keywords diff --git a/doc/src/language/functions.md b/doc/src/language/functions.md index 5b9498a6..362dc057 100644 --- a/doc/src/language/functions.md +++ b/doc/src/language/functions.md @@ -101,6 +101,21 @@ a statement in the script can freely call a function defined afterwards. This is similar to Rust and many other modern languages, such as JavaScript's `function` keyword. +`is_def_fn` +----------- + +Use `is_def_fn` to detect if a function is defined (and therefore callable), based on its name +and the number of parameters. + +```rust +fn foo(x) { x + 1 } + +is_def_fn("foo", 1) == true; + +is_def_fn("bar", 1) == false; +``` + + Arguments are Passed by Value ---------------------------- diff --git a/doc/src/language/variables.md b/doc/src/language/variables.md index 455c075b..fd211e19 100644 --- a/doc/src/language/variables.md +++ b/doc/src/language/variables.md @@ -37,6 +37,8 @@ If none is provided, it defaults to [`()`]. A variable defined within a statement block is _local_ to that block. +Use `is_def_var` to detect if a variable is defined. + ```rust let x; // ok - value is '()' let x = 3; // ok @@ -57,4 +59,10 @@ X == 123; x == 999; // access to local 'x' } x == 42; // the parent block's 'x' is not changed + +is_def_var("x") == true; + +is_def_var("_x") == true; + +is_def_var("y") == false; ``` diff --git a/doc/src/rust/modules/resolvers.md b/doc/src/rust/modules/resolvers.md index c042b9f1..abca9c27 100644 --- a/doc/src/rust/modules/resolvers.md +++ b/doc/src/rust/modules/resolvers.md @@ -32,28 +32,97 @@ are _merged_ into a _unified_ namespace. | my_module.rhai | ------------------ +// This function overrides any in the main script. private fn inner_message() { "hello! from module!" } -fn greet(callback) { print(callback.call()); } +fn greet() { + print(inner_message()); // call function in module script +} + +fn greet_main() { + print(main_message()); // call function not in module script +} ------------- | main.rhai | ------------- -fn main_message() { "hi! from main!" } +// This function is overridden by the module script. +fn inner_message() { "hi! from main!" } + +// This function is found by the module script. +fn main_message() { "main here!" } import "my_module" as m; -m::greet(|| "hello, " + "world!"); // works - anonymous function in global +m::greet(); // prints "hello! from module!" -m::greet(|| inner_message()); // works - function in module - -m::greet(|| main_message()); // works - function in global +m::greet_main(); // prints "main here!" ``` +### Simulating virtual functions + +When calling a namespace-qualified function defined within a module, other functions defined within +the same module script override any similar-named functions (with the same number of parameters) +defined in the global namespace. This is to ensure that a module acts as a self-contained unit and +functions defined in the calling script do not override module code. + +In some situations, however, it is actually beneficial to do it in reverse: have module code call functions +defined in the calling script (i.e. in the global namespace) if they exist, and only call those defined +in the module script if none are found. + +One such situation is the need to provide a _default implementation_ to a simulated _virtual_ function: + +```rust +------------------ +| my_module.rhai | +------------------ + +// Do not do this (it will override the main script): +// fn message() { "hello! from module!" } + +// This function acts as the default implementation. +private fn default_message() { "hello! from module!" } + +// This function depends on a 'virtual' function 'message' +// which is not defined in the module script. +fn greet() { + if is_def_fn("message", 0) { // 'is_def_fn' detects if 'message' is defined. + print(message()); + } else { + print(default_message()); + } +} + +------------- +| main.rhai | +------------- + +// The main script defines 'message' which is needed by the module script. +fn message() { "hi! from main!" } + +import "my_module" as m; + +m::greet(); // prints "hi! from main!" + +-------------- +| main2.rhai | +-------------- + +// The main script does not define 'message' which is needed by the module script. + +import "my_module" as m; + +m::greet(); // prints "hello! from module!" +``` + +### Changing the base directory + The base directory can be changed via the `FileModuleResolver::new_with_path` constructor function. -`FileModuleResolver::create_module` loads a script file and returns a module. +### Returning a module instead + +`FileModuleResolver::create_module` loads a script file and returns a module with the standard behavior. `StaticModuleResolver` diff --git a/src/engine.rs b/src/engine.rs index 29c742af..77f4c1e3 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -99,7 +99,10 @@ pub const KEYWORD_EVAL: &str = "eval"; pub const KEYWORD_FN_PTR: &str = "Fn"; pub const KEYWORD_FN_PTR_CALL: &str = "call"; pub const KEYWORD_FN_PTR_CURRY: &str = "curry"; +#[cfg(not(feature = "no_closure"))] pub const KEYWORD_IS_SHARED: &str = "is_shared"; +pub const KEYWORD_IS_DEF_VAR: &str = "is_def_var"; +pub const KEYWORD_IS_DEF_FN: &str = "is_def_fn"; pub const KEYWORD_THIS: &str = "this"; pub const FN_TO_STRING: &str = "to_string"; #[cfg(not(feature = "no_object"))] diff --git a/src/fn_call.rs b/src/fn_call.rs index 9ee74b86..7908c424 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -4,8 +4,8 @@ use crate::any::Dynamic; use crate::calc_fn_hash; use crate::engine::{ search_imports, search_namespace, search_scope_only, Engine, Imports, State, KEYWORD_DEBUG, - KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_IS_SHARED, - KEYWORD_PRINT, KEYWORD_TYPE_OF, + KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_IS_DEF_FN, + KEYWORD_IS_DEF_VAR, KEYWORD_PRINT, KEYWORD_TYPE_OF, }; use crate::error::ParseErrorType; use crate::fn_native::{FnCallArgs, FnPtr}; @@ -33,6 +33,9 @@ use crate::engine::{FN_IDX_GET, FN_IDX_SET}; #[cfg(not(feature = "no_object"))] use crate::engine::{Map, Target, FN_GET, FN_SET}; +#[cfg(not(feature = "no_closure"))] +use crate::engine::KEYWORD_IS_SHARED; + #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_function"))] use crate::scope::Entry as ScopeEntry; @@ -744,15 +747,18 @@ impl Engine { .into(), false, )) - } else if cfg!(not(feature = "no_closure")) - && _fn_name == KEYWORD_IS_SHARED - && idx.is_empty() - { + } else if { + #[cfg(not(feature = "no_closure"))] + { + _fn_name == KEYWORD_IS_SHARED && idx.is_empty() + } + #[cfg(feature = "no_closure")] + false + } { // is_shared call Ok((target.is_shared().into(), false)) } else { - #[cfg(not(feature = "no_object"))] - let redirected; + let _redirected; let mut hash = hash_script; // Check if it is a map method call in OOP style @@ -761,8 +767,8 @@ impl Engine { if let Some(val) = map.get(_fn_name) { if let Some(fn_ptr) = val.read_lock::() { // Remap the function name - redirected = fn_ptr.get_fn_name().clone(); - _fn_name = &redirected; + _redirected = fn_ptr.get_fn_name().clone(); + _fn_name = &_redirected; // Add curried arguments if !fn_ptr.curry().is_empty() { fn_ptr @@ -877,7 +883,8 @@ impl Engine { } // Handle is_shared() - if cfg!(not(feature = "no_closure")) && name == KEYWORD_IS_SHARED && args_expr.len() == 1 { + #[cfg(not(feature = "no_closure"))] + if name == KEYWORD_IS_SHARED && args_expr.len() == 1 { let expr = args_expr.get(0).unwrap(); let value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; @@ -917,6 +924,48 @@ impl Engine { } } + // Handle is_def_var() + if name == KEYWORD_IS_DEF_VAR && args_expr.len() == 1 { + let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); + + if !self.has_override(lib, hash_fn, hash_script, pub_only) { + let expr = args_expr.get(0).unwrap(); + if let Ok(var_name) = self + .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? + .as_str() + { + return Ok(scope.contains(var_name).into()); + } + } + } + + // Handle is_def_fn() + if name == KEYWORD_IS_DEF_FN && args_expr.len() == 2 { + let hash_fn = calc_fn_hash( + empty(), + name, + 2, + [TypeId::of::(), TypeId::of::()] + .iter() + .cloned(), + ); + + if !self.has_override(lib, hash_fn, hash_script, pub_only) { + let fn_name_expr = args_expr.get(0).unwrap(); + let num_params_expr = args_expr.get(1).unwrap(); + + if let (Ok(fn_name), Ok(num_params)) = ( + self.eval_expr(scope, mods, state, lib, this_ptr, fn_name_expr, level)? + .as_str(), + self.eval_expr(scope, mods, state, lib, this_ptr, num_params_expr, level)? + .as_int(), + ) { + let hash = calc_fn_hash(empty(), fn_name, num_params as usize, empty()); + return Ok(lib.contains_fn(hash, false).into()); + } + } + } + // Handle eval() if name == KEYWORD_EVAL && args_expr.len() == 1 { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); diff --git a/src/module/mod.rs b/src/module/mod.rs index f888f52b..4bb5ce57 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -454,6 +454,9 @@ impl Module { /// Arguments are simply passed in as a mutable array of `&mut Dynamic`, /// which is guaranteed to contain enough arguments of the correct types. /// + /// The function is assumed to be a _method_, meaning that the first argument should not be consumed. + /// All other arguments can be consumed. + /// /// To access a primary parameter value (i.e. cloning is cheap), use: `args[n].clone().cast::()` /// /// To access a parameter value and avoid cloning, use `std::mem::take(args[n]).cast::()`. @@ -1299,8 +1302,8 @@ impl Module { .into_iter() .for_each(|ScopeEntry { value, alias, .. }| { // Variables with an alias left in the scope become module variables - if alias.is_some() { - module.variables.insert(*alias.unwrap(), value); + if let Some(alias) = alias { + module.variables.insert(*alias, value); } }); diff --git a/src/token.rs b/src/token.rs index 6245310a..bcc758f8 100644 --- a/src/token.rs +++ b/src/token.rs @@ -2,9 +2,12 @@ use crate::engine::{ Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, - KEYWORD_IS_SHARED, KEYWORD_PRINT, KEYWORD_THIS, KEYWORD_TYPE_OF, + KEYWORD_IS_DEF_FN, KEYWORD_IS_DEF_VAR, KEYWORD_PRINT, KEYWORD_THIS, KEYWORD_TYPE_OF, }; +#[cfg(not(feature = "no_closure"))] +use crate::engine::KEYWORD_IS_SHARED; + use crate::error::LexError; use crate::parser::INT; use crate::utils::StaticVec; @@ -507,9 +510,11 @@ impl Token { | "await" | "yield" => Reserved(syntax.into()), KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR - | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_IS_SHARED | KEYWORD_THIS => { - Reserved(syntax.into()) - } + | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_IS_DEF_VAR + | KEYWORD_IS_DEF_FN | KEYWORD_THIS => Reserved(syntax.into()), + + #[cfg(not(feature = "no_closure"))] + KEYWORD_IS_SHARED => Reserved(syntax.into()), _ => return None, }) @@ -1455,7 +1460,9 @@ pub fn is_keyword_function(name: &str) -> bool { #[cfg(not(feature = "no_closure"))] KEYWORD_IS_SHARED => true, KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR - | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY => true, + | KEYWORD_FN_PTR_CALL | KEYWORD_FN_PTR_CURRY | KEYWORD_IS_DEF_VAR | KEYWORD_IS_DEF_FN => { + true + } _ => false, } } @@ -1465,7 +1472,8 @@ pub fn is_keyword_function(name: &str) -> bool { #[inline(always)] pub fn can_override_keyword(name: &str) -> bool { match name { - KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR => true, + KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR + | KEYWORD_IS_DEF_VAR | KEYWORD_IS_DEF_FN => true, _ => false, } } From 1e13e6be5f8b92609eaae040c41ce6acf9da527c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 3 Oct 2020 18:49:11 +0800 Subject: [PATCH 10/29] Doc formatting. --- src/engine.rs | 6 +++--- src/error.rs | 2 +- src/lib.rs | 10 ++++------ src/module/mod.rs | 2 +- src/parser.rs | 18 ++++++++++-------- src/token.rs | 10 +++++----- src/utils.rs | 4 ++-- 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 77f4c1e3..9e12fe0d 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -60,7 +60,7 @@ pub type Array = Vec; #[cfg(not(feature = "no_object"))] pub type Map = HashMap; -/// [INTERNALS] A stack of imported modules. +/// _[INTERNALS]_ A stack of imported modules. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -289,7 +289,7 @@ impl> From for Target<'_> { } } -/// [INTERNALS] A type that holds all the current states of the Engine. +/// _[INTERNALS]_ A type that holds all the current states of the Engine. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -318,7 +318,7 @@ impl State { } } -/// [INTERNALS] A type containing all the limits imposed by the `Engine`. +/// _[INTERNALS]_ A type containing all the limits imposed by the `Engine`. /// Exported under the `internals` feature only. /// /// ## WARNING diff --git a/src/error.rs b/src/error.rs index 1d5ea7f8..2c210d60 100644 --- a/src/error.rs +++ b/src/error.rs @@ -10,7 +10,7 @@ use crate::stdlib::{ string::{String, ToString}, }; -/// [INTERNALS] Error encountered when tokenizing the script text. +/// _[INTERNALS]_ Error encountered when tokenizing the script text. /// Exported under the `internals` feature only. /// /// ## WARNING diff --git a/src/lib.rs b/src/lib.rs index 13536be4..f61af46c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -128,16 +128,14 @@ pub mod module_resolvers { pub use crate::module::resolvers::*; } -/// Serialization support for [`serde`](https://crates.io/crates/serde). -/// -/// Requires the `serde` feature. +/// _[SERDE]_ Serialization support for [`serde`](https://crates.io/crates/serde). +/// Exported under the `serde` feature. #[cfg(feature = "serde")] pub mod ser { pub use crate::serde::ser::to_dynamic; } -/// Deserialization support for [`serde`](https://crates.io/crates/serde). -/// -/// Requires the `serde` feature. +/// _[SERDE]_ Deserialization support for [`serde`](https://crates.io/crates/serde). +/// Exported under the `serde` feature. #[cfg(feature = "serde")] pub mod de { pub use crate::serde::de::from_dynamic; diff --git a/src/module/mod.rs b/src/module/mod.rs index 4bb5ce57..e62ef520 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1457,7 +1457,7 @@ impl Module { } } -/// [INTERNALS] A chain of module names to qualify a variable or function call. +/// _[INTERNALS]_ A chain of module names to qualify a variable or function call. /// Exported under the `internals` feature only. /// /// A `u64` hash key is cached for quick search purposes. diff --git a/src/parser.rs b/src/parser.rs index f197ebc2..f85215d4 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -91,7 +91,8 @@ impl AST { &self.0 } - /// [INTERNALS] Get the statements. + /// _[INTERNALS]_ Get the statements. + /// Exported under the `internals` feature only. #[cfg(feature = "internals")] #[deprecated(note = "this method is volatile and may change")] pub fn statements(&self) -> &[Stmt] { @@ -109,7 +110,8 @@ impl AST { &self.1 } - /// [INTERNALS] Get the internal `Module` containing all script-defined functions. + /// _[INTERNALS]_ Get the internal `Module` containing all script-defined functions. + /// Exported under the `internals` feature only. #[cfg(feature = "internals")] #[deprecated(note = "this method is volatile and may change")] pub fn lib(&self) -> &Module { @@ -374,7 +376,7 @@ impl FnAccess { } } -/// [INTERNALS] A type containing information on a scripted function. +/// _[INTERNALS]_ A type containing information on a scripted function. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -416,7 +418,7 @@ impl fmt::Display for ScriptFnDef { } } -/// [INTERNALS] A type encapsulating the mode of a `return`/`throw` statement. +/// _[INTERNALS]_ A type encapsulating the mode of a `return`/`throw` statement. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -561,7 +563,7 @@ impl ParseSettings { } } -/// [INTERNALS] A Rhai statement. +/// _[INTERNALS]_ A Rhai statement. /// Exported under the `internals` feature only. /// /// Each variant is at most one pointer in size (for speed), @@ -721,7 +723,7 @@ impl Stmt { } } -/// [INTERNALS] A type wrapping a custom syntax definition. +/// _[INTERNALS]_ A type wrapping a custom syntax definition. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -742,7 +744,7 @@ impl Hash for CustomExpr { } } -/// [INTERNALS] A type wrapping a floating-point number. +/// _[INTERNALS]_ A type wrapping a floating-point number. /// Exported under the `internals` feature only. /// /// This type is mainly used to provide a standard `Hash` implementation @@ -763,7 +765,7 @@ impl Hash for FloatWrapper { } } -/// [INTERNALS] An expression sub-tree. +/// _[INTERNALS]_ An expression sub-tree. /// Exported under the `internals` feature only. /// /// Each variant is at most one pointer in size (for speed), diff --git a/src/token.rs b/src/token.rs index bcc758f8..3494b02e 100644 --- a/src/token.rs +++ b/src/token.rs @@ -147,7 +147,7 @@ impl fmt::Debug for Position { } } -/// [INTERNALS] A Rhai language token. +/// _[INTERNALS]_ A Rhai language token. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -709,7 +709,7 @@ impl From for String { } } -/// [INTERNALS] State of the tokenizer. +/// _[INTERNALS]_ State of the tokenizer. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -729,7 +729,7 @@ pub struct TokenizeState { pub include_comments: bool, } -/// [INTERNALS] Trait that encapsulates a peekable character input stream. +/// _[INTERNALS]_ Trait that encapsulates a peekable character input stream. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -743,7 +743,7 @@ pub trait InputStream { fn peek_next(&mut self) -> Option; } -/// [INTERNALS] Parse a string literal wrapped by `enclosing_char`. +/// _[INTERNALS]_ Parse a string literal wrapped by `enclosing_char`. /// Exported under the `internals` feature only. /// /// ## WARNING @@ -931,7 +931,7 @@ fn scan_comment( } } -/// [INTERNALS] Get the next token from the `InputStream`. +/// _[INTERNALS]_ Get the next token from the `InputStream`. /// Exported under the `internals` feature only. /// /// ## WARNING diff --git a/src/utils.rs b/src/utils.rs index a257d910..94110d1e 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -61,7 +61,7 @@ impl BuildHasher for StraightHasherBuilder { } } -/// [INTERNALS] Calculate a `u64` hash key from a module-qualified function name and parameter types. +/// _[INTERNALS]_ Calculate a `u64` hash key from a module-qualified function name and parameter types. /// Exported under the `internals` feature only. /// /// Module names are passed in via `&str` references from an iterator. @@ -89,7 +89,7 @@ pub fn calc_fn_hash<'a>( s.finish() } -/// [INTERNALS] Alias to [`smallvec::SmallVec<[T; 4]>`](https://crates.io/crates/smallvec), +/// _[INTERNALS]_ Alias to [`smallvec::SmallVec<[T; 4]>`](https://crates.io/crates/smallvec), /// which is a specialized `Vec` backed by a small, fixed-size array when there are <= 4 items stored. /// Exported under the `internals` feature only. pub type StaticVec = SmallVec<[T; 4]>; From 9664ae42a7c75dc6395cda7c5756f35c53a27344 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 3 Oct 2020 21:59:19 +0800 Subject: [PATCH 11/29] Add is_def_XXX tests. --- tests/constants.rs | 26 ++++++++++++++++++++++++++ tests/functions.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/tests/constants.rs b/tests/constants.rs index 266b6ac2..4569cda0 100644 --- a/tests/constants.rs +++ b/tests/constants.rs @@ -21,3 +21,29 @@ fn test_constant() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_var_is_def() -> Result<(), Box> { + let engine = Engine::new(); + + assert!(engine.eval::( + r#" + let x = 42; + is_def_var("x") + "# + )?); + assert!(!engine.eval::( + r#" + let x = 42; + is_def_var("y") + "# + )?); + assert!(engine.eval::( + r#" + const x = 42; + is_def_var("x") + "# + )?); + + Ok(()) +} diff --git a/tests/functions.rs b/tests/functions.rs index 34143ab6..062eb1ae 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -173,3 +173,29 @@ fn test_function_captures() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_function_is_def() -> Result<(), Box> { + let engine = Engine::new(); + + assert!(engine.eval::( + r#" + fn foo(x) { x + 1 } + is_def_fn("foo", 1) + "# + )?); + assert!(!engine.eval::( + r#" + fn foo(x) { x + 1 } + is_def_fn("bar", 1) + "# + )?); + assert!(!engine.eval::( + r#" + fn foo(x) { x + 1 } + is_def_fn("foo", 0) + "# + )?); + + Ok(()) +} From 23d0f52284a82076d7579486444644e1769d2c82 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 3 Oct 2020 23:27:30 +0800 Subject: [PATCH 12/29] Better error messages. --- RELEASES.md | 1 + src/engine.rs | 195 ++++++++++++++++++++++++++++--------------------- src/fn_call.rs | 104 ++++++++++++++------------ src/result.rs | 17 +++-- 4 files changed, 180 insertions(+), 137 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 5f082cb0..4aea4692 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -11,6 +11,7 @@ Breaking changes * `Module::iter_script_fn_info` is removed and merged into `Module::iter_script_fn`. * The `merge_namespaces` parameter to `Module::eval_ast_as_new` is removed and now defaults to `true`. * `GlobalFileModuleResolver` is removed because its performance gain over the `FileModuleResolver` is no longer very significant. +* `EvalAltResult::ErrorCharMismatch` is renamed to `EvalAltResult::ErrorMismatchDataType`. New features ------------ diff --git a/src/engine.rs b/src/engine.rs index 9e12fe0d..842bea3a 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -229,24 +229,32 @@ impl Target<'_> { } } /// Update the value of the `Target`. - /// Position in `EvalAltResult` is `None` and must be set afterwards. - pub fn set_value(&mut self, new_val: Dynamic) -> Result<(), Box> { + pub fn set_value( + &mut self, + new_val: Dynamic, + target_pos: Position, + new_pos: Position, + ) -> Result<(), Box> { match self { Self::Ref(r) => **r = new_val, #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_object"))] Self::LockGuard((r, _)) => **r = new_val, Self::Value(_) => { - return EvalAltResult::ErrorAssignmentToUnknownLHS(Position::none()).into(); + return EvalAltResult::ErrorAssignmentToUnknownLHS(target_pos).into(); } #[cfg(not(feature = "no_index"))] Self::StringChar(string, index, _) if string.is::() => { let mut s = string.write_lock::().unwrap(); // Replace the character at the specified index position - let new_ch = new_val - .as_char() - .map_err(|_| EvalAltResult::ErrorCharMismatch(Position::none()))?; + let new_ch = new_val.as_char().map_err(|err| { + Box::new(EvalAltResult::ErrorMismatchDataType( + err.to_string(), + "char".to_string(), + new_pos, + )) + })?; let mut chars = s.chars().collect::>(); let ch = chars[*index]; @@ -413,56 +421,7 @@ impl fmt::Debug for Engine { impl Default for Engine { fn default() -> Self { - // Create the new scripting Engine - let mut engine = Self { - id: None, - - packages: Default::default(), - global_module: Default::default(), - - #[cfg(not(feature = "no_module"))] - #[cfg(not(feature = "no_std"))] - #[cfg(not(target_arch = "wasm32"))] - module_resolver: Some(Box::new(resolvers::FileModuleResolver::new())), - #[cfg(not(feature = "no_module"))] - #[cfg(any(feature = "no_std", target_arch = "wasm32",))] - module_resolver: None, - - type_names: None, - disabled_symbols: None, - custom_keywords: None, - custom_syntax: None, - - // default print/debug implementations - print: Box::new(default_print), - debug: Box::new(default_print), - - // progress callback - progress: None, - - // optimization level - optimization_level: if cfg!(feature = "no_optimize") { - OptimizationLevel::None - } else { - OptimizationLevel::Simple - }, - - #[cfg(not(feature = "unchecked"))] - limits: Limits { - max_call_stack_depth: MAX_CALL_STACK_DEPTH, - max_expr_depth: MAX_EXPR_DEPTH, - max_function_expr_depth: MAX_FUNCTION_EXPR_DEPTH, - max_operations: 0, - max_modules: usize::MAX, - max_string_size: 0, - max_array_size: 0, - max_map_size: 0, - }, - }; - - engine.load_package(StandardPackage::new().get()); - - engine + Self::new() } } @@ -628,7 +587,56 @@ pub fn search_scope_only<'s, 'a>( impl Engine { /// Create a new `Engine` pub fn new() -> Self { - Default::default() + // Create the new scripting Engine + let mut engine = Self { + id: None, + + packages: Default::default(), + global_module: Default::default(), + + #[cfg(not(feature = "no_module"))] + #[cfg(not(feature = "no_std"))] + #[cfg(not(target_arch = "wasm32"))] + module_resolver: Some(Box::new(resolvers::FileModuleResolver::new())), + #[cfg(not(feature = "no_module"))] + #[cfg(any(feature = "no_std", target_arch = "wasm32",))] + module_resolver: None, + + type_names: None, + disabled_symbols: None, + custom_keywords: None, + custom_syntax: None, + + // default print/debug implementations + print: Box::new(default_print), + debug: Box::new(default_print), + + // progress callback + progress: None, + + // optimization level + optimization_level: if cfg!(feature = "no_optimize") { + OptimizationLevel::None + } else { + OptimizationLevel::Simple + }, + + #[cfg(not(feature = "unchecked"))] + limits: Limits { + max_call_stack_depth: MAX_CALL_STACK_DEPTH, + max_expr_depth: MAX_EXPR_DEPTH, + max_function_expr_depth: MAX_FUNCTION_EXPR_DEPTH, + max_operations: 0, + max_modules: usize::MAX, + max_string_size: 0, + max_array_size: 0, + max_map_size: 0, + }, + }; + + engine.load_package(StandardPackage::new().get()); + + engine } /// Create a new `Engine` with minimal built-in functions. @@ -686,6 +694,7 @@ impl Engine { chain_type: ChainType, level: usize, new_val: Option, + new_pos: Position, ) -> Result<(Dynamic, bool), Box> { if chain_type == ChainType::None { panic!(); @@ -718,7 +727,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, obj_ptr, expr, idx_values, next_chain, level, - new_val, + new_val, new_pos, ) .map_err(|err| err.new_position(*pos)) } @@ -732,10 +741,7 @@ impl Engine { { // Indexed value is a reference - update directly Ok(ref mut obj_ptr) => { - obj_ptr - .set_value(new_val.unwrap()) - .map_err(|err| err.new_position(rhs.position()))?; - + obj_ptr.set_value(new_val.unwrap(), rhs.position(), new_pos)?; None } Err(err) => match *err { @@ -799,8 +805,7 @@ impl Engine { let mut val = self .get_indexed_mut(state, lib, target, index, *pos, true, false, level)?; - val.set_value(new_val.unwrap()) - .map_err(|err| err.new_position(rhs.position()))?; + val.set_value(new_val.unwrap(), rhs.position(), new_pos)?; Ok((Default::default(), true)) } // {xxx:map}.id @@ -868,7 +873,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, &mut val, expr, idx_values, next_chain, level, - new_val, + new_val, new_pos, ) .map_err(|err| err.new_position(*pos)) } @@ -902,6 +907,7 @@ impl Engine { next_chain, level, new_val, + new_pos, ) .map_err(|err| err.new_position(*pos))?; @@ -941,7 +947,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, target, expr, idx_values, next_chain, - level, new_val, + level, new_val, new_pos, ) .map_err(|err| err.new_position(*pos)) } @@ -972,6 +978,7 @@ impl Engine { expr: &Expr, level: usize, new_val: Option, + new_pos: Position, ) -> Result> { let ((dot_lhs, dot_rhs, op_pos), chain_type) = match expr { Expr::Index(x) => (x.as_ref(), ChainType::Index), @@ -1007,7 +1014,8 @@ impl Engine { let obj_ptr = &mut target.into(); self.eval_dot_index_chain_helper( - state, lib, &mut None, obj_ptr, dot_rhs, idx_values, chain_type, level, new_val, + state, lib, &mut None, obj_ptr, dot_rhs, idx_values, chain_type, level, + new_val, new_pos, ) .map(|(v, _)| v) .map_err(|err| err.new_position(*op_pos)) @@ -1022,6 +1030,7 @@ impl Engine { let obj_ptr = &mut val.into(); self.eval_dot_index_chain_helper( state, lib, this_ptr, obj_ptr, dot_rhs, idx_values, chain_type, level, new_val, + new_pos, ) .map(|(v, _)| v) .map_err(|err| err.new_position(*op_pos)) @@ -1409,9 +1418,9 @@ impl Engine { let mut rhs_val = self.eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)?; - let _new_val = Some(if op.is_empty() { + let (_new_val, _new_pos) = if op.is_empty() { // Normal assignment - rhs_val + (Some(rhs_val), rhs_expr.position()) } else { // Op-assignment - always map to `lhs = lhs op rhs` let op = &op[..op.len() - 1]; // extract operator without = @@ -1419,12 +1428,16 @@ impl Engine { &mut self.eval_expr(scope, mods, state, lib, this_ptr, lhs_expr, level)?, &mut rhs_val, ]; - self.exec_fn_call( - state, lib, op, 0, args, false, false, false, None, &None, level, - ) - .map(|(v, _)| v) - .map_err(|err| err.new_position(*op_pos))? - }); + + let result = self + .exec_fn_call( + state, lib, op, 0, args, false, false, false, None, &None, level, + ) + .map(|(v, _)| v) + .map_err(|err| err.new_position(*op_pos))?; + + (Some(result), rhs_expr.position()) + }; match lhs_expr { // name op= rhs @@ -1433,7 +1446,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] Expr::Index(_) => { self.eval_dot_index_chain( - scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, + scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, _new_pos, )?; Ok(Default::default()) } @@ -1441,7 +1454,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] Expr::Dot(_) => { self.eval_dot_index_chain( - scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, + scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, _new_pos, )?; Ok(Default::default()) } @@ -1458,15 +1471,31 @@ impl Engine { // lhs[idx_expr] #[cfg(not(feature = "no_index"))] - Expr::Index(_) => { - self.eval_dot_index_chain(scope, mods, state, lib, this_ptr, expr, level, None) - } + Expr::Index(_) => self.eval_dot_index_chain( + scope, + mods, + state, + lib, + this_ptr, + expr, + level, + None, + Position::none(), + ), // lhs.dot_rhs #[cfg(not(feature = "no_object"))] - Expr::Dot(_) => { - self.eval_dot_index_chain(scope, mods, state, lib, this_ptr, expr, level, None) - } + Expr::Dot(_) => self.eval_dot_index_chain( + scope, + mods, + state, + lib, + this_ptr, + expr, + level, + None, + Position::none(), + ), #[cfg(not(feature = "no_index"))] Expr::Array(x) => Ok(Dynamic(Union::Array(Box::new( diff --git a/src/fn_call.rs b/src/fn_call.rs index 7908c424..bd850f34 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -614,7 +614,7 @@ impl Engine { mods: &mut Imports, state: &mut State, lib: &Module, - script_expr: &Dynamic, + script: &str, _level: usize, ) -> Result> { self.inc_operations(state)?; @@ -628,14 +628,6 @@ impl Engine { )); } - let script = script_expr.as_str().map_err(|typ| { - EvalAltResult::ErrorMismatchOutputType( - self.map_type_name(type_name::()).into(), - typ.into(), - Position::none(), - ) - })?; - // Compile the script text // No optimizations because we only run it once let mut ast = self.compile_with_scope_and_optimization_level( @@ -804,7 +796,7 @@ impl Engine { // Feed the changed temp value back if updated && !is_ref && !is_value { let new_val = target.as_mut().clone(); - target.set_value(new_val)?; + target.set_value(new_val, Position::none(), Position::none())?; } Ok((result, updated)) @@ -828,6 +820,15 @@ impl Engine { capture: bool, level: usize, ) -> Result> { + fn make_type_err(engine: &Engine, typ: &str, pos: Position) -> Box { + EvalAltResult::ErrorMismatchDataType( + typ.into(), + engine.map_type_name(type_name::()).into(), + pos, + ) + .into() + } + // Handle Fn() if name == KEYWORD_FN_PTR && args_expr.len() == 1 { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); @@ -839,14 +840,7 @@ impl Engine { return arg_value .take_immutable_string() - .map_err(|typ| { - EvalAltResult::ErrorMismatchOutputType( - self.map_type_name(type_name::()).into(), - typ.into(), - expr.position(), - ) - .into() - }) + .map_err(|typ| make_type_err::(self, typ, expr.position())) .and_then(|s| FnPtr::try_from(s)) .map(Into::::into) .map_err(|err| err.new_position(expr.position())); @@ -856,18 +850,17 @@ impl Engine { // Handle curry() if name == KEYWORD_FN_PTR_CURRY && args_expr.len() > 1 { let expr = args_expr.get(0).unwrap(); - let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; - if !fn_ptr.is::() { - return EvalAltResult::ErrorMismatchOutputType( - self.map_type_name(type_name::()).into(), - self.map_type_name(fn_ptr.type_name()).into(), + if !arg_value.is::() { + return Err(make_type_err::( + self, + self.map_type_name(arg_value.type_name()), expr.position(), - ) - .into(); + )); } - let (fn_name, fn_curry) = fn_ptr.cast::().take_data(); + let (fn_name, fn_curry) = arg_value.cast::().take_data(); let curry: StaticVec<_> = args_expr .iter() @@ -902,10 +895,10 @@ impl Engine { && !self.has_override(lib, 0, hash_script, pub_only) { let expr = args_expr.get(0).unwrap(); - let fn_name = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; - if fn_name.is::() { - let fn_ptr = fn_name.cast::(); + if arg_value.is::() { + let fn_ptr = arg_value.cast::(); curry = fn_ptr.curry().iter().cloned().collect(); // Redirect function name redirected = fn_ptr.take_data().0; @@ -915,12 +908,11 @@ impl Engine { // Recalculate hash hash_script = calc_fn_hash(empty(), name, curry.len() + args_expr.len(), empty()); } else { - return EvalAltResult::ErrorMismatchOutputType( - self.map_type_name(type_name::()).into(), - fn_name.type_name().into(), + return Err(make_type_err::( + self, + self.map_type_name(arg_value.type_name()), expr.position(), - ) - .into(); + )); } } @@ -930,10 +922,13 @@ impl Engine { if !self.has_override(lib, hash_fn, hash_script, pub_only) { let expr = args_expr.get(0).unwrap(); - if let Ok(var_name) = self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? + let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let var_name = arg_value .as_str() - { + .map_err(|err| make_type_err::(self, err, expr.position()))?; + if var_name.is_empty() { + return Ok(false.into()); + } else { return Ok(scope.contains(var_name).into()); } } @@ -954,12 +949,21 @@ impl Engine { let fn_name_expr = args_expr.get(0).unwrap(); let num_params_expr = args_expr.get(1).unwrap(); - if let (Ok(fn_name), Ok(num_params)) = ( - self.eval_expr(scope, mods, state, lib, this_ptr, fn_name_expr, level)? - .as_str(), - self.eval_expr(scope, mods, state, lib, this_ptr, num_params_expr, level)? - .as_int(), - ) { + let arg0_value = + self.eval_expr(scope, mods, state, lib, this_ptr, fn_name_expr, level)?; + let arg1_value = + self.eval_expr(scope, mods, state, lib, this_ptr, num_params_expr, level)?; + + let fn_name = arg0_value.as_str().map_err(|err| { + make_type_err::(self, err, fn_name_expr.position()) + })?; + let num_params = arg1_value + .as_int() + .map_err(|err| make_type_err::(self, err, num_params_expr.position()))?; + + if fn_name.is_empty() || num_params < 0 { + return Ok(false.into()); + } else { let hash = calc_fn_hash(empty(), fn_name, num_params as usize, empty()); return Ok(lib.contains_fn(hash, false).into()); } @@ -974,10 +978,16 @@ impl Engine { // eval - only in function call style let prev_len = scope.len(); let expr = args_expr.get(0).unwrap(); - let script = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; - let result = self - .eval_script_expr(scope, mods, state, lib, &script, level + 1) - .map_err(|err| err.new_position(expr.position())); + let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let script = arg_value + .as_str() + .map_err(|typ| make_type_err::(self, typ, expr.position()))?; + let result = if !script.is_empty() { + self.eval_script_expr(scope, mods, state, lib, script, level + 1) + .map_err(|err| err.new_position(expr.position())) + } else { + Ok(().into()) + }; // IMPORTANT! If the eval defines new variables in the current scope, // all variable offsets from this point on will be mis-aligned. diff --git a/src/result.rs b/src/result.rs index 16f18981..8ad32926 100644 --- a/src/result.rs +++ b/src/result.rs @@ -46,8 +46,9 @@ pub enum EvalAltResult { ErrorUnboundThis(Position), /// Non-boolean operand encountered for boolean operator. Wrapped value is the operator. ErrorBooleanArgMismatch(String, Position), - /// Non-character value encountered where a character is required. - ErrorCharMismatch(Position), + /// Data is not of the required type. + /// Wrapped values are the type requested and type of the actual result. + ErrorMismatchDataType(String, String, Position), /// Array access out-of-bounds. /// Wrapped values are the current number of elements in the array and the index number. ErrorArrayBounds(usize, INT, Position), @@ -120,7 +121,7 @@ impl EvalAltResult { Self::ErrorFunctionNotFound(_, _) => "Function not found", Self::ErrorUnboundThis(_) => "'this' is not bound", Self::ErrorBooleanArgMismatch(_, _) => "Boolean operator expects boolean operands", - Self::ErrorCharMismatch(_) => "Character expected", + Self::ErrorMismatchDataType(_, _, _) => "Data type is incorrect", Self::ErrorNumericIndexExpr(_) => { "Indexing into an array or string expects an integer index" } @@ -215,7 +216,10 @@ impl fmt::Display for EvalAltResult { Self::ErrorAssignmentToConstant(s, _) => write!(f, "{}: '{}'", desc, s)?, Self::ErrorMismatchOutputType(r, s, _) => { - write!(f, "{} (expecting {}): {}", desc, s, r)? + write!(f, "Output type is incorrect: {} (expecting {})", r, s)? + } + Self::ErrorMismatchDataType(r, s, _) => { + write!(f, "Data type is incorrect: {} (expecting {})", r, s)? } Self::ErrorArithmetic(s, _) => f.write_str(s)?, @@ -225,7 +229,6 @@ impl fmt::Display for EvalAltResult { Self::ErrorBooleanArgMismatch(op, _) => { write!(f, "{} operator expects boolean operands", op)? } - Self::ErrorCharMismatch(_) => write!(f, "string indexing expects a character value")?, Self::ErrorArrayBounds(_, index, _) if *index < 0 => { write!(f, "{}: {} < 0", desc, index)? } @@ -291,7 +294,7 @@ impl EvalAltResult { | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) | Self::ErrorBooleanArgMismatch(_, pos) - | Self::ErrorCharMismatch(pos) + | Self::ErrorMismatchDataType(_, _, pos) | Self::ErrorArrayBounds(_, _, pos) | Self::ErrorStringBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) @@ -333,7 +336,7 @@ impl EvalAltResult { | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) | Self::ErrorBooleanArgMismatch(_, pos) - | Self::ErrorCharMismatch(pos) + | Self::ErrorMismatchDataType(_, _, pos) | Self::ErrorArrayBounds(_, _, pos) | Self::ErrorStringBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) From d802829156a2eb6ea012aac3c5e1ddc07d634d6c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 3 Oct 2020 23:29:34 +0800 Subject: [PATCH 13/29] Bump version. --- Cargo.toml | 2 +- doc/src/context.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1103a7c4..aafb2ac3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ members = [ [package] name = "rhai" -version = "0.19.0" +version = "0.20.0" edition = "2018" authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung", "jhwgh1968"] description = "Embedded scripting for Rust" diff --git a/doc/src/context.json b/doc/src/context.json index 792990b1..79f961ec 100644 --- a/doc/src/context.json +++ b/doc/src/context.json @@ -1,5 +1,5 @@ { - "version": "0.19.0", + "version": "0.20.0", "repoHome": "https://github.com/jonathandturner/rhai/blob/master", "repoTree": "https://github.com/jonathandturner/rhai/tree/master", "rootUrl": "", From a962debf0d5a9d459729233c02a9a3df78b0d228 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 4 Oct 2020 10:40:44 +0800 Subject: [PATCH 14/29] Simplify target back propagation. --- src/engine.rs | 16 ++++++++++++++++ src/fn_call.rs | 8 +++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 842bea3a..327ccc8e 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -150,6 +150,7 @@ pub enum Target<'a> { #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] impl Target<'_> { /// Is the `Target` a reference pointing to other data? + #[allow(dead_code)] #[inline(always)] pub fn is_ref(&self) -> bool { match self { @@ -163,6 +164,7 @@ impl Target<'_> { } } /// Is the `Target` an owned value? + #[allow(dead_code)] #[inline(always)] pub fn is_value(&self) -> bool { match self { @@ -176,6 +178,7 @@ impl Target<'_> { } } /// Is the `Target` a shared value? + #[allow(dead_code)] #[inline(always)] pub fn is_shared(&self) -> bool { match self { @@ -228,6 +231,19 @@ impl Target<'_> { Self::StringChar(_, _, ref mut r) => r, } } + /// Propagate a changed value back to the original source. + /// This has no effect except for string indexing. + pub fn propagate_changed_value(&mut self) { + match self { + Self::Ref(_) | Self::LockGuard(_) | Self::Value(_) => (), + #[cfg(not(feature = "no_index"))] + Self::StringChar(_, _, ch) => { + let new_val = ch.clone(); + self.set_value(new_val, Position::none(), Position::none()) + .unwrap(); + } + } + } /// Update the value of the `Target`. pub fn set_value( &mut self, diff --git a/src/fn_call.rs b/src/fn_call.rs index bd850f34..b78e222f 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -670,7 +670,6 @@ impl Engine { level: usize, ) -> Result<(Dynamic, bool), Box> { let is_ref = target.is_ref(); - let is_value = target.is_value(); // Get a reference to the mutation target Dynamic let obj = target.as_mut(); @@ -793,10 +792,9 @@ impl Engine { ) }?; - // Feed the changed temp value back - if updated && !is_ref && !is_value { - let new_val = target.as_mut().clone(); - target.set_value(new_val, Position::none(), Position::none())?; + // Propagate the changed value back to the source if necessary + if updated { + target.propagate_changed_value(); } Ok((result, updated)) From b91a07359614bfc327349bd508a9f37240f2ccda Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 4 Oct 2020 23:05:33 +0800 Subject: [PATCH 15/29] Add events handler pattern. --- doc/src/SUMMARY.md | 41 +++++---- doc/src/patterns/events.md | 182 +++++++++++++++++++++++++++++++++++++ doc/src/patterns/index.md | 6 +- 3 files changed, 206 insertions(+), 23 deletions(-) create mode 100644 doc/src/patterns/events.md diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 5193b7bb..f8e279cc 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -52,18 +52,19 @@ The Rhai Scripting Language 1. [Comments](language/comments.md) 2. [Values and Types](language/values-and-types.md) 1. [Dynamic Values](language/dynamic.md) - 2. [type_of()](language/type-of.md) - 3. [Numbers](language/numbers.md) + 2. [Serialization/Deserialization with `serde`](rust/serde.md) + 3. [type_of()](language/type-of.md) + 4. [Numbers](language/numbers.md) 1. [Operators](language/num-op.md) 2. [Functions](language/num-fn.md) 3. [Value Conversions](language/convert.md) - 4. [Strings and Characters](language/strings-chars.md) + 5. [Strings and Characters](language/strings-chars.md) 1. [Built-in Functions](language/string-fn.md) - 5. [Arrays](language/arrays.md) - 6. [Object Maps](language/object-maps.md) + 6. [Arrays](language/arrays.md) + 7. [Object Maps](language/object-maps.md) 1. [Parse from JSON](language/json.md) 2. [Special Support for OOP](language/object-maps-oop.md) - 7. [Time-Stamps](language/timestamps.md) + 8. [Time-Stamps](language/timestamps.md) 3. [Keywords](language/keywords.md) 4. [Statements](language/statements.md) 5. [Variables](language/variables.md) @@ -104,29 +105,29 @@ The Rhai Scripting Language 7. [Maximum Number of Modules](safety/max-modules.md) 8. [Maximum Call Stack Depth](safety/max-call-stack.md) 9. [Maximum Statement Depth](safety/max-stmt-depth.md) -7. [Advanced Topics](advanced.md) - 1. [Advanced Patterns](patterns/index.md) - 1. [Object-Oriented Programming (OOP)](patterns/oop.md) - 2. [Loadable Configuration](patterns/config.md) - 3. [Control Layer](patterns/control.md) - 4. [Singleton Command](patterns/singleton.md) - 5. [One Engine Instance Per Call](patterns/parallel.md) - 2. [Capture Scope for Function Call](language/fn-capture.md) - 3. [Serialization/Deserialization of `Dynamic` with `serde`](rust/serde.md) - 4. [Script Optimization](engine/optimize/index.md) +7. [Usage Patterns](patterns/index.md) + 1. [Object-Oriented Programming (OOP)](patterns/oop.md) + 2. [Loadable Configuration](patterns/config.md) + 3. [Control Layer](patterns/control.md) + 4. [Singleton Command](patterns/singleton.md) + 5. [One Engine Instance Per Call](patterns/parallel.md) + 6. [Scriptable Event Handler with State](patterns/events.md) +8. [Advanced Topics](advanced.md) + 1. [Capture Scope for Function Call](language/fn-capture.md) + 2. [Script Optimization](engine/optimize/index.md) 1. [Optimization Levels](engine/optimize/optimize-levels.md) 2. [Re-Optimize an AST](engine/optimize/reoptimize.md) 3. [Eager Function Evaluation](engine/optimize/eager.md) 4. [Side-Effect Considerations](engine/optimize/side-effects.md) 5. [Volatility Considerations](engine/optimize/volatility.md) 6. [Subtle Semantic Changes](engine/optimize/semantics.md) - 5. [Low-Level API](rust/register-raw.md) - 6. [Use as DSL](engine/dsl.md) + 3. [Low-Level API](rust/register-raw.md) + 4. [Use as DSL](engine/dsl.md) 1. [Disable Keywords and/or Operators](engine/disable.md) 2. [Custom Operators](engine/custom-op.md) 3. [Extending with Custom Syntax](engine/custom-syntax.md) - 7. [Multiple Instantiation](patterns/multiple.md) -8. [Appendix](appendix/index.md) + 5. [Multiple Instantiation](patterns/multiple.md) +9. [Appendix](appendix/index.md) 1. [Keywords](appendix/keywords.md) 2. [Operators and Symbols](appendix/operators.md) 3. [Literals](appendix/literals.md) diff --git a/doc/src/patterns/events.md b/doc/src/patterns/events.md new file mode 100644 index 00000000..9334e726 --- /dev/null +++ b/doc/src/patterns/events.md @@ -0,0 +1,182 @@ +Scriptable Event Handler with State +================================== + +{{#include ../links.md}} + + +Usage Scenario +-------------- + +* A system sends _events_ that must be handled. + +* Flexibility in event handling must be provided, through user-side scripting. + +* State must be kept between invocations of event handlers. + +* Default implementations of event handlers can be provided. + + +Key Concepts +------------ + +* An _event handler_ object is declared that holds the following items: + * [`Engine`] with registered functions serving as an API, + * [`AST`] of the user script, + * a [`Scope`] containing state. + +* Upon an event, the appropriate event handler function in the script is called via [`Engine::call_fn`][`call_fn`]. + +* Optionally, trap the `EvalAltResult::ErrorFunctionNotFound` error to provide a default implementation. + + +Implementation +-------------- + +### Declare Handler Object + +In most cases, it would be simpler to store an [`Engine`] instance together with the handler object +because it only requires registering all API functions only once. + +In rare cases where handlers are created and destroyed in a tight loop, a new [`Engine`] instance +can be created for each event. See [_One Engine Instance Per Call_](parallel.md) for more details. + +```rust +use rhai::{Engine, Scope, AST, EvalAltResult}; + +// Event handler +struct Handler { + // Scripting engine + pub engine: Engine, + // Use a custom 'Scope' to keep stored state + pub scope: Scope<'static>, + // Program script + pub ast: AST +} +``` + +### Initialize Handler Object + +Steps to initialize the event handler: + +1. Register an API with the [`Engine`], +2. Create a custom [`Scope`] to serve as the stored state, +3. Add default state variables into the custom [`Scope`], +4. Get the handler script and [compile][`AST`] it, +5. Store the compiled [`AST`] for future evaluations, +6. Run the [`AST`] to initialize event handler state variables. + +```rust +impl Handler { + pub new(path: impl Into) -> Self { + let mut engine = Engine::new(); + + // Register API functions here + engine + .register_fn("func1", func1) + .register_fn("func2", func2) + .register_fn("func3", func3) + .register_type_with_name::("SomeType") + .register_get_set("value", + |obj: &mut SomeType| obj.data, + |obj: &mut SomeType, value: i64| obj.data = value + ); + + // Create a custom 'Scope' to hold state + let mut scope = Scope::new(); + + // Add initialized state into the custom 'Scope' + scope.push("state1", false); + scope.push("state2", SomeType::new(42)); + + // Compile the handler script. + // In a real application you'd be handling errors... + let ast = engine.compile_file(path).unwrap(); + + // Evaluate the script to initialize it and other state variables. + // In a real application you'd again be handling errors... + engine.consume_ast_with_scope(&mut scope, &ast).unwrap(); + + // The event handler is essentially these three items: + Handler { engine, scope, ast } + } +} +``` + +### Hook up events + +There is usually an interface or trait that gets called when an event comes from the system. + +Mapping an event from the system into a scripted handler is straight-forward: + +```rust +impl Handler { + // Say there are three events: 'start', 'end', 'update'. + // In a real application you'd be handling errors... + pub fn on_event(&mut self, event_name: &str, event_data: i64) { + match event_name { + // The 'start' event maps to function 'start'. + // In a real application you'd be handling errors... + "start" => + self.engine + .call_fn(&mut self.scope, &self.ast, "start", (event_data,)).unwrap(), + + // The 'end' event maps to function 'end'. + // In a real application you'd be handling errors... + "end" => + self.engine + .call_fn(&mut self.scope, &self.ast, "end", (event_data,)).unwrap(), + + // The 'update' event maps to function 'update'. + // This event provides a default implementation when the scripted function + // is not found. + "update" => + self.engine + .call_fn(&mut self.scope, &self.ast, "update", (event_data,)) + .or_else(|err| match *err { + EvalAltResult::ErrorFunctionNotFound(fn_name, _) if fn_name == "update" => { + // Default implementation of 'update' event handler + self.scope.set_value("state2", SomeType::new(42)); + // Turn function-not-found into a success + Ok(().into()) + } + _ => Err(err.into()) + }) + .unwrap() + } + } +} +``` + +### Sample Handler Script + +Because the stored state is kept in a custom [`Scope`], it is possible for all functions defined +in the handler script to access and modify these state variables. + +The API registered with the [`Engine`] can be also used throughout the script. + +```rust +fn start(data) { + if state1 { + throw "Already started!"; + } + if func1(state2) || func2() { + throw "Conditions not yet ready to start!"; + } + state1 = true; + state2.value = 0; +} + +fn end(data) { + if !state1 { + throw "Not yet started!"; + } + if func1(state2) || func2() { + throw "Conditions not yet ready to start!"; + } + state1 = false; +} + +fn update(data) { + state2.value += func3(data); +} +``` diff --git a/doc/src/patterns/index.md b/doc/src/patterns/index.md index b0e5d633..fb651ae8 100644 --- a/doc/src/patterns/index.md +++ b/doc/src/patterns/index.md @@ -1,7 +1,7 @@ -Advanced Patterns -================= +Usage Patterns +============== {{#include ../links.md}} -Leverage the full power and flexibility of Rhai in advanced scenarios. +Leverage the full power and flexibility of Rhai in different scenarios. From 0d0affd5e9f01cccbb35f9d32a35d384c94126e6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 10:27:31 +0800 Subject: [PATCH 16/29] Eagerly evaluate built-in operators for OptimizationLevel::Simple. --- RELEASES.md | 4 +- doc/src/engine/optimize/eager.md | 18 +--- doc/src/engine/optimize/index.md | 82 +++++++++++------ doc/src/engine/optimize/optimize-levels.md | 5 +- doc/src/engine/optimize/reoptimize.md | 10 +- doc/src/engine/optimize/side-effects.md | 17 ++-- doc/src/links.md | 1 + examples/repl.rs | 2 +- src/fn_call.rs | 28 +++++- src/module/mod.rs | 17 ++++ src/optimize.rs | 102 ++++++++++++--------- 11 files changed, 182 insertions(+), 104 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 4aea4692..394e5e80 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -16,7 +16,9 @@ Breaking changes New features ------------ -* `is_def_var()` to detect if variable is defined and `is_def_fn()` to detect if script function is defined. +* `OptimizationLevel::Simple` now eagerly evaluates built-in binary operators of primary types (if not overloaded). +* Added `is_def_var()` to detect if variable is defined and `is_def_fn()` to detect if script function is defined. +* Added `Module::get_script_fn` to get a scripted function in a module, if any, based on name and number of parameters. Version 0.19.0 diff --git a/doc/src/engine/optimize/eager.md b/doc/src/engine/optimize/eager.md index b6bb26c8..2071154b 100644 --- a/doc/src/engine/optimize/eager.md +++ b/doc/src/engine/optimize/eager.md @@ -3,8 +3,8 @@ Eager Function Evaluation When Using Full Optimization Level {{#include ../../links.md}} -When the optimization level is [`OptimizationLevel::Full`], the [`Engine`] assumes all functions to be _pure_ and will _eagerly_ -evaluated all function calls with constant arguments, using the result to replace the call. +When the optimization level is [`OptimizationLevel::Full`], the [`Engine`] assumes all functions to be _pure_ +and will _eagerly_ evaluated all function calls with constant arguments, using the result to replace the call. This also applies to all operators (which are implemented as functions). @@ -14,8 +14,8 @@ For instance, the same example above: // When compiling the following with OptimizationLevel::Full... const DECISION = 1; - // this condition is now eliminated because 'DECISION == 1' -if DECISION == 1 { // is a function call to the '==' function, and it returns 'true' + // this condition is now eliminated because 'sign(DECISION) > 0' +if DECISION.sign() > 0 { // is a call to the 'sign' and '>' functions, and they return 'true' print("hello!"); // this block is promoted to the parent level } else { print("boo!"); // this block is eliminated because it is never reached @@ -24,13 +24,3 @@ if DECISION == 1 { // is a function call to the '==' function, and it r print("hello!"); // <- the above is equivalent to this // ('print' and 'debug' are handled specially) ``` - -Because of the eager evaluation of functions, many constant expressions will be evaluated and replaced by the result. -This does not happen with [`OptimizationLevel::Simple`] which doesn't assume all functions to be _pure_. - -```rust -// When compiling the following with OptimizationLevel::Full... - -let x = (1+2)*3-4/5%6; // <- will be replaced by 'let x = 9' -let y = (1>2) || (3<=4); // <- will be replaced by 'let y = true' -``` diff --git a/doc/src/engine/optimize/index.md b/doc/src/engine/optimize/index.md index e28aa748..6f293181 100644 --- a/doc/src/engine/optimize/index.md +++ b/doc/src/engine/optimize/index.md @@ -61,17 +61,63 @@ For fixed script texts, the constant values can be provided in a user-defined [` to the [`Engine`] for use in compilation and evaluation. -Watch Out for Function Calls ---------------------------- +Eager Operator Evaluations +------------------------- Beware, however, that most operators are actually function calls, and those functions can be overridden, -so they are not optimized away: +so whether they are optimized away depends on the situation: + +* If the operands are not _constant_ values, it is not optimized. + +* If the operator is [overloaded][operator overloading], it is not optimized because the overloading function may not be _pure_ + (i.e. may cause side-effects when called). + +* If the operator is not _binary_, it is not optimized. Only binary operators are built-in to Rhai. + +* If the operands are not of the same type, it is not optimized. + +* If the operator is not _built-in_ (see list of [built-in operators]), it is not optimized. + +* If the operator is a binary built-in operator for a [standard type][standard types], it is called and replaced by a constant result. + +Rhai guarantees that no external function will be run (in order not to trigger side-effects) during the +optimization process (unless the optimization level is set to [`OptimizationLevel::Full`]). ```rust -const DECISION = 1; +const DECISION = 1; // this is an integer, one of the standard types -if DECISION == 1 { // NOT optimized away because you can define - : // your own '==' function to override the built-in default! +if DECISION == 1 { // this is optimized into 'true' + : +} else if DECISION == 2 { // this is optimized into 'false' + : +} else if DECISION == 3 { // this is optimized into 'false' + : +} else { + : +} +``` + +Because of the eager evaluation of operators for [standard types], many constant expressions will be evaluated +and replaced by the result. + +```rust +let x = (1+2)*3-4/5%6; // will be replaced by 'let x = 9' + +let y = (1>2) || (3<=4); // will be replaced by 'let y = true' +``` + +For operators that are not optimized away due to one of the above reasons, the function calls +are simply left behind: + +```rust +// Assume 'new_state' returns some custom type that is NOT one of the standard types. +// Also assume that the '==; operator is defined for that custom type. +const DECISION_1 = new_state(1); +const DECISION_2 = new_state(2); +const DECISION_3 = new_state(3); + +if DECISION == 1 { // NOT optimized away because the operator is not built-in + : // and may cause side-effects if called! : } else if DECISION == 2 { // same here, NOT optimized away : @@ -82,28 +128,4 @@ if DECISION == 1 { // NOT optimized away because you can define } ``` -because no operator functions will be run (in order not to trigger side-effects) during the optimization process -(unless the optimization level is set to [`OptimizationLevel::Full`]). - -So, instead, do this: - -```rust -const DECISION_1 = true; -const DECISION_2 = false; -const DECISION_3 = false; - -if DECISION_1 { - : // this branch is kept and promoted to the parent level -} else if DECISION_2 { - : // this branch is eliminated -} else if DECISION_3 { - : // this branch is eliminated -} else { - : // this branch is eliminated -} -``` - -In general, boolean constants are most effective for the optimizer to automatically prune -large `if`-`else` branches because they do not depend on operators. - Alternatively, turn the optimizer to [`OptimizationLevel::Full`]. diff --git a/doc/src/engine/optimize/optimize-levels.md b/doc/src/engine/optimize/optimize-levels.md index 97dc27ea..0f43938b 100644 --- a/doc/src/engine/optimize/optimize-levels.md +++ b/doc/src/engine/optimize/optimize-levels.md @@ -8,9 +8,10 @@ There are three levels of optimization: `None`, `Simple` and `Full`. * `None` is obvious - no optimization on the AST is performed. * `Simple` (default) performs only relatively _safe_ optimizations without causing side-effects - (i.e. it only relies on static analysis and will not actually perform any function calls). + (i.e. it only relies on static analysis and [built-in operators] for constant [standard types], + and will not perform any external function calls). -* `Full` is _much_ more aggressive, _including_ running functions on constant arguments to determine their result. +* `Full` is _much_ more aggressive, _including_ calling external functions on constant arguments to determine their result. One benefit to this is that many more optimization opportunities arise, especially with regards to comparison operators. diff --git a/doc/src/engine/optimize/reoptimize.md b/doc/src/engine/optimize/reoptimize.md index e8292e4c..38726649 100644 --- a/doc/src/engine/optimize/reoptimize.md +++ b/doc/src/engine/optimize/reoptimize.md @@ -16,11 +16,11 @@ The final, optimized [`AST`] is then used for evaluations. // Compile master script to AST let master_ast = engine.compile( r" - if SCENARIO_1 { + if SCENARIO == 1 { do_work(); - } else if SCENARIO_2 { + } else if SCENARIO == 2 { do_something(); - } else if SCENARIO_3 { + } else if SCENARIO == 3 { do_something_else(); } else { do_nothing(); @@ -29,9 +29,7 @@ r" // Create a new 'Scope' - put constants in it to aid optimization let mut scope = Scope::new(); -scope.push_constant("SCENARIO_1", true); -scope.push_constant("SCENARIO_2", false); -scope.push_constant("SCENARIO_3", false); +scope.push_constant("SCENARIO", 1_i64); // Re-optimize the AST let new_ast = engine.optimize_ast(&scope, master_ast.clone(), OptimizationLevel::Simple); diff --git a/doc/src/engine/optimize/side-effects.md b/doc/src/engine/optimize/side-effects.md index 76009c55..fbacea0b 100644 --- a/doc/src/engine/optimize/side-effects.md +++ b/doc/src/engine/optimize/side-effects.md @@ -3,17 +3,20 @@ Side-Effect Considerations for Full Optimization Level {{#include ../../links.md}} -All of Rhai's built-in functions (and operators which are implemented as functions) are _pure_ (i.e. they do not mutate state -nor cause any side-effects, with the exception of `print` and `debug` which are handled specially) so using -[`OptimizationLevel::Full`] is usually quite safe _unless_ custom types and functions are registered. +All of Rhai's built-in functions (and operators which are implemented as functions) are _pure_ +(i.e. they do not mutate state nor cause any side-effects, with the exception of `print` and `debug` +which are handled specially) so using [`OptimizationLevel::Full`] is usually quite safe _unless_ +custom types and functions are registered. -If custom functions are registered, they _may_ be called (or maybe not, if the calls happen to lie within a pruned code block). +If custom functions are registered, they _may_ be called (or maybe not, if the calls happen to lie +within a pruned code block). -If custom functions are registered to overload built-in operators, they will also be called when the operators are used -(in an `if` statement, for example) causing side-effects. +If custom functions are registered to overload built-in operators, they will also be called when +the operators are used (in an `if` statement, for example) causing side-effects. Therefore, the rule-of-thumb is: * _Always_ register custom types and functions _after_ compiling scripts if [`OptimizationLevel::Full`] is used. -* _DO NOT_ depend on knowledge that the functions have no side-effects, because those functions can change later on and, when that happens, existing scripts may break in subtle ways. +* _DO NOT_ depend on knowledge that the functions have no side-effects, because those functions can change later on and, + when that happens, existing scripts may break in subtle ways. diff --git a/doc/src/links.md b/doc/src/links.md index 2fd9d152..ac7f1f4a 100644 --- a/doc/src/links.md +++ b/doc/src/links.md @@ -100,6 +100,7 @@ [function namespaces]: {{rootUrl}}/language/fn-namespaces.md [anonymous function]: {{rootUrl}}/language/fn-anon.md [anonymous functions]: {{rootUrl}}/language/fn-anon.md +[operator overloading]: {{rootUrl}}/rust/operators.md [`Module`]: {{rootUrl}}/language/modules/index.md [module]: {{rootUrl}}/language/modules/index.md diff --git a/examples/repl.rs b/examples/repl.rs index ee73c193..5a76b93c 100644 --- a/examples/repl.rs +++ b/examples/repl.rs @@ -146,7 +146,7 @@ fn main() { #[cfg(not(feature = "no_optimize"))] { - ast = engine.optimize_ast(&scope, r, OptimizationLevel::Full); + ast = engine.optimize_ast(&scope, r, OptimizationLevel::Simple); } #[cfg(feature = "no_optimize")] diff --git a/src/fn_call.rs b/src/fn_call.rs index b78e222f..473a948d 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -439,7 +439,33 @@ impl Engine { } // Has a system function an override? - fn has_override(&self, lib: &Module, hash_fn: u64, hash_script: u64, pub_only: bool) -> bool { + pub(crate) fn has_override_by_name_and_arguments( + &self, + lib: &Module, + name: &str, + arg_types: &[TypeId], + pub_only: bool, + ) -> bool { + let arg_len = if arg_types.is_empty() { + usize::MAX + } else { + arg_types.len() + }; + + let hash_fn = calc_fn_hash(empty(), name, arg_len, arg_types.iter().cloned()); + let hash_script = calc_fn_hash(empty(), name, arg_types.len(), empty()); + + self.has_override(lib, hash_fn, hash_script, pub_only) + } + + // Has a system function an override? + pub(crate) fn has_override( + &self, + lib: &Module, + hash_fn: u64, + hash_script: u64, + pub_only: bool, + ) -> bool { // NOTE: We skip script functions for global_module and packages, and native functions for lib // First check script-defined functions diff --git a/src/module/mod.rs b/src/module/mod.rs index e62ef520..10680b33 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -294,6 +294,23 @@ impl Module { hash_script } + /// Get a script-defined function in the module based on name and number of parameters. + pub fn get_script_fn( + &self, + name: &str, + num_params: usize, + public_only: bool, + ) -> Option<&Shared> { + self.functions + .values() + .find(|(fn_name, access, num, _, _)| { + (!public_only || *access == FnAccess::Public) + && *num == num_params + && fn_name == name + }) + .map(|(_, _, _, _, f)| f.get_shared_fn_def()) + } + /// Does a sub-module exist in the module? /// /// # Examples diff --git a/src/optimize.rs b/src/optimize.rs index 0e916878..c21423ab 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -5,6 +5,7 @@ use crate::calc_fn_hash; use crate::engine::{ Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF, }; +use crate::fn_call::run_builtin_binary_op; use crate::fn_native::FnPtr; use crate::module::Module; use crate::parser::{map_dynamic_to_expr, Expr, ScriptFnDef, Stmt, AST}; @@ -568,58 +569,75 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { } } + // Call built-in functions + Expr::FnCall(mut x) + if x.1.is_none() // Non-qualified + && state.optimization_level == OptimizationLevel::Simple // simple optimizations + && x.3.len() == 2 // binary call + && x.3.iter().all(Expr::is_constant) // all arguments are constants + => { + let ((name, _, _, pos), _, _, args, _) = x.as_mut(); + + let arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); + let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); + + // Search for overloaded operators (can override built-in). + if !state.engine.has_override_by_name_and_arguments(state.lib, name, arg_types.as_ref(), false) { + if let Some(expr) = run_builtin_binary_op(name, &arg_values[0], &arg_values[1]) + .ok().flatten() + .and_then(|result| map_dynamic_to_expr(result, *pos)) + { + state.set_dirty(); + return expr; + } + } + + x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); + Expr::FnCall(x) + } + // Eagerly call functions Expr::FnCall(mut x) if x.1.is_none() // Non-qualified && state.optimization_level == OptimizationLevel::Full // full optimizations - && x.3.iter().all(|expr| expr.is_constant()) // all arguments are constants + && x.3.iter().all(Expr::is_constant) // all arguments are constants => { let ((name, _, _, pos), _, _, args, def_value) = x.as_mut(); - // First search in functions lib (can override built-in) - // Cater for both normal function call style and method call style (one additional arguments) - let has_script_fn = cfg!(not(feature = "no_function")) && state.lib.iter_fn().find(|(_, _, _, _,f)| { - if !f.is_script() { return false; } - let fn_def = f.get_fn_def(); - fn_def.name == name && (args.len()..=args.len() + 1).contains(&fn_def.params.len()) - }).is_some(); + // First search for script-defined functions (can override built-in) + let has_script_fn = cfg!(not(feature = "no_function")) + && state.lib.get_script_fn(name, args.len(), false).is_some(); - if has_script_fn { - // A script-defined function overrides the built-in function - do not make the call - x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); - return Expr::FnCall(x); + if !has_script_fn { + let mut arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); + + // Save the typename of the first argument if it is `type_of()` + // This is to avoid `call_args` being passed into the closure + let arg_for_type_of = if name == KEYWORD_TYPE_OF && arg_values.len() == 1 { + state.engine.map_type_name(arg_values[0].type_name()) + } else { + "" + }; + + if let Some(expr) = call_fn_with_constant_arguments(&state, name, arg_values.as_mut()) + .or_else(|| { + if !arg_for_type_of.is_empty() { + // Handle `type_of()` + Some(arg_for_type_of.to_string().into()) + } else { + // Otherwise use the default value, if any + def_value.map(|v| v.into()) + } + }) + .and_then(|result| map_dynamic_to_expr(result, *pos)) + { + state.set_dirty(); + return expr; + } } - let mut arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); - - // Save the typename of the first argument if it is `type_of()` - // This is to avoid `call_args` being passed into the closure - let arg_for_type_of = if name == KEYWORD_TYPE_OF && arg_values.len() == 1 { - state.engine.map_type_name(arg_values[0].type_name()) - } else { - "" - }; - - call_fn_with_constant_arguments(&state, name, arg_values.as_mut()) - .or_else(|| { - if !arg_for_type_of.is_empty() { - // Handle `type_of()` - Some(arg_for_type_of.to_string().into()) - } else { - // Otherwise use the default value, if any - def_value.map(|v| v.into()) - } - }) - .and_then(|result| map_dynamic_to_expr(result, *pos)) - .map(|expr| { - state.set_dirty(); - expr - }) - .unwrap_or_else(|| { - // Optimize function call arguments - x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); - Expr::FnCall(x) - }) + x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); + Expr::FnCall(x) } // id(args ..) -> optimize function call arguments From 4356d02828c08c9c9b999570b4ce4c50d07f55fb Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 12:05:46 +0800 Subject: [PATCH 17/29] Fix no_object builds. --- src/engine.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/engine.rs b/src/engine.rs index 327ccc8e..231ceb1c 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -233,9 +233,13 @@ impl Target<'_> { } /// Propagate a changed value back to the original source. /// This has no effect except for string indexing. + #[cfg(not(feature = "no_object"))] + #[inline(always)] pub fn propagate_changed_value(&mut self) { match self { - Self::Ref(_) | Self::LockGuard(_) | Self::Value(_) => (), + Self::Ref(_) | Self::Value(_) => (), + #[cfg(not(feature = "no_closure"))] + Self::LockGuard(_) => (), #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, ch) => { let new_val = ch.clone(); From 29bf7902863a633cf3743b6bbf4ca352ce15f662 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 12:09:45 +0800 Subject: [PATCH 18/29] Fix no_function build. --- src/module/mod.rs | 1 + src/optimize.rs | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/module/mod.rs b/src/module/mod.rs index 10680b33..606f7e95 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -295,6 +295,7 @@ impl Module { } /// Get a script-defined function in the module based on name and number of parameters. + #[cfg(not(feature = "no_function"))] pub fn get_script_fn( &self, name: &str, diff --git a/src/optimize.rs b/src/optimize.rs index c21423ab..114aeef7 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -605,8 +605,10 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { let ((name, _, _, pos), _, _, args, def_value) = x.as_mut(); // First search for script-defined functions (can override built-in) - let has_script_fn = cfg!(not(feature = "no_function")) - && state.lib.get_script_fn(name, args.len(), false).is_some(); + #[cfg(not(feature = "no_function"))] + let has_script_fn = state.lib.get_script_fn(name, args.len(), false).is_some(); + #[cfg(feature = "no_function")] + let has_script_fn = false; if !has_script_fn { let mut arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); From b67a743306adbbe3538bd633b9472e58fd41f2ba Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 12:14:34 +0800 Subject: [PATCH 19/29] Do not eagerly evaluate is_def_fn and is_def_var. --- src/optimize.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index 114aeef7..c1b2b0e2 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -3,7 +3,8 @@ use crate::any::Dynamic; use crate::calc_fn_hash; use crate::engine::{ - Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF, + Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_IS_DEF_FN, KEYWORD_IS_DEF_VAR, + KEYWORD_PRINT, KEYWORD_TYPE_OF, }; use crate::fn_call::run_builtin_binary_op; use crate::fn_native::FnPtr; @@ -385,7 +386,13 @@ fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt { /// Optimize an expression. fn optimize_expr(expr: Expr, state: &mut State) -> Expr { // These keywords are handled specially - const DONT_EVAL_KEYWORDS: [&str; 3] = [KEYWORD_PRINT, KEYWORD_DEBUG, KEYWORD_EVAL]; + const DONT_EVAL_KEYWORDS: &[&str] = &[ + KEYWORD_PRINT, + KEYWORD_DEBUG, + KEYWORD_EVAL, + KEYWORD_IS_DEF_FN, + KEYWORD_IS_DEF_VAR, + ]; match expr { // expr - do not promote because there is a reason it is wrapped in an `Expr::Expr` From 82d48df73493e3612227080cd059816e8bb1bfc1 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 13:45:57 +0800 Subject: [PATCH 20/29] Merge data type mismatch errors. --- RELEASES.md | 4 +- src/engine.rs | 129 +++++++++++++++++++++--------------------------- src/fn_call.rs | 39 ++++++--------- src/result.rs | 66 +++++++------------------ tests/syntax.rs | 4 +- 5 files changed, 92 insertions(+), 150 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 394e5e80..4649f7a5 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -11,13 +11,13 @@ Breaking changes * `Module::iter_script_fn_info` is removed and merged into `Module::iter_script_fn`. * The `merge_namespaces` parameter to `Module::eval_ast_as_new` is removed and now defaults to `true`. * `GlobalFileModuleResolver` is removed because its performance gain over the `FileModuleResolver` is no longer very significant. -* `EvalAltResult::ErrorCharMismatch` is renamed to `EvalAltResult::ErrorMismatchDataType`. +* The following `EvalAltResult` variants are removed and merged into `EvalAltResult::ErrorMismatchDataType`: `ErrorCharMismatch`, `ErrorNumericIndexExpr`, `ErrorStringIndexExpr`, `ErrorImportExpr`, `ErrorLogicGuard`, `ErrorBooleanArgMismatch` New features ------------ * `OptimizationLevel::Simple` now eagerly evaluates built-in binary operators of primary types (if not overloaded). -* Added `is_def_var()` to detect if variable is defined and `is_def_fn()` to detect if script function is defined. +* Added `is_def_var()` to detect if variable is defined, and `is_def_fn()` to detect if script function is defined. * Added `Module::get_script_fn` to get a scripted function in a module, if any, based on name and number of parameters. diff --git a/src/engine.rs b/src/engine.rs index 231ceb1c..d4c326ce 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -7,7 +7,7 @@ use crate::fn_native::{Callback, FnPtr}; use crate::module::{Module, ModuleRef}; use crate::optimize::OptimizationLevel; use crate::packages::{Package, PackagesCollection, StandardPackage}; -use crate::parser::{Expr, ReturnType, Stmt}; +use crate::parser::{Expr, ReturnType, Stmt, INT}; use crate::r#unsafe::unsafe_cast_var_name_to_lifetime; use crate::result::EvalAltResult; use crate::scope::{EntryType as ScopeEntryType, Scope}; @@ -33,6 +33,7 @@ use crate::utils::ImmutableString; use crate::any::DynamicWriteLock; use crate::stdlib::{ + any::type_name, boxed::Box, collections::{HashMap, HashSet}, fmt, format, @@ -243,7 +244,7 @@ impl Target<'_> { #[cfg(not(feature = "no_index"))] Self::StringChar(_, _, ch) => { let new_val = ch.clone(); - self.set_value(new_val, Position::none(), Position::none()) + self.set_value((new_val, Position::none()), Position::none()) .unwrap(); } } @@ -251,15 +252,14 @@ impl Target<'_> { /// Update the value of the `Target`. pub fn set_value( &mut self, - new_val: Dynamic, + new_val: (Dynamic, Position), target_pos: Position, - new_pos: Position, ) -> Result<(), Box> { match self { - Self::Ref(r) => **r = new_val, + Self::Ref(r) => **r = new_val.0, #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_object"))] - Self::LockGuard((r, _)) => **r = new_val, + Self::LockGuard((r, _)) => **r = new_val.0, Self::Value(_) => { return EvalAltResult::ErrorAssignmentToUnknownLHS(target_pos).into(); } @@ -268,11 +268,11 @@ impl Target<'_> { let mut s = string.write_lock::().unwrap(); // Replace the character at the specified index position - let new_ch = new_val.as_char().map_err(|err| { + let new_ch = new_val.0.as_char().map_err(|err| { Box::new(EvalAltResult::ErrorMismatchDataType( err.to_string(), "char".to_string(), - new_pos, + new_val.1, )) })?; @@ -713,8 +713,7 @@ impl Engine { idx_values: &mut StaticVec, chain_type: ChainType, level: usize, - new_val: Option, - new_pos: Position, + new_val: Option<(Dynamic, Position)>, ) -> Result<(Dynamic, bool), Box> { if chain_type == ChainType::None { panic!(); @@ -747,7 +746,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, obj_ptr, expr, idx_values, next_chain, level, - new_val, new_pos, + new_val, ) .map_err(|err| err.new_position(*pos)) } @@ -761,7 +760,7 @@ impl Engine { { // Indexed value is a reference - update directly Ok(ref mut obj_ptr) => { - obj_ptr.set_value(new_val.unwrap(), rhs.position(), new_pos)?; + obj_ptr.set_value(new_val.unwrap(), rhs.position())?; None } Err(err) => match *err { @@ -777,7 +776,7 @@ impl Engine { if let Some(mut new_val) = _call_setter { let val = target.as_mut(); let val_type_name = val.type_name(); - let args = &mut [val, &mut idx_val2, &mut new_val]; + let args = &mut [val, &mut idx_val2, &mut new_val.0]; self.exec_fn_call( state, lib, FN_IDX_SET, 0, args, is_ref, true, false, None, &None, @@ -825,7 +824,7 @@ impl Engine { let mut val = self .get_indexed_mut(state, lib, target, index, *pos, true, false, level)?; - val.set_value(new_val.unwrap(), rhs.position(), new_pos)?; + val.set_value(new_val.unwrap(), rhs.position())?; Ok((Default::default(), true)) } // {xxx:map}.id @@ -842,7 +841,7 @@ impl Engine { Expr::Property(x) if new_val.is_some() => { let ((_, _, setter), pos) = x.as_ref(); let mut new_val = new_val; - let mut args = [target.as_mut(), new_val.as_mut().unwrap()]; + let mut args = [target.as_mut(), &mut new_val.as_mut().unwrap().0]; self.exec_fn_call( state, lib, setter, 0, &mut args, is_ref, true, false, None, &None, level, @@ -893,7 +892,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, &mut val, expr, idx_values, next_chain, level, - new_val, new_pos, + new_val, ) .map_err(|err| err.new_position(*pos)) } @@ -927,7 +926,6 @@ impl Engine { next_chain, level, new_val, - new_pos, ) .map_err(|err| err.new_position(*pos))?; @@ -967,7 +965,7 @@ impl Engine { self.eval_dot_index_chain_helper( state, lib, this_ptr, target, expr, idx_values, next_chain, - level, new_val, new_pos, + level, new_val, ) .map_err(|err| err.new_position(*pos)) } @@ -997,8 +995,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, expr: &Expr, level: usize, - new_val: Option, - new_pos: Position, + new_val: Option<(Dynamic, Position)>, ) -> Result> { let ((dot_lhs, dot_rhs, op_pos), chain_type) = match expr { Expr::Index(x) => (x.as_ref(), ChainType::Index), @@ -1034,8 +1031,7 @@ impl Engine { let obj_ptr = &mut target.into(); self.eval_dot_index_chain_helper( - state, lib, &mut None, obj_ptr, dot_rhs, idx_values, chain_type, level, - new_val, new_pos, + state, lib, &mut None, obj_ptr, dot_rhs, idx_values, chain_type, level, new_val, ) .map(|(v, _)| v) .map_err(|err| err.new_position(*op_pos)) @@ -1050,7 +1046,6 @@ impl Engine { let obj_ptr = &mut val.into(); self.eval_dot_index_chain_helper( state, lib, this_ptr, obj_ptr, dot_rhs, idx_values, chain_type, level, new_val, - new_pos, ) .map(|(v, _)| v) .map_err(|err| err.new_position(*op_pos)) @@ -1159,7 +1154,7 @@ impl Engine { // val_array[idx] let index = idx .as_int() - .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_pos))?; + .map_err(|err| self.make_type_mismatch_err::(err, idx_pos))?; let arr_len = arr.len(); @@ -1178,15 +1173,15 @@ impl Engine { Dynamic(Union::Map(map)) => { // val_map[idx] Ok(if _create { - let index = idx - .take_immutable_string() - .map_err(|_| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; + let index = idx.take_immutable_string().map_err(|err| { + self.make_type_mismatch_err::(err, idx_pos) + })?; map.entry(index).or_insert_with(Default::default).into() } else { - let index = idx - .read_lock::() - .ok_or_else(|| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; + let index = idx.read_lock::().ok_or_else(|| { + self.make_type_mismatch_err::("", idx_pos) + })?; map.get_mut(&*index) .map(Target::from) @@ -1200,7 +1195,7 @@ impl Engine { let chars_len = s.chars().count(); let index = idx .as_int() - .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_pos))?; + .map_err(|err| self.make_type_mismatch_err::(err, idx_pos))?; if index >= 0 { let offset = index as usize; @@ -1438,9 +1433,9 @@ impl Engine { let mut rhs_val = self.eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)?; - let (_new_val, _new_pos) = if op.is_empty() { + let _new_val = if op.is_empty() { // Normal assignment - (Some(rhs_val), rhs_expr.position()) + Some((rhs_val, rhs_expr.position())) } else { // Op-assignment - always map to `lhs = lhs op rhs` let op = &op[..op.len() - 1]; // extract operator without = @@ -1456,7 +1451,7 @@ impl Engine { .map(|(v, _)| v) .map_err(|err| err.new_position(*op_pos))?; - (Some(result), rhs_expr.position()) + Some((result, rhs_expr.position())) }; match lhs_expr { @@ -1466,7 +1461,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] Expr::Index(_) => { self.eval_dot_index_chain( - scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, _new_pos, + scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, )?; Ok(Default::default()) } @@ -1474,7 +1469,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] Expr::Dot(_) => { self.eval_dot_index_chain( - scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, _new_pos, + scope, mods, state, lib, this_ptr, lhs_expr, level, _new_val, )?; Ok(Default::default()) } @@ -1491,31 +1486,15 @@ impl Engine { // lhs[idx_expr] #[cfg(not(feature = "no_index"))] - Expr::Index(_) => self.eval_dot_index_chain( - scope, - mods, - state, - lib, - this_ptr, - expr, - level, - None, - Position::none(), - ), + Expr::Index(_) => { + self.eval_dot_index_chain(scope, mods, state, lib, this_ptr, expr, level, None) + } // lhs.dot_rhs #[cfg(not(feature = "no_object"))] - Expr::Dot(_) => self.eval_dot_index_chain( - scope, - mods, - state, - lib, - this_ptr, - expr, - level, - None, - Position::none(), - ), + Expr::Dot(_) => { + self.eval_dot_index_chain(scope, mods, state, lib, this_ptr, expr, level, None) + } #[cfg(not(feature = "no_index"))] Expr::Array(x) => Ok(Dynamic(Union::Array(Box::new( @@ -1562,16 +1541,12 @@ impl Engine { Ok((self .eval_expr(scope, mods, state, lib, this_ptr, lhs, level)? .as_bool() - .map_err(|_| { - EvalAltResult::ErrorBooleanArgMismatch("AND".into(), lhs.position()) - })? + .map_err(|err| self.make_type_mismatch_err::(err, lhs.position()))? && // Short-circuit using && self .eval_expr(scope, mods, state, lib, this_ptr, rhs, level)? .as_bool() - .map_err(|_| { - EvalAltResult::ErrorBooleanArgMismatch("AND".into(), rhs.position()) - })?) + .map_err(|err| self.make_type_mismatch_err::(err, rhs.position()))?) .into()) } @@ -1580,16 +1555,12 @@ impl Engine { Ok((self .eval_expr(scope, mods, state, lib, this_ptr, lhs, level)? .as_bool() - .map_err(|_| { - EvalAltResult::ErrorBooleanArgMismatch("OR".into(), lhs.position()) - })? + .map_err(|err| self.make_type_mismatch_err::(err, lhs.position()))? || // Short-circuit using || self .eval_expr(scope, mods, state, lib, this_ptr, rhs, level)? .as_bool() - .map_err(|_| { - EvalAltResult::ErrorBooleanArgMismatch("OR".into(), rhs.position()) - })?) + .map_err(|err| self.make_type_mismatch_err::(err, rhs.position()))?) .into()) } @@ -1671,7 +1642,7 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .as_bool() - .map_err(|_| EvalAltResult::ErrorLogicGuard(expr.position()).into()) + .map_err(|err| self.make_type_mismatch_err::(err, expr.position())) .and_then(|guard_val| { if guard_val { self.eval_stmt(scope, mods, state, lib, this_ptr, if_block, level) @@ -1704,7 +1675,9 @@ impl Engine { } } Ok(false) => return Ok(Default::default()), - Err(_) => return EvalAltResult::ErrorLogicGuard(expr.position()).into(), + Err(err) => { + return Err(self.make_type_mismatch_err::(err, expr.position())) + } } }, @@ -1873,7 +1846,7 @@ impl Engine { ) } } else { - EvalAltResult::ErrorImportExpr(expr.position()).into() + Err(self.make_type_mismatch_err::("", expr.position())) } } @@ -2068,4 +2041,14 @@ impl Engine { .and_then(|t| t.get(name).map(String::as_str)) .unwrap_or_else(|| map_std_type_name(name)) } + + /// Make a Box>. + pub fn make_type_mismatch_err(&self, typ: &str, pos: Position) -> Box { + EvalAltResult::ErrorMismatchDataType( + typ.into(), + self.map_type_name(type_name::()).into(), + pos, + ) + .into() + } } diff --git a/src/fn_call.rs b/src/fn_call.rs index 473a948d..557a3fd2 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -844,15 +844,6 @@ impl Engine { capture: bool, level: usize, ) -> Result> { - fn make_type_err(engine: &Engine, typ: &str, pos: Position) -> Box { - EvalAltResult::ErrorMismatchDataType( - typ.into(), - engine.map_type_name(type_name::()).into(), - pos, - ) - .into() - } - // Handle Fn() if name == KEYWORD_FN_PTR && args_expr.len() == 1 { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); @@ -864,7 +855,9 @@ impl Engine { return arg_value .take_immutable_string() - .map_err(|typ| make_type_err::(self, typ, expr.position())) + .map_err(|typ| { + self.make_type_mismatch_err::(typ, expr.position()) + }) .and_then(|s| FnPtr::try_from(s)) .map(Into::::into) .map_err(|err| err.new_position(expr.position())); @@ -877,8 +870,7 @@ impl Engine { let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; if !arg_value.is::() { - return Err(make_type_err::( - self, + return Err(self.make_type_mismatch_err::( self.map_type_name(arg_value.type_name()), expr.position(), )); @@ -932,8 +924,7 @@ impl Engine { // Recalculate hash hash_script = calc_fn_hash(empty(), name, curry.len() + args_expr.len(), empty()); } else { - return Err(make_type_err::( - self, + return Err(self.make_type_mismatch_err::( self.map_type_name(arg_value.type_name()), expr.position(), )); @@ -947,9 +938,9 @@ impl Engine { if !self.has_override(lib, hash_fn, hash_script, pub_only) { let expr = args_expr.get(0).unwrap(); let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; - let var_name = arg_value - .as_str() - .map_err(|err| make_type_err::(self, err, expr.position()))?; + let var_name = arg_value.as_str().map_err(|err| { + self.make_type_mismatch_err::(err, expr.position()) + })?; if var_name.is_empty() { return Ok(false.into()); } else { @@ -979,11 +970,11 @@ impl Engine { self.eval_expr(scope, mods, state, lib, this_ptr, num_params_expr, level)?; let fn_name = arg0_value.as_str().map_err(|err| { - make_type_err::(self, err, fn_name_expr.position()) + self.make_type_mismatch_err::(err, fn_name_expr.position()) + })?; + let num_params = arg1_value.as_int().map_err(|err| { + self.make_type_mismatch_err::(err, num_params_expr.position()) })?; - let num_params = arg1_value - .as_int() - .map_err(|err| make_type_err::(self, err, num_params_expr.position()))?; if fn_name.is_empty() || num_params < 0 { return Ok(false.into()); @@ -1003,9 +994,9 @@ impl Engine { let prev_len = scope.len(); let expr = args_expr.get(0).unwrap(); let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; - let script = arg_value - .as_str() - .map_err(|typ| make_type_err::(self, typ, expr.position()))?; + let script = arg_value.as_str().map_err(|typ| { + self.make_type_mismatch_err::(typ, expr.position()) + })?; let result = if !script.is_empty() { self.eval_script_expr(scope, mods, state, lib, script, level + 1) .map_err(|err| err.new_position(expr.position())) diff --git a/src/result.rs b/src/result.rs index 8ad32926..b5a359be 100644 --- a/src/result.rs +++ b/src/result.rs @@ -34,21 +34,26 @@ pub enum EvalAltResult { #[cfg(not(target_arch = "wasm32"))] ErrorReadingScriptFile(PathBuf, Position, std::io::Error), - /// Call to an unknown function. Wrapped value is the signature of the function. + /// 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. ErrorFunctionNotFound(String, Position), /// An error has occurred inside a called function. - /// Wrapped values are the name of the function and the interior error. + /// Wrapped values are the function name and the interior error. ErrorInFunctionCall(String, Box, Position), + /// Usage of an unknown module. Wrapped value is the module name. + ErrorModuleNotFound(String, Position), /// An error has occurred while loading a module. - /// Wrapped value are the name of the module and the interior error. + /// Wrapped value are the module name and the interior error. ErrorInModule(String, Box, Position), /// Access to `this` that is not bound. ErrorUnboundThis(Position), - /// Non-boolean operand encountered for boolean operator. Wrapped value is the operator. - ErrorBooleanArgMismatch(String, Position), /// Data is not of the required type. /// Wrapped values are the type requested and type of the actual result. ErrorMismatchDataType(String, String, Position), + /// Returned type is not the same as the required output type. + /// Wrapped values are the type requested and type of the actual result. + ErrorMismatchOutputType(String, String, Position), /// Array access out-of-bounds. /// Wrapped values are the current number of elements in the array and the index number. ErrorArrayBounds(usize, INT, Position), @@ -56,33 +61,19 @@ pub enum EvalAltResult { /// Wrapped values are the current number of characters in the string and the index number. ErrorStringBounds(usize, INT, Position), /// Trying to index into a type that is not an array, an object map, or a string, and has no indexer function defined. + /// Wrapped value is the type name. ErrorIndexingType(String, Position), - /// Trying to index into an array or string with an index that is not `i64`. - ErrorNumericIndexExpr(Position), - /// Trying to index into a map with an index that is not `String`. - ErrorStringIndexExpr(Position), - /// Trying to import with an expression that is not `String`. - ErrorImportExpr(Position), /// Invalid arguments for `in` operator. ErrorInExpr(Position), - /// The guard expression in an `if` or `while` statement does not return a boolean value. - ErrorLogicGuard(Position), /// The `for` statement encounters a type that is not an iterator. ErrorFor(Position), - /// Usage of an unknown variable. Wrapped value is the name of the variable. - ErrorVariableNotFound(String, Position), - /// Usage of an unknown module. Wrapped value is the name of the module. - ErrorModuleNotFound(String, Position), - /// Data race detected when accessing a variable. Wrapped value is the name of the variable. + /// Data race detected when accessing a variable. Wrapped value is the variable name. ErrorDataRace(String, Position), /// Assignment to an inappropriate LHS (left-hand-side) expression. ErrorAssignmentToUnknownLHS(Position), - /// Assignment to a constant variable. + /// Assignment to a constant variable. Wrapped value is the variable name. ErrorAssignmentToConstant(String, Position), - /// Returned type is not the same as the required output type. - /// Wrapped values are the type requested and type of the actual result. - ErrorMismatchOutputType(String, String, Position), - /// Inappropriate member access. + /// Inappropriate property access. Wrapped value is the property name. ErrorDotExpr(String, Position), /// Arithmetic error encountered. Wrapped value is the error message. ErrorArithmetic(String, Position), @@ -92,7 +83,7 @@ pub enum EvalAltResult { ErrorTooManyModules(Position), /// Call stack over maximum limit. ErrorStackOverflow(Position), - /// Data value over maximum size limit. Wrapped values are the data type, maximum size and current size. + /// Data value over maximum size limit. Wrapped values are the type name, maximum size and current size. ErrorDataTooLarge(String, usize, usize, Position), /// The script is prematurely terminated. ErrorTerminated(Position), @@ -120,16 +111,10 @@ impl EvalAltResult { Self::ErrorInModule(_, _, _) => "Error in module", Self::ErrorFunctionNotFound(_, _) => "Function not found", Self::ErrorUnboundThis(_) => "'this' is not bound", - Self::ErrorBooleanArgMismatch(_, _) => "Boolean operator expects boolean operands", Self::ErrorMismatchDataType(_, _, _) => "Data type is incorrect", - Self::ErrorNumericIndexExpr(_) => { - "Indexing into an array or string expects an integer index" - } - Self::ErrorStringIndexExpr(_) => "Indexing into an object map expects a string index", Self::ErrorIndexingType(_, _) => { "Indexing can only be performed on an array, an object map, a string, or a type with an indexer function defined" } - Self::ErrorImportExpr(_) => "Importing a module expects a string path", Self::ErrorArrayBounds(_, index, _) if *index < 0 => { "Array access expects non-negative index" } @@ -140,7 +125,6 @@ impl EvalAltResult { } Self::ErrorStringBounds(0, _, _) => "Empty string has nothing to index", Self::ErrorStringBounds(_, _, _) => "String index out of bounds", - Self::ErrorLogicGuard(_) => "Boolean value expected", Self::ErrorFor(_) => "For loop expects an array, object map, or range", Self::ErrorVariableNotFound(_, _) => "Variable not found", Self::ErrorModuleNotFound(_, _) => "Module not found", @@ -198,11 +182,7 @@ impl fmt::Display for EvalAltResult { Self::ErrorDotExpr(s, _) if !s.is_empty() => write!(f, "{}", s)?, Self::ErrorIndexingType(_, _) - | Self::ErrorNumericIndexExpr(_) - | Self::ErrorStringIndexExpr(_) | Self::ErrorUnboundThis(_) - | Self::ErrorImportExpr(_) - | Self::ErrorLogicGuard(_) | Self::ErrorFor(_) | Self::ErrorAssignmentToUnknownLHS(_) | Self::ErrorInExpr(_) @@ -218,6 +198,9 @@ impl fmt::Display for EvalAltResult { Self::ErrorMismatchOutputType(r, s, _) => { write!(f, "Output type is incorrect: {} (expecting {})", r, s)? } + Self::ErrorMismatchDataType(r, s, _) if r.is_empty() => { + write!(f, "Data type is incorrect, expecting {}", s)? + } Self::ErrorMismatchDataType(r, s, _) => { write!(f, "Data type is incorrect: {} (expecting {})", r, s)? } @@ -226,9 +209,6 @@ impl fmt::Display for EvalAltResult { Self::ErrorLoopBreak(_, _) => f.write_str(desc)?, Self::Return(_, _) => f.write_str(desc)?, - Self::ErrorBooleanArgMismatch(op, _) => { - write!(f, "{} operator expects boolean operands", op)? - } Self::ErrorArrayBounds(_, index, _) if *index < 0 => { write!(f, "{}: {} < 0", desc, index)? } @@ -293,15 +273,10 @@ impl EvalAltResult { | Self::ErrorInFunctionCall(_, _, pos) | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) - | Self::ErrorBooleanArgMismatch(_, pos) | Self::ErrorMismatchDataType(_, _, pos) | Self::ErrorArrayBounds(_, _, pos) | Self::ErrorStringBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) - | Self::ErrorNumericIndexExpr(pos) - | Self::ErrorStringIndexExpr(pos) - | Self::ErrorImportExpr(pos) - | Self::ErrorLogicGuard(pos) | Self::ErrorFor(pos) | Self::ErrorVariableNotFound(_, pos) | Self::ErrorModuleNotFound(_, pos) @@ -335,15 +310,10 @@ impl EvalAltResult { | Self::ErrorInFunctionCall(_, _, pos) | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) - | Self::ErrorBooleanArgMismatch(_, pos) | Self::ErrorMismatchDataType(_, _, pos) | Self::ErrorArrayBounds(_, _, pos) | Self::ErrorStringBounds(_, _, pos) | Self::ErrorIndexingType(_, pos) - | Self::ErrorNumericIndexExpr(pos) - | Self::ErrorStringIndexExpr(pos) - | Self::ErrorImportExpr(pos) - | Self::ErrorLogicGuard(pos) | Self::ErrorFor(pos) | Self::ErrorVariableNotFound(_, pos) | Self::ErrorModuleNotFound(_, pos) diff --git a/tests/syntax.rs b/tests/syntax.rs index a0cfa5b1..0f0090f9 100644 --- a/tests/syntax.rs +++ b/tests/syntax.rs @@ -35,9 +35,7 @@ fn test_custom_syntax() -> Result<(), Box> { if !engine .eval_expression_tree(context, scope, expr)? .as_bool() - .map_err(|_| { - EvalAltResult::ErrorBooleanArgMismatch("do-while".into(), expr.position()) - })? + .map_err(|err| engine.make_type_mismatch_err::(err, expr.position()))? { break; } From 2f6bb643aa1965cd5bf0d69338ca06aa9b6145e0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 18:07:40 +0800 Subject: [PATCH 21/29] Remove Module::get_script_function_by_signature. --- RELEASES.md | 2 +- src/api.rs | 2 +- src/module/mod.rs | 18 ------------------ 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 4649f7a5..a980377e 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -9,6 +9,7 @@ Breaking changes * `AST::iter_functions` now returns an iterator instead of taking a closure. * `Module::iter_script_fn_info` is removed and merged into `Module::iter_script_fn`. +* `Module::get_script_function_by_signature` renamed to `Module::get_script_fn` and returns `&>`. * The `merge_namespaces` parameter to `Module::eval_ast_as_new` is removed and now defaults to `true`. * `GlobalFileModuleResolver` is removed because its performance gain over the `FileModuleResolver` is no longer very significant. * The following `EvalAltResult` variants are removed and merged into `EvalAltResult::ErrorMismatchDataType`: `ErrorCharMismatch`, `ErrorNumericIndexExpr`, `ErrorStringIndexExpr`, `ErrorImportExpr`, `ErrorLogicGuard`, `ErrorBooleanArgMismatch` @@ -18,7 +19,6 @@ New features * `OptimizationLevel::Simple` now eagerly evaluates built-in binary operators of primary types (if not overloaded). * Added `is_def_var()` to detect if variable is defined, and `is_def_fn()` to detect if script function is defined. -* Added `Module::get_script_fn` to get a scripted function in a module, if any, based on name and number of parameters. Version 0.19.0 diff --git a/src/api.rs b/src/api.rs index b24b3b95..4e772f27 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1596,7 +1596,7 @@ impl Engine { args: &mut [&mut Dynamic], ) -> FuncReturn { let fn_def = lib - .get_script_function_by_signature(name, args.len(), true) + .get_script_fn(name, args.len(), true) .ok_or_else(|| EvalAltResult::ErrorFunctionNotFound(name.into(), Position::none()))?; let mut state = State::new(); diff --git a/src/module/mod.rs b/src/module/mod.rs index 606f7e95..47bfc398 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1103,24 +1103,6 @@ impl Module { } } - /// Get a script-defined function definition from a module. - #[cfg(not(feature = "no_function"))] - pub fn get_script_function_by_signature( - &self, - name: &str, - num_params: usize, - pub_only: bool, - ) -> Option<&ScriptFnDef> { - // Qualifiers (none) + function name + number of arguments. - let hash_script = calc_fn_hash(empty(), name, num_params, empty()); - let func = self.get_fn(hash_script, pub_only)?; - if func.is_script() { - Some(func.get_fn_def()) - } else { - None - } - } - /// Get a modules-qualified function. /// Name and Position in `EvalAltResult` are None and must be set afterwards. /// From 44f8d9e429096f2a1fe9e071d3dd28afa5cf42c8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 21:52:39 +0800 Subject: [PATCH 22/29] Refine Module::iter_script_fn_info. --- RELEASES.md | 2 +- src/fn_call.rs | 2 +- src/module/mod.rs | 58 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index a980377e..95ee14e7 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -8,8 +8,8 @@ Breaking changes ---------------- * `AST::iter_functions` now returns an iterator instead of taking a closure. -* `Module::iter_script_fn_info` is removed and merged into `Module::iter_script_fn`. * `Module::get_script_function_by_signature` renamed to `Module::get_script_fn` and returns `&>`. +* `Module::num_fn`, `Module::num_var` and `Module::num_iter` are removed and merged into `Module::count`. * The `merge_namespaces` parameter to `Module::eval_ast_as_new` is removed and now defaults to `true`. * `GlobalFileModuleResolver` is removed because its performance gain over the `FileModuleResolver` is no longer very significant. * The following `EvalAltResult` variants are removed and merged into `EvalAltResult::ErrorMismatchDataType`: `ErrorCharMismatch`, `ErrorNumericIndexExpr`, `ErrorStringIndexExpr`, `ErrorImportExpr`, `ErrorLogicGuard`, `ErrorBooleanArgMismatch` diff --git a/src/fn_call.rs b/src/fn_call.rs index 557a3fd2..c9439637 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -663,7 +663,7 @@ impl Engine { )?; // If new functions are defined within the eval string, it is an error - if ast.lib().num_fn() != 0 { + if ast.lib().count().0 != 0 { return Err(ParseErrorType::WrongFnDefinition.into()); } diff --git a/src/module/mod.rs b/src/module/mod.rs index 47bfc398..6d9681e6 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1228,17 +1228,13 @@ impl Module { self } - /// Get the number of variables in the module. - pub fn num_var(&self) -> usize { - self.variables.len() - } - /// Get the number of functions in the module. - pub fn num_fn(&self) -> usize { - self.variables.len() - } - /// Get the number of type iterators in the module. - pub fn num_iter(&self) -> usize { - self.variables.len() + /// Get the number of variables, functions and type iterators in the module. + pub fn count(&self) -> (usize, usize, usize) { + ( + self.variables.len(), + self.variables.len(), + self.variables.len(), + ) } /// Get an iterator to the variables in the module. @@ -1252,8 +1248,14 @@ impl Module { } /// Get an iterator over all script-defined functions in the module. + /// + /// Function metadata includes: + /// 1) Access mode (`FnAccess::Public` or `FnAccess::Private`). + /// 2) Function name (as string slice). + /// 3) Number of parameters. + /// 4) Shared reference to function definition `ScriptFnDef`. #[cfg(not(feature = "no_function"))] - pub fn iter_script_fn<'a>( + pub(crate) fn iter_script_fn<'a>( &'a self, ) -> impl Iterator)> + 'a { self.functions @@ -1267,6 +1269,38 @@ impl Module { }) } + /// Get an iterator over all script-defined functions in the module. + /// + /// Function metadata includes: + /// 1) Access mode (`FnAccess::Public` or `FnAccess::Private`). + /// 2) Function name (as string slice). + /// 3) Number of parameters. + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "internals"))] + pub fn iter_script_fn_info(&self) -> impl Iterator { + self.functions + .values() + .filter(|(_, _, _, _, f)| f.is_script()) + .map(|(name, access, num_params, _, _)| (*access, name.as_str(), *num_params)) + } + + /// Get an iterator over all script-defined functions in the module. + /// + /// Function metadata includes: + /// 1) Access mode (`FnAccess::Public` or `FnAccess::Private`). + /// 2) Function name (as string slice). + /// 3) Number of parameters. + /// 4) _[INTERNALS]_ Shared reference to function definition `ScriptFnDef`. + /// Exported under the internals feature only. + #[cfg(not(feature = "no_function"))] + #[cfg(feature = "internals")] + #[inline(always)] + pub fn iter_script_fn_info( + &self, + ) -> impl Iterator)> { + self.iter_script_fn() + } + /// Create a new `Module` by evaluating an `AST`. /// /// The entire `AST` is encapsulated into each function, allowing functions From 1de44c7ecd67cc3618649a36b2c58ce6d1673ac0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 23:02:50 +0800 Subject: [PATCH 23/29] Reserve $ symbol. --- doc/src/appendix/literals.md | 2 +- doc/src/appendix/operators.md | 96 +++++++++++++++++++------------- doc/src/engine/optimize/index.md | 26 +++++---- doc/src/start/features.md | 2 +- src/token.rs | 2 + 5 files changed, 76 insertions(+), 52 deletions(-) diff --git a/doc/src/appendix/literals.md b/doc/src/appendix/literals.md index 2d51ba36..3e5a5fdf 100644 --- a/doc/src/appendix/literals.md +++ b/doc/src/appendix/literals.md @@ -8,7 +8,7 @@ Literals Syntax | `INT` | decimal: `42`, `-123`, `0`
hex: `0x????..`
binary: `0b????..`
octal: `0o????..` | | `FLOAT` | `42.0`, `-123.456`, `0.0` | | [String] | `"... \x?? \u???? \U???????? ..."` | -| Character | single: `'?'`
ASCII hex: `'\x??'`
Unicode: `'\u????'`, `'\U????????'` | +| [Character][string] | single: `'?'`
ASCII hex: `'\x??'`
Unicode: `'\u????'`, `'\U????????'` | | [`Array`] | `[ ???, ???, ??? ]` | | [Object map] | `#{ a: ???, b: ???, c: ???, "def": ??? }` | | Boolean true | `true` | diff --git a/doc/src/appendix/operators.md b/doc/src/appendix/operators.md index e53ba01b..c0fa4292 100644 --- a/doc/src/appendix/operators.md +++ b/doc/src/appendix/operators.md @@ -7,46 +7,62 @@ Operators and Symbols Operators --------- -| Operator | Description | Binary? | Binding direction | -| :---------------: | ------------------------------ | :-----: | :---------------: | -| `+` | add | yes | left | -| `-` | subtract, Minus | yes/no | left | -| `*` | multiply | yes | left | -| `/` | divide | yes | left | -| `%` | modulo | yes | left | -| `~` | power | yes | left | -| `>>` | right bit-shift | yes | left | -| `<<` | left bit-shift | yes | left | -| `&` | bit-wise _And_, boolean _And_ | yes | left | -| \| | bit-wise _Or_, boolean _Or_ | yes | left | -| `^` | bit-wise _Xor_, boolean _Xor_ | yes | left | -| `==` | equals to | yes | left | -| `~=` | not equals to | yes | left | -| `>` | greater than | yes | left | -| `>=` | greater than or equals to | yes | left | -| `<` | less than | yes | left | -| `<=` | less than or equals to | yes | left | -| `&&` | boolean _And_ (short-circuits) | yes | left | -| \|\| | boolean _Or_ (short-circuits) | yes | left | -| `!` | boolean _Not_ | no | left | -| `[` .. `]` | indexing | yes | right | -| `.` | property access, method call | yes | right | +| Operator | Description | Binary? | Binding direction | +| :-----------------------------------------------------------------------------------------: | -------------------------------------- | :--------: | :---------------: | +| `+` | add | yes | left | +| `-` | 1) subtract
2) negative | yes
no | left
right | +| `*` | multiply | yes | left | +| `/` | divide | yes | left | +| `%` | modulo | yes | left | +| `~` | power | yes | left | +| `>>` | right bit-shift | yes | left | +| `<<` | left bit-shift | yes | left | +| `&` | 1) bit-wise _And_
2) boolean _And_ | yes | left | +| \| | 1) bit-wise _Or_
2) boolean _Or_ | yes | left | +| `^` | 1) bit-wise _Xor_
2) boolean _Xor_ | yes | left | +| `=`, `+=`, `-=`, `*=`, `/=`,
`~=`, `%=`, `<<=`, `>>=`, `&=`,
\|=, `^=` | assignments | yes | right | +| `==` | equals to | yes | left | +| `~=` | not equals to | yes | left | +| `>` | greater than | yes | left | +| `>=` | greater than or equals to | yes | left | +| `<` | less than | yes | left | +| `<=` | less than or equals to | yes | left | +| `&&` | boolean _And_ (short-circuits) | yes | left | +| \|\| | boolean _Or_ (short-circuits) | yes | left | +| `!` | boolean _Not_ | no | left | +| `[` .. `]` | indexing | yes | right | +| `.` | 1) property access
2) method call | yes | right | -Symbols -------- +Symbols and Patterns +-------------------- -| Symbol | Description | -| ------------ | ------------------------ | -| `:` | property value separator | -| `::` | module path separator | -| `#` | _reserved_ | -| `=>` | _reserved_ | -| `->` | _reserved_ | -| `<-` | _reserved_ | -| `===` | _reserved_ | -| `!==` | _reserved_ | -| `:=` | _reserved_ | -| `::<` .. `>` | _reserved_ | -| `@` | _reserved_ | -| `(*` .. `*)` | _reserved_ | +| Symbol | Name | Description | +| ---------------------------------- | :------------------: | ------------------------------------- | +| `;` | semicolon | statement separator | +| `,` | comma | list separator | +| `:` | colon | [object map] property value separator | +| `::` | path | module path separator | +| `#{` .. `}` | hash map | [object map] literal | +| `"` .. `"` | double quote | [string] | +| `'` .. `'` | single quote | [character][string] | +| `\` | escape | escape character literal | +| `(` .. `)` | parentheses | expression grouping | +| `{` .. `}` | braces | block statement | +| \| .. \| | pipes | closure | +| `[` .. `]` | brackets | [array] literal | +| `!` | bang | function call in calling scope | +| `//` | comment | line comment | +| `/*` .. `*/` | comment | block comment | +| `(*` .. `*)` | comment | _reserved_ | +| `<` .. `>` | angular brackets | _reserved_ | +| `#` | hash | _reserved_ | +| `@` | at | _reserved_ | +| `$` | dollar | _reserved_ | +| `=>` | double arrow | _reserved_ | +| `->` | arrow | _reserved_ | +| `<-` | left arrow | _reserved_ | +| `===` | strict equals to | _reserved_ | +| `!==` | strict not equals to | _reserved_ | +| `:=` | assignment | _reserved_ | +| `::<` .. `>` | turbofish | _reserved_ | diff --git a/doc/src/engine/optimize/index.md b/doc/src/engine/optimize/index.md index 6f293181..59ae0f6a 100644 --- a/doc/src/engine/optimize/index.md +++ b/doc/src/engine/optimize/index.md @@ -47,9 +47,13 @@ Constants propagation is used to remove dead code: ```rust const ABC = true; + if ABC || some_work() { print("done!"); } // 'ABC' is constant so it is replaced by 'true'... + if true || some_work() { print("done!"); } // since '||' short-circuits, 'some_work' is never called + if true { print("done!"); } // <- the line above is equivalent to this + print("done!"); // <- the line above is further simplified to this // because the condition is always true ``` @@ -84,13 +88,15 @@ Rhai guarantees that no external function will be run (in order not to trigger s optimization process (unless the optimization level is set to [`OptimizationLevel::Full`]). ```rust -const DECISION = 1; // this is an integer, one of the standard types +// The following is most likely generated by machine. -if DECISION == 1 { // this is optimized into 'true' +const DECISION = 1; // this is an integer, one of the standard types + +if DECISION == 1 { // this is optimized into 'true' : -} else if DECISION == 2 { // this is optimized into 'false' +} else if DECISION == 2 { // this is optimized into 'false' : -} else if DECISION == 3 { // this is optimized into 'false' +} else if DECISION == 3 { // this is optimized into 'false' : } else { : @@ -101,9 +107,9 @@ Because of the eager evaluation of operators for [standard types], many constant and replaced by the result. ```rust -let x = (1+2)*3-4/5%6; // will be replaced by 'let x = 9' +let x = (1+2) * 3-4 / 5%6; // will be replaced by 'let x = 9' -let y = (1>2) || (3<=4); // will be replaced by 'let y = true' +let y = (1 > 2) || (3 < =4); // will be replaced by 'let y = true' ``` For operators that are not optimized away due to one of the above reasons, the function calls @@ -116,12 +122,12 @@ const DECISION_1 = new_state(1); const DECISION_2 = new_state(2); const DECISION_3 = new_state(3); -if DECISION == 1 { // NOT optimized away because the operator is not built-in - : // and may cause side-effects if called! +if DECISION == 1 { // NOT optimized away because the operator is not built-in + : // and may cause side-effects if called! : -} else if DECISION == 2 { // same here, NOT optimized away +} else if DECISION == 2 { // same here, NOT optimized away : -} else if DECISION == 3 { // same here, NOT optimized away +} else if DECISION == 3 { // same here, NOT optimized away : } else { : diff --git a/doc/src/start/features.md b/doc/src/start/features.md index baf1d176..e82b1df1 100644 --- a/doc/src/start/features.md +++ b/doc/src/start/features.md @@ -26,8 +26,8 @@ more control over what a script can (or cannot) do. | `no_closure` | no | disables [capturing][automatic currying] external variables in [anonymous functions] to simulate _closures_, or [capturing the calling scope]({{rootUrl}}/language/fn-capture.md) in function calls | | `no_std` | no | builds for `no-std` (implies `no_closure`). Notice that additional dependencies will be pulled in to replace `std` features | | `serde` | yes | enables serialization/deserialization via `serde`. Notice that the [`serde`](https://crates.io/crates/serde) crate will be pulled in together with its dependencies | -| `internals` | yes | exposes internal data structures (e.g. [`AST`] nodes). Beware that Rhai internals are volatile and may change from version to version | | `unicode-xid-ident` | no | allows [Unicode Standard Annex #31](http://www.unicode.org/reports/tr31/) as identifiers | +| `internals` | yes | exposes internal data structures (e.g. [`AST`] nodes). Beware that Rhai internals are volatile and may change from version to version | Example diff --git a/src/token.rs b/src/token.rs index 3494b02e..97a65862 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1391,6 +1391,8 @@ fn get_next_token_inner( ('@', _) => return Some((Token::Reserved("@".into()), start_pos)), + ('$', _) => return Some((Token::Reserved("$".into()), start_pos)), + ('\0', _) => unreachable!(), (ch, _) if ch.is_whitespace() => (), From 8809d25d3cbb8d38d23a930d9cba48c13d80942e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 6 Oct 2020 21:25:05 +0800 Subject: [PATCH 24/29] Add Dynamic::from(&str) --- RELEASES.md | 1 + src/any.rs | 6 ++++++ tests/string.rs | 17 ++++++++++++++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/RELEASES.md b/RELEASES.md index 95ee14e7..ba52e9cc 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -19,6 +19,7 @@ New features * `OptimizationLevel::Simple` now eagerly evaluates built-in binary operators of primary types (if not overloaded). * Added `is_def_var()` to detect if variable is defined, and `is_def_fn()` to detect if script function is defined. +* `Dynamic::from(&str)` now constructs a `Dynamic` with a copy of the string as value. Version 0.19.0 diff --git a/src/any.rs b/src/any.rs index 10133b43..2ef41da6 100644 --- a/src/any.rs +++ b/src/any.rs @@ -572,6 +572,12 @@ impl Dynamic { .clone() .into(); } + if TypeId::of::() == TypeId::of::<&str>() { + return ::downcast_ref::<&str>(&value) + .unwrap() + .to_string() + .into(); + } if TypeId::of::() == TypeId::of::<()>() { return ().into(); } diff --git a/tests/string.rs b/tests/string.rs index 13f78640..cd4aa6cb 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, ImmutableString, RegisterFn, INT}; +use rhai::{Dynamic, Engine, EvalAltResult, ImmutableString, RegisterFn, Scope, INT}; #[test] fn test_string() -> Result<(), Box> { @@ -49,6 +49,21 @@ fn test_string() -> Result<(), Box> { Ok(()) } +#[test] +fn test_string_dynamic() -> Result<(), Box> { + let engine = Engine::new(); + let mut scope = Scope::new(); + scope.push("x", Dynamic::from("foo")); + scope.push("y", String::from("foo")); + scope.push("z", "foo"); + + assert!(engine.eval_with_scope::(&mut scope, r#"x == "foo""#)?); + assert!(engine.eval_with_scope::(&mut scope, r#"y == "foo""#)?); + assert!(engine.eval_with_scope::(&mut scope, r#"z == "foo""#)?); + + Ok(()) +} + #[cfg(not(feature = "no_object"))] #[test] fn test_string_substring() -> Result<(), Box> { From ae1157a140bc19cc105e7b7014f1cb62f1f2907b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 6 Oct 2020 22:09:26 +0800 Subject: [PATCH 25/29] Remove Expr::get_constant_str and change Expr::get_constant_value not to panic. --- src/engine.rs | 11 +++---- src/optimize.rs | 4 +-- src/parser.rs | 85 ++++++++++++++++++++++++++----------------------- 3 files changed, 52 insertions(+), 48 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index d4c326ce..63fb15ea 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1454,8 +1454,9 @@ impl Engine { Some((result, rhs_expr.position())) }; + // Must be either `var[index] op= val` or `var.prop op= val` match lhs_expr { - // name op= rhs + // name op= rhs (handled above) Expr::Variable(_) => unreachable!(), // idx_lhs[idx_expr] op= rhs #[cfg(not(feature = "no_index"))] @@ -1473,12 +1474,8 @@ impl Engine { )?; Ok(Default::default()) } - // Error assignment to constant - expr if expr.is_constant() => EvalAltResult::ErrorAssignmentToConstant( - expr.get_constant_str(), - expr.position(), - ) - .into(), + // Constant expression (should be caught during parsing) + expr if expr.is_constant() => unreachable!(), // Syntax error expr => EvalAltResult::ErrorAssignmentToUnknownLHS(expr.position()).into(), } diff --git a/src/optimize.rs b/src/optimize.rs index c1b2b0e2..73c5e015 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -585,7 +585,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { => { let ((name, _, _, pos), _, _, args, _) = x.as_mut(); - let arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); + let arg_values: StaticVec<_> = args.iter().map(|e| e.get_constant_value().unwrap()).collect(); let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); // Search for overloaded operators (can override built-in). @@ -618,7 +618,7 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { let has_script_fn = false; if !has_script_fn { - let mut arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); + let mut arg_values: StaticVec<_> = args.iter().map(|e| e.get_constant_value().unwrap()).collect(); // Save the typename of the first argument if it is `type_of()` // This is to avoid `call_args` being passed into the closure diff --git a/src/parser.rs b/src/parser.rs index f85215d4..a4dd3b1c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -12,13 +12,17 @@ use crate::syntax::FnCustomSyntaxEval; use crate::token::{is_keyword_function, is_valid_identifier, Position, Token, TokenStream}; use crate::utils::{StaticVec, StraightHasherBuilder}; +#[cfg(not(feature = "no_index"))] +use crate::engine::Array; + +#[cfg(not(feature = "no_object"))] +use crate::engine::{make_getter, make_setter, Map}; + #[cfg(not(feature = "no_function"))] use crate::engine::{FN_ANONYMOUS, KEYWORD_FN_PTR_CURRY}; -#[cfg(not(feature = "no_object"))] -use crate::engine::{make_getter, make_setter}; - use crate::stdlib::{ + any::TypeId, borrow::Cow, boxed::Box, char, @@ -847,14 +851,40 @@ impl Default for Expr { } impl Expr { + /// Get the type of an expression. + /// + /// Returns `None` if the expression's result type is not constant. + pub fn get_type_id(&self) -> Option { + Some(match self { + Self::Expr(x) => return x.get_type_id(), + + Self::IntegerConstant(_) => TypeId::of::(), + #[cfg(not(feature = "no_float"))] + Self::FloatConstant(_) => TypeId::of::(), + Self::CharConstant(_) => TypeId::of::(), + Self::StringConstant(_) => TypeId::of::(), + Self::FnPointer(_) => TypeId::of::(), + Self::True(_) | Self::False(_) | Self::In(_) | Self::And(_) | Self::Or(_) => { + TypeId::of::() + } + Self::Unit(_) => TypeId::of::<()>(), + + #[cfg(not(feature = "no_index"))] + Self::Array(_) => TypeId::of::(), + + #[cfg(not(feature = "no_object"))] + Self::Map(_) => TypeId::of::(), + + _ => return None, + }) + } + /// Get the `Dynamic` value of a constant expression. /// - /// # Panics - /// - /// Panics when the expression is not constant. - pub fn get_constant_value(&self) -> Dynamic { - match self { - Self::Expr(x) => x.get_constant_value(), + /// Returns `None` if the expression is not constant. + pub fn get_constant_value(&self) -> Option { + Some(match self { + Self::Expr(x) => return x.get_constant_value(), Self::IntegerConstant(x) => x.0.into(), #[cfg(not(feature = "no_float"))] @@ -871,45 +901,22 @@ impl Expr { #[cfg(not(feature = "no_index"))] Self::Array(x) if x.0.iter().all(Self::is_constant) => Dynamic(Union::Array(Box::new( - x.0.iter().map(Self::get_constant_value).collect::>(), + x.0.iter() + .map(|v| v.get_constant_value().unwrap()) + .collect(), ))), #[cfg(not(feature = "no_object"))] Self::Map(x) if x.0.iter().all(|(_, v)| v.is_constant()) => { Dynamic(Union::Map(Box::new( x.0.iter() - .map(|((k, _), v)| (k.clone(), v.get_constant_value())) - .collect::>(), + .map(|((k, _), v)| (k.clone(), v.get_constant_value().unwrap())) + .collect(), ))) } - _ => unreachable!("cannot get value of non-constant expression"), - } - } - - /// Get the display value of a constant expression. - /// - /// # Panics - /// - /// Panics when the expression is not constant. - pub fn get_constant_str(&self) -> String { - match self { - Self::Expr(x) => x.get_constant_str(), - - #[cfg(not(feature = "no_float"))] - Self::FloatConstant(x) => x.0.to_string(), - - Self::IntegerConstant(x) => x.0.to_string(), - Self::CharConstant(x) => x.0.to_string(), - Self::StringConstant(_) => "string".to_string(), - Self::True(_) => "true".to_string(), - Self::False(_) => "false".to_string(), - Self::Unit(_) => "()".to_string(), - - Self::Array(x) if x.0.iter().all(Self::is_constant) => "array".to_string(), - - _ => unreachable!("cannot get value of non-constant expression"), - } + _ => return None, + }) } /// Get the `Position` of the expression. From 762072685d33c6f28b30e48f2c0752c26c1d0733 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 6 Oct 2020 22:35:27 +0800 Subject: [PATCH 26/29] Move script optimization into separate section. --- doc/src/SUMMARY.md | 43 +++++++++++++++--------------- doc/src/advanced.md | 2 -- doc/src/language/modules/export.md | 10 +++++++ doc/src/language/modules/import.md | 6 +++++ doc/src/language/modules/index.md | 12 +++------ doc/src/links.md | 6 ++--- doc/src/rust/modules/ast.md | 5 +++- doc/src/rust/modules/index.md | 14 ++++++++++ doc/src/rust/modules/resolvers.md | 2 ++ 9 files changed, 65 insertions(+), 35 deletions(-) create mode 100644 doc/src/rust/modules/index.md diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index f8e279cc..9b6f1fc5 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -45,7 +45,12 @@ The Rhai Scripting Language 1. [Built-in Packages](rust/packages/builtin.md) 2. [Load a Plugin Module as a Package](rust/packages/plugin.md) 3. [Manually Create a Custom Package](rust/packages/create.md) - 10. [Plugins](plugins/index.md) + 10. [Modules](rust/modules/index.md) + 1. [Create from Rust](rust/modules/create.md) + 2. [Create from AST](rust/modules/ast.md) + 3. [Module Resolvers](rust/modules/resolvers.md) + 1. [Custom Implementation](rust/modules/imp-resolver.md) + 11. [Plugins](plugins/index.md) 1. [Export a Rust Module](plugins/module.md) 2. [Export a Rust Function](plugins/function.md) 5. [Rhai Language Reference](language/index.md) @@ -89,10 +94,6 @@ The Rhai Scripting Language 17. [Modules](language/modules/index.md) 1. [Export Variables, Functions and Sub-Modules](language/modules/export.md) 2. [Import Modules](language/modules/import.md) - 3. [Create from Rust](rust/modules/create.md) - 4. [Create from AST](rust/modules/ast.md) - 5. [Module Resolvers](rust/modules/resolvers.md) - 1. [Custom Implementation](rust/modules/imp-resolver.md) 18. [Eval Statement](language/eval.md) 6. [Safety and Protection](safety/index.md) 1. [Checked Arithmetic](safety/checked.md) @@ -105,29 +106,29 @@ The Rhai Scripting Language 7. [Maximum Number of Modules](safety/max-modules.md) 8. [Maximum Call Stack Depth](safety/max-call-stack.md) 9. [Maximum Statement Depth](safety/max-stmt-depth.md) -7. [Usage Patterns](patterns/index.md) +7. [Script Optimization](engine/optimize/index.md) + 1. [Optimization Levels](engine/optimize/optimize-levels.md) + 2. [Re-Optimize an AST](engine/optimize/reoptimize.md) + 3. [Eager Function Evaluation](engine/optimize/eager.md) + 4. [Side-Effect Considerations](engine/optimize/side-effects.md) + 5. [Volatility Considerations](engine/optimize/volatility.md) + 6. [Subtle Semantic Changes](engine/optimize/semantics.md) +8. [Usage Patterns](patterns/index.md) 1. [Object-Oriented Programming (OOP)](patterns/oop.md) 2. [Loadable Configuration](patterns/config.md) 3. [Control Layer](patterns/control.md) 4. [Singleton Command](patterns/singleton.md) 5. [One Engine Instance Per Call](patterns/parallel.md) 6. [Scriptable Event Handler with State](patterns/events.md) -8. [Advanced Topics](advanced.md) +9. [Advanced Topics](advanced.md) 1. [Capture Scope for Function Call](language/fn-capture.md) - 2. [Script Optimization](engine/optimize/index.md) - 1. [Optimization Levels](engine/optimize/optimize-levels.md) - 2. [Re-Optimize an AST](engine/optimize/reoptimize.md) - 3. [Eager Function Evaluation](engine/optimize/eager.md) - 4. [Side-Effect Considerations](engine/optimize/side-effects.md) - 5. [Volatility Considerations](engine/optimize/volatility.md) - 6. [Subtle Semantic Changes](engine/optimize/semantics.md) - 3. [Low-Level API](rust/register-raw.md) - 4. [Use as DSL](engine/dsl.md) + 2. [Low-Level API](rust/register-raw.md) + 3. [Use as DSL](engine/dsl.md) 1. [Disable Keywords and/or Operators](engine/disable.md) 2. [Custom Operators](engine/custom-op.md) 3. [Extending with Custom Syntax](engine/custom-syntax.md) - 5. [Multiple Instantiation](patterns/multiple.md) -9. [Appendix](appendix/index.md) - 1. [Keywords](appendix/keywords.md) - 2. [Operators and Symbols](appendix/operators.md) - 3. [Literals](appendix/literals.md) + 4. [Multiple Instantiation](patterns/multiple.md) +10. [Appendix](appendix/index.md) + 1. [Keywords](appendix/keywords.md) + 2. [Operators and Symbols](appendix/operators.md) + 3. [Literals](appendix/literals.md) diff --git a/doc/src/advanced.md b/doc/src/advanced.md index 482ae621..7ed51620 100644 --- a/doc/src/advanced.md +++ b/doc/src/advanced.md @@ -11,8 +11,6 @@ This section covers advanced features such as: * [`serde`] integration. -* [Script optimization]. - * Low-level [function registration API]({{rootUrl}}/rust/register-raw.md) * [Domain-Specific Languages][DSL]. diff --git a/doc/src/language/modules/export.md b/doc/src/language/modules/export.md index aaa8eb2d..70977290 100644 --- a/doc/src/language/modules/export.md +++ b/doc/src/language/modules/export.md @@ -4,6 +4,16 @@ Export Variables, Functions and Sub-Modules in Module {{#include ../../links.md}} +The easiest way to expose a package of functions as a self-contained [module] is to do it via a Rhai script itself. + +See the section on [_Creating a Module from AST_]({{rootUrl}}/rust/modules/ast.md) for more details. + +The script text is evaluated, variables are then selectively exposed via the [`export`] statement. +Functions defined by the script are automatically exported. + +Modules loaded within this module at the global level become _sub-modules_ and are also automatically exported. + + Export Global Variables ---------------------- diff --git a/doc/src/language/modules/import.md b/doc/src/language/modules/import.md index b592ff74..44ee5412 100644 --- a/doc/src/language/modules/import.md +++ b/doc/src/language/modules/import.md @@ -4,6 +4,12 @@ Import a Module {{#include ../../links.md}} +Before a module can be used (via an `import` statement) in a script, there must be a [module resolver] +registered into the [`Engine`], the default being the `FileModuleResolver`. + +See the section on [_Module Resolvers_][module resolver] for more details. + + `import` Statement ----------------- diff --git a/doc/src/language/modules/index.md b/doc/src/language/modules/index.md index cac54f3d..9df73faf 100644 --- a/doc/src/language/modules/index.md +++ b/doc/src/language/modules/index.md @@ -6,13 +6,9 @@ Modules Rhai allows organizing code (functions, both Rust-based or script-based, and variables) into _modules_. Modules can be disabled via the [`no_module`] feature. -A module is of the type `Module` and encapsulates a Rhai script together with the functions defined -by that script. +A module is of the type `Module` and holds a collection of functions, variables, iterators and sub-modules. +It may be created entirely from Rust functions, or it may encapsulate a Rhai script together with the functions +and variables defined by that script. -The script text is run, variables are then selectively exposed via the [`export`] statement. -Functions defined by the script are automatically exported. - -Modules loaded within this module at the global level become _sub-modules_ and are also automatically exported. - -Other scripts can then load this module and use the variables and functions exported +Other scripts can then load this module and use the functions and variables exported as if they were defined inside the same script. diff --git a/doc/src/links.md b/doc/src/links.md index ac7f1f4a..54826c66 100644 --- a/doc/src/links.md +++ b/doc/src/links.md @@ -102,9 +102,9 @@ [anonymous functions]: {{rootUrl}}/language/fn-anon.md [operator overloading]: {{rootUrl}}/rust/operators.md -[`Module`]: {{rootUrl}}/language/modules/index.md -[module]: {{rootUrl}}/language/modules/index.md -[modules]: {{rootUrl}}/language/modules/index.md +[`Module`]: {{rootUrl}}/rust/modules/index.md +[module]: {{rootUrl}}/rust/modules/index.md +[modules]: {{rootUrl}}/rust/modules/index.md [module resolver]: {{rootUrl}}/rust/modules/resolvers.md [`export`]: {{rootUrl}}/language/modules/export.md [`import`]: {{rootUrl}}/language/modules/import.md diff --git a/doc/src/rust/modules/ast.md b/doc/src/rust/modules/ast.md index 51f1b187..1410fa20 100644 --- a/doc/src/rust/modules/ast.md +++ b/doc/src/rust/modules/ast.md @@ -10,9 +10,12 @@ Create a Module from an AST A _module_ can be created from a single script (or pre-compiled [`AST`]) containing global variables, functions and sub-modules via the `Module::eval_ast_as_new` method. +See the section on [_Exporting Variables, Functions and Sub-Modules_][`export`] for details on how to prepare +a Rhai script for this purpose as well as to control which functions/variables to export. + When given an [`AST`], it is first evaluated, then the following items are exposed as members of the new module: -* Global variables - essentially all variables that remain in the [`Scope`] at the end of a script run - that are exported. Variables not exported (via the `export` statement) remain hidden. +* Global variables - all variables exported via the `export` statement (those not exported remain hidden). * Functions not specifically marked `private`. diff --git a/doc/src/rust/modules/index.md b/doc/src/rust/modules/index.md new file mode 100644 index 00000000..9df73faf --- /dev/null +++ b/doc/src/rust/modules/index.md @@ -0,0 +1,14 @@ +Modules +======= + +{{#include ../../links.md}} + +Rhai allows organizing code (functions, both Rust-based or script-based, and variables) into _modules_. +Modules can be disabled via the [`no_module`] feature. + +A module is of the type `Module` and holds a collection of functions, variables, iterators and sub-modules. +It may be created entirely from Rust functions, or it may encapsulate a Rhai script together with the functions +and variables defined by that script. + +Other scripts can then load this module and use the functions and variables exported +as if they were defined inside the same script. diff --git a/doc/src/rust/modules/resolvers.md b/doc/src/rust/modules/resolvers.md index abca9c27..961110cd 100644 --- a/doc/src/rust/modules/resolvers.md +++ b/doc/src/rust/modules/resolvers.md @@ -5,6 +5,8 @@ Module Resolvers When encountering an [`import`] statement, Rhai attempts to _resolve_ the module based on the path string. +See the section on [_Importing Modules_][`import`] for more details. + _Module Resolvers_ are service types that implement the [`ModuleResolver`][traits] trait. From ae76d9b8aeb0402fbe58021336fa92bfbd920df5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 7 Oct 2020 10:43:39 +0800 Subject: [PATCH 27/29] Add instructions on how to build The Book. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 5b06f34c..cc68898c 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,10 @@ Documentation See [The Rhai Book](https://schungx.github.io/rhai) for details on the Rhai scripting engine and language. +To build _The Book_, first install [`mdbook`](https://github.com/rust-lang/mdBook) +and [`mdbook-tera`](https://github.com/avitex/mdbook-tera) (for templating). +Running `mdbook build` builds it. + Playground ---------- From df1dd5190eb5c825e01f44895b25a3561faeda02 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 7 Oct 2020 10:43:53 +0800 Subject: [PATCH 28/29] Add usage pattern on multi-layer functions. --- doc/src/SUMMARY.md | 5 +- doc/src/engine/custom-syntax.md | 2 +- doc/src/patterns/multi-layer.md | 117 ++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 doc/src/patterns/multi-layer.md diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 9b6f1fc5..ce9c1554 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -118,8 +118,9 @@ The Rhai Scripting Language 2. [Loadable Configuration](patterns/config.md) 3. [Control Layer](patterns/control.md) 4. [Singleton Command](patterns/singleton.md) - 5. [One Engine Instance Per Call](patterns/parallel.md) - 6. [Scriptable Event Handler with State](patterns/events.md) + 5. [Multi-Layer Functions](patterns/multi-layer.md) + 6. [One Engine Instance Per Call](patterns/parallel.md) + 7. [Scriptable Event Handler with State](patterns/events.md) 9. [Advanced Topics](advanced.md) 1. [Capture Scope for Function Call](language/fn-capture.md) 2. [Low-Level API](rust/register-raw.md) diff --git a/doc/src/engine/custom-syntax.md b/doc/src/engine/custom-syntax.md index 3f88112c..f1d8e481 100644 --- a/doc/src/engine/custom-syntax.md +++ b/doc/src/engine/custom-syntax.md @@ -131,7 +131,7 @@ It should simply be passed straight-through the the [`Engine`]. ### Access Arguments The most important argument is `inputs` where the matched identifiers (`$ident$`), expressions/statements (`$expr$`) -and statement blocks (`$block$) are provided. +and statement blocks (`$block$`) are provided. To access a particular argument, use the following patterns: diff --git a/doc/src/patterns/multi-layer.md b/doc/src/patterns/multi-layer.md new file mode 100644 index 00000000..8a296c72 --- /dev/null +++ b/doc/src/patterns/multi-layer.md @@ -0,0 +1,117 @@ +Multi-Layer Functions +===================== + +{{#include ../links.md}} + + +Usage Scenario +-------------- + +* A system is divided into separate _layers_, each providing logic in terms of scripted [functions]. + +* A lower layer provides _default implementations_ of certain functions. + +* Higher layers each provide progressively more specific implementations of the same functions. + +* A more specific function, if defined in a higher layer, always overrides the implementation in a lower layer. + +* This is akin to object-oriented programming but with functions. + +* This type of system is extremely convenient for dynamic business rules configuration, setting corporate-wide + policies, granting permissions for specific roles etc. where specific, local rules need to override + corporate-wide defaults. + + +Key Concepts +------------ + +* Each layer is a separate script. + +* The lowest layer script is compiled into a base [`AST`]. + +* Higher layer scripts are also compiled into [`AST`] and _merged_ into the base using `AST::merge`, + overriding any existing functions. + + +Examples +-------- + +Assume the following four scripts: + +```rust +---------------- +| default.rhai | +---------------- + +// Default implementation of 'foo'. +fn foo(x) { x + 1 } + +// Default implementation of 'bar'. +fn bar(x, y) { x + y } + +// Default implementation of 'no_touch'. +fn no_touch() { throw "do not touch me!"; } + + +--------------- +| lowest.rhai | +--------------- + +// Specific implementation of 'foo'. +fn foo(x) { x * 2 } + +// New implementation for this layer. +fn baz() { print("hello!"); } + + +--------------- +| middle.rhai | +--------------- + +// Specific implementation of 'bar'. +fn bar(x, y) { x - y } + +// Specific implementation of 'baz'. +fn baz() { print("hey!"); } + + +---------------- +| highest.rhai | +---------------- + +// Specific implementation of 'foo'. +fn foo(x) { x + 42 } +``` + +Load and merge them sequentially: + +```rust +let engine = Engine::new(); + +// Compile the baseline default implementations. +let mut ast = engine.compile_file("default.rhai".into())?; + +// Merge in the first layer. +let lowest = engine.compile_file("lowest.rhai".into())?; +ast = ast.merge(&lowest); + +// Merge in the second layer. +let middle = engine.compile_file("middle.rhai".into())?; +ast = ast.merge(&middle); + +// Merge in the third layer. +let highest = engine.compile_file("highest.rhai".into())?; +ast = ast.merge(&highest); + +// Now, 'ast' contains the following functions: +// +// fn no_touch() { // from 'default.rhai' +// throw "do not touch me!"; +// } +// fn foo(x) { x + 42 } // from 'highest.rhai' +// fn bar(x, y) { x - y } // from 'middle.rhai' +// fn baz() { print("hey!"); } // from 'middle.rhai' +``` + +Unfortunately, there is no `super` call that calls the base implementation +(i.e. no way for a higher-layer function to call an equivalent lower-layer function). From 3340760b35ce1abb6c7a65c4f977e42858c805f8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 7 Oct 2020 11:44:06 +0800 Subject: [PATCH 29/29] Fix no_std build. --- src/any.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/any.rs b/src/any.rs index 2ef41da6..3dfbad6b 100644 --- a/src/any.rs +++ b/src/any.rs @@ -21,7 +21,7 @@ use crate::stdlib::{ boxed::Box, fmt, ops::{Deref, DerefMut}, - string::String, + string::{String, ToString}, }; #[cfg(not(feature = "no_closure"))] @@ -531,7 +531,7 @@ impl Dynamic { /// assert_eq!(result.type_name(), "i64"); /// assert_eq!(result.to_string(), "42"); /// - /// let result = Dynamic::from("hello".to_string()); + /// let result = Dynamic::from("hello"); /// assert_eq!(result.type_name(), "string"); /// assert_eq!(result.to_string(), "hello"); ///