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::*;