Module:;eval_ast_as_new defaults to merging namespaces.

This commit is contained in:
Stephen Chung 2020-10-03 11:42:54 +08:00
parent a72f70846f
commit eec3f4e1bf
12 changed files with 22 additions and 412 deletions

View File

@ -9,6 +9,8 @@ Breaking changes
* `AST::iter_functions` now returns 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`. * `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 Version 0.19.0

View File

@ -7,7 +7,7 @@ use rhai::{module_resolvers::StaticModuleResolver, Engine, Module, OptimizationL
use test::Bencher; use test::Bencher;
#[bench] #[bench]
fn bench_eval_module_merged(bench: &mut Bencher) { fn bench_eval_module(bench: &mut Bencher) {
let script = r#" let script = r#"
fn foo(x) { x + 1 } fn foo(x) { x + 1 }
fn bar(x) { foo(x) } fn bar(x) { foo(x) }
@ -18,38 +18,7 @@ fn bench_eval_module_merged(bench: &mut Bencher) {
let ast = engine.compile(script).unwrap(); let ast = engine.compile(script).unwrap();
let module = Module::eval_ast_as_new(Default::default(), &ast, true, &engine).unwrap(); let module = Module::eval_ast_as_new(Default::default(), &ast, &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(); let mut resolver = StaticModuleResolver::new();
resolver.insert("testing", module); resolver.insert("testing", module);

View File

@ -88,84 +88,3 @@ fn say_hello() {
} }
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!'
```

View File

@ -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. * Global modules that remain in the [`Scope`] at the end of a script run.
`Module::eval_ast_as_new` encapsulates the entire `AST` into each function call, merging the module namespace
`merge_namespaces` Parameter with the global namespace. Therefore, functions defined within the same module script can cross-call each other.
---------------------------
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.
Examples Examples
@ -77,15 +64,9 @@ let ast = engine.compile(r#"
"#)?; "#)?;
// Convert the 'AST' into a module, using the 'Engine' to evaluate it first // Convert the 'AST' into a module, using the 'Engine' to evaluate it first
// // A copy of the entire 'AST' is encapsulated into each function,
// The second parameter ('merge_namespaces'), when set to true, will encapsulate // allowing functions in the module script to cross-call each other.
// a copy of the entire 'AST' into each function, allowing functions in the module script let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?;
// 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)?;
// 'module' now contains: // 'module' now contains:
// - sub-module: 'foobar' (renamed from 'extra') // - sub-module: 'foobar' (renamed from 'extra')

View File

@ -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. `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` `StaticModuleResolver`
--------------------- ---------------------

View File

@ -1266,19 +1266,10 @@ impl Module {
/// Create a new `Module` by evaluating an `AST`. /// Create a new `Module` by evaluating an `AST`.
/// ///
/// ### `merge_namespaces` parameter /// The entire `AST` is encapsulated into each function, allowing functions
/// /// to cross-call each other. Functions in the global namespace, plus all functions
/// * If `true`, the entire `AST` is encapsulated into each function, allowing functions /// defined in the module, are _merged_ into a _unified_ namespace before each call.
/// to cross-call each other. Functions in the global namespace, plus all functions /// Therefore, all functions will be found.
/// 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.
/// ///
/// # Examples /// # Examples
/// ///
@ -1288,19 +1279,14 @@ impl Module {
/// ///
/// let engine = Engine::new(); /// let engine = Engine::new();
/// let ast = engine.compile("let answer = 42; export answer;")?; /// 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!(module.contains_var("answer"));
/// assert_eq!(module.get_var_value::<i64>("answer").unwrap(), 42); /// assert_eq!(module.get_var_value::<i64>("answer").unwrap(), 42);
/// # Ok(()) /// # Ok(())
/// # } /// # }
/// ``` /// ```
#[cfg(not(feature = "no_module"))] #[cfg(not(feature = "no_module"))]
pub fn eval_ast_as_new( pub fn eval_ast_as_new(mut scope: Scope, ast: &AST, engine: &Engine) -> FuncReturn<Self> {
mut scope: Scope,
ast: &AST,
merge_namespaces: bool,
engine: &Engine,
) -> FuncReturn<Self> {
let mut mods = Imports::new(); let mut mods = Imports::new();
// Run the script // Run the script
@ -1324,7 +1310,7 @@ impl Module {
}); });
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
if merge_namespaces { {
let ast_lib: Shared<Module> = ast.lib().clone().into(); let ast_lib: Shared<Module> = ast.lib().clone().into();
ast.iter_functions() ast.iter_functions()
@ -1370,8 +1356,6 @@ impl Module {
}, },
); );
}); });
} else {
module.merge(ast.lib());
} }
Ok(module) Ok(module)

View File

@ -153,7 +153,7 @@ impl ModuleResolver for FileModuleResolver {
let c = self.cache.read().unwrap(); let c = self.cache.read().unwrap();
if let Some(ast) = c.get(&file_path) { 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)) Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos))
})?; })?;
None None
@ -163,7 +163,7 @@ impl ModuleResolver for FileModuleResolver {
Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) 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)) Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos))
})?; })?;
Some(ast) Some(ast)

View File

@ -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<HashMap<PathBuf, AST>>,
#[cfg(feature = "sync")]
cache: RwLock<HashMap<PathBuf, AST>>,
}
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<P: Into<PathBuf>>(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<P: Into<PathBuf>, E: Into<String>>(
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<P: Into<PathBuf>>(
&self,
engine: &Engine,
path: &str,
) -> Result<Module, Box<EvalAltResult>> {
self.resolve(engine, path, Default::default())
}
}
impl ModuleResolver for GlobalFileModuleResolver {
fn resolve(
&self,
engine: &Engine,
path: &str,
pos: Position,
) -> Result<Module, Box<EvalAltResult>> {
// 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)
}
}

View File

@ -17,14 +17,6 @@ mod file;
#[cfg(not(target_arch = "wasm32"))] #[cfg(not(target_arch = "wasm32"))]
pub use file::FileModuleResolver; 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; mod stat;
pub use stat::StaticModuleResolver; pub use stat::StaticModuleResolver;

View File

@ -265,7 +265,7 @@ fn test_module_from_ast() -> Result<(), Box<EvalAltResult>> {
engine.set_module_resolver(Some(resolver1)); 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(); let mut resolver2 = StaticModuleResolver::new();
resolver2.insert("testing", module); resolver2.insert("testing", module);
@ -374,7 +374,7 @@ fn test_module_ast_namespace() -> Result<(), Box<EvalAltResult>> {
let ast = engine.compile(script)?; 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(); let mut resolver = StaticModuleResolver::new();
resolver.insert("testing", module); resolver.insert("testing", module);

View File

@ -36,12 +36,9 @@ fn test_packages_with_script() -> Result<(), Box<EvalAltResult>> {
let mut engine = Engine::new(); let mut engine = Engine::new();
let ast = engine.compile("fn foo(x) { x + 1 } fn bar(x) { foo(x) + 1 }")?; 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); engine.load_package(module);
assert_eq!(engine.eval::<INT>("foo(41)")?, 42); assert_eq!(engine.eval::<INT>("foo(41)")?, 42);
let module = Module::eval_ast_as_new(Scope::new(), &ast, true, &engine)?;
engine.load_package(module);
assert_eq!(engine.eval::<INT>("bar(40)")?, 42); assert_eq!(engine.eval::<INT>("bar(40)")?, 42);
Ok(()) Ok(())

View File

@ -47,6 +47,7 @@ mod test {
macro_rules! gen_unary_functions { macro_rules! gen_unary_functions {
($op_name:ident = $op_fn:ident ( $($arg_type:ident),+ ) -> $return_type:ident) => { ($op_name:ident = $op_fn:ident ( $($arg_type:ident),+ ) -> $return_type:ident) => {
mod $op_name { $( mod $op_name { $(
#[allow(non_snake_case)]
pub mod $arg_type { pub mod $arg_type {
use super::super::*; use super::super::*;