From b3d318ef7f9ba344ec9061f1e58b8e4418485fc8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 7 Nov 2020 23:33:21 +0800 Subject: [PATCH 1/5] Module resolver returns shared module. --- RELEASES.md | 6 +++ doc/src/engine/custom-syntax.md | 1 + doc/src/engine/var.md | 1 + doc/src/language/fn-namespaces.md | 2 + doc/src/language/fn-ptr.md | 9 +++- doc/src/plugins/function.md | 10 ++--- doc/src/plugins/module.md | 10 ++--- doc/src/rust/modules/imp-resolver.md | 15 +++---- doc/src/rust/modules/resolvers.md | 6 +-- doc/src/rust/register-raw.md | 1 + src/engine.rs | 52 ++++++++++++------------ src/engine_api.rs | 2 +- src/fn_call.rs | 21 +++++----- src/fn_native.rs | 61 ++++++++++++++++------------ src/module/mod.rs | 6 +-- src/module/resolvers/collection.rs | 3 +- src/module/resolvers/file.rs | 46 +++++++++------------ src/module/resolvers/mod.rs | 4 +- src/module/resolvers/stat.rs | 37 ++++++++--------- src/optimize.rs | 1 + 20 files changed, 158 insertions(+), 136 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 0b9659df..f1d034d8 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -8,6 +8,12 @@ Breaking changes ---------------- * Modules imported at global level can now be accessed in functions. +* `ModuleResolver::resolve` now returns `Shared` for better resources sharing when loading modules. + +Enhancements +------------ + +* Modules imported via `import` statements at global level can now be used in functions. There is no longer any need to re-`import` the modules at the beginning of each function block. Version 0.19.4 diff --git a/doc/src/engine/custom-syntax.md b/doc/src/engine/custom-syntax.md index ebac08f0..ee086ec9 100644 --- a/doc/src/engine/custom-syntax.md +++ b/doc/src/engine/custom-syntax.md @@ -121,6 +121,7 @@ where: | `context` | `&mut EvalContext` | mutable reference to the current evaluation _context_ | | - `context.scope` | `&mut Scope` | mutable reference to the current [`Scope`]; variables can be added to/removed from it | | - `context.engine()` | `&Engine` | reference to the current [`Engine`] | +| - `context.imports()` | `&Imports` | reference to the current stack of [modules] imported via `import` statements | | - `context.iter_namespaces()` | `impl Iterator` | iterator of the namespaces (as [modules]) containing all script-defined functions | | - `context.this_ptr()` | `Option<&Dynamic>` | reference to the current bound [`this`] pointer, if any | | - `context.call_level()` | `usize` | the current nesting level of function calls | diff --git a/doc/src/engine/var.md b/doc/src/engine/var.md index 5af45a41..9abfe0f0 100644 --- a/doc/src/engine/var.md +++ b/doc/src/engine/var.md @@ -74,6 +74,7 @@ where: | `context` | `&EvalContext` | reference to the current evaluation _context_ | | - `context.scope` | `&Scope` | reference to the current [`Scope`] containing all variables up to the current evaluation position | | - `context.engine()` | `&Engine` | reference to the current [`Engine`] | +| - `context.imports()` | `&Imports` | reference to the current stack of [modules] imported via `import` statements | | - `context.iter_namespaces()` | `impl Iterator` | iterator of the namespaces (as [modules]) containing all script-defined functions | | - `context.this_ptr()` | `Option<&Dynamic>` | reference to the current bound [`this`] pointer, if any | | - `context.call_level()` | `usize` | the current nesting level of function calls | diff --git a/doc/src/language/fn-namespaces.md b/doc/src/language/fn-namespaces.md index f4071421..0d798ae4 100644 --- a/doc/src/language/fn-namespaces.md +++ b/doc/src/language/fn-namespaces.md @@ -30,6 +30,8 @@ There is one _global_ namespace for every [`Engine`], which includes: * All the Rust functions defined in [packages] that are loaded into the [`Engine`]. +* All the [modules] imported via [`import`] statements. + In addition, during evaluation of an [`AST`], all script-defined functions bundled together within the [`AST`] are added to the global namespace and override any existing registered functions of the same names and number of parameters. diff --git a/doc/src/language/fn-ptr.md b/doc/src/language/fn-ptr.md index d10ff4d4..40fd8bcd 100644 --- a/doc/src/language/fn-ptr.md +++ b/doc/src/language/fn-ptr.md @@ -223,7 +223,14 @@ engine.register_raw_fn("super_call", ------------------ `FnPtr::call_dynamic` takes a parameter of type `NativeCallContext` which holds the _native call context_ -of the particular call to a registered Rust function. +of the particular call to a registered Rust function. It is a type that exposes the following: + +| Field | Type | Description | +| ------------------- | :-----------------------------: | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `engine()` | `&Engine` | the current [`Engine`], with all configurations and settings.
This is sometimes useful for calling a script-defined function within the same evaluation context using [`Engine::call_fn`][`call_fn`], or calling a [function pointer]. | +| `imports()` | `Option<&Imports>` | reference to the current stack of [modules] imported via `import` statements (if any) | +| `iter_namespaces()` | `impl Iterator` | iterator of the namespaces (as [modules]) containing all script-defined functions | + This type is normally provided by the [`Engine`] (e.g. when using [`Engine::register_fn_raw`](../rust/register-raw.md)). However, it may also be manually constructed from a tuple: diff --git a/doc/src/plugins/function.md b/doc/src/plugins/function.md index e63448d4..f539be19 100644 --- a/doc/src/plugins/function.md +++ b/doc/src/plugins/function.md @@ -85,11 +85,11 @@ specially by the plugins system. `NativeCallContext` is a type that encapsulates the current _native call context_ and exposes the following: -* `NativeCallContext::engine(): &Engine` - the current [`Engine`], with all configurations and settings. - This is sometimes useful for calling a script-defined function within the same evaluation context - using [`Engine::call_fn`][`call_fn`]. - -* `NativeCallContext::namespace(): &Module` - the global namespace of script-defined functions, as a [`Module`]. +| Field | Type | Description | +| ------------------- | :-----------------------------: | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `engine()` | `&Engine` | the current [`Engine`], with all configurations and settings.
This is sometimes useful for calling a script-defined function within the same evaluation context using [`Engine::call_fn`][`call_fn`], or calling a [function pointer]. | +| `imports()` | `Option<&Imports>` | reference to the current stack of [modules] imported via `import` statements (if any) | +| `iter_namespaces()` | `impl Iterator` | iterator of the namespaces (as [modules]) containing all script-defined functions | This first parameter, if exists, will be stripped before all other processing. It is _virtual_. Most importantly, it does _not_ count as a parameter to the function and there is no need to provide diff --git a/doc/src/plugins/module.md b/doc/src/plugins/module.md index a60d6fed..e51f5103 100644 --- a/doc/src/plugins/module.md +++ b/doc/src/plugins/module.md @@ -342,11 +342,11 @@ specially by the plugins system. `NativeCallContext` is a type that encapsulates the current _native call context_ and exposes the following: -* `NativeCallContext::engine(): &Engine` - the current [`Engine`], with all configurations and settings. - This is sometimes useful for calling a script-defined function within the same evaluation context - using [`Engine::call_fn`][`call_fn`]. - -* `NativeCallContext::namespace(): &Module` - the global namespace of script-defined functions, as a [`Module`]. +| Field | Type | Description | +| ------------------- | :-----------------------------: | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `engine()` | `&Engine` | the current [`Engine`], with all configurations and settings.
This is sometimes useful for calling a script-defined function within the same evaluation context using [`Engine::call_fn`][`call_fn`], or calling a [function pointer]. | +| `imports()` | `Option<&Imports>` | reference to the current stack of [modules] imported via `import` statements (if any) | +| `iter_namespaces()` | `impl Iterator` | iterator of the namespaces (as [modules]) containing all script-defined functions | This first parameter, if exists, will be stripped before all other processing. It is _virtual_. Most importantly, it does _not_ count as a parameter to the function and there is no need to provide diff --git a/doc/src/rust/modules/imp-resolver.md b/doc/src/rust/modules/imp-resolver.md index 58aada13..2c6dd001 100644 --- a/doc/src/rust/modules/imp-resolver.md +++ b/doc/src/rust/modules/imp-resolver.md @@ -13,7 +13,7 @@ which contains only one function: `resolve`. When Rhai prepares to load a module, `ModuleResolver::resolve` is called with the name of the _module path_ (i.e. the path specified in the [`import`] statement). -* Upon success, it should return a [`Module`]. +* Upon success, it should return an [`Rc`][module] (or `Arc` under [`sync`]). * If the path does not resolve to a valid module, return `EvalAltResult::ErrorModuleNotFound`. @@ -37,14 +37,15 @@ impl ModuleResolver for MyModuleResolver { engine: &Engine, // reference to the current 'Engine' path: &str, // the module path pos: Position, // position of the 'import' statement - ) -> Result> { + ) -> Result, Box> { // Check module path. if is_valid_module_path(path) { - // Load the custom module. - load_secret_module(path).map_err(|err| - // Return EvalAltResult::ErrorInModule upon loading error - EvalAltResult::ErrorInModule(err.to_string(), pos).into() - ) + load_secret_module(path) // load the custom module + .map(Rc::new) // share it + .map_err(|err| + // Return EvalAltResult::ErrorInModule upon loading error + EvalAltResult::ErrorInModule(err.to_string(), pos).into() + ) } else { // Return EvalAltResult::ErrorModuleNotFound if the path is invalid Err(EvalAltResult::ErrorModuleNotFound(path.into(), pos).into()) diff --git a/doc/src/rust/modules/resolvers.md b/doc/src/rust/modules/resolvers.md index 961110cd..9bafb91b 100644 --- a/doc/src/rust/modules/resolvers.md +++ b/doc/src/rust/modules/resolvers.md @@ -29,6 +29,8 @@ Loads a script file (based off the current directory) with `.rhai` extension. All functions in the _global_ namespace, plus all those defined in the same module, are _merged_ into a _unified_ namespace. +Modules are also _cached_ so a script file is only evaluated _once_, even when repeatedly imported. + ```rust ------------------ | my_module.rhai | @@ -122,10 +124,6 @@ m::greet(); // prints "hello! from module!" The base directory can be changed via the `FileModuleResolver::new_with_path` constructor function. -### Returning a module instead - -`FileModuleResolver::create_module` loads a script file and returns a module with the standard behavior. - `StaticModuleResolver` --------------------- diff --git a/doc/src/rust/register-raw.md b/doc/src/rust/register-raw.md index 38f9b4b1..c277436c 100644 --- a/doc/src/rust/register-raw.md +++ b/doc/src/rust/register-raw.md @@ -70,6 +70,7 @@ where: | `T` | `impl Clone` | return type of the function | | `context` | `NativeCallContext` | the current _native call context_ | | - `context.engine()` | `&Engine` | the current [`Engine`], with all configurations and settings.
This is sometimes useful for calling a script-defined function within the same evaluation context using [`Engine::call_fn`][`call_fn`], or calling a [function pointer]. | +| - `context.imports()` | `Option<&Imports>` | reference to the current stack of [modules] imported via `import` statements (if any) | | - `context.iter_namespaces()` | `impl Iterator` | iterator of the namespaces (as [modules]) containing all script-defined functions | | `args` | `&mut [&mut Dynamic]` | a slice containing `&mut` references to [`Dynamic`] values.
The slice is guaranteed to contain enough arguments _of the correct types_. | diff --git a/src/engine.rs b/src/engine.rs index 8f6429e1..4e563dd1 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -3,7 +3,7 @@ use crate::ast::{BinaryExpr, Expr, FnCallInfo, Ident, IdentX, ReturnType, Stmt}; use crate::dynamic::{map_std_type_name, Dynamic, Union, Variant}; use crate::fn_call::run_builtin_op_assignment; -use crate::fn_native::{Callback, FnPtr, OnVarCallback}; +use crate::fn_native::{Callback, FnPtr, OnVarCallback, Shared}; use crate::module::{Module, ModuleRef}; use crate::optimize::OptimizationLevel; use crate::packages::{Package, PackagesCollection, StandardPackage}; @@ -73,48 +73,49 @@ pub type Map = HashMap; // // 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. -// -// `Imports` is implemented as two `Vec`'s of exactly the same length. That's because a `Module` is large, -// so packing the import names together improves cache locality. #[derive(Debug, Clone, Default)] -pub struct Imports(StaticVec, StaticVec); +pub struct Imports(StaticVec<(Shared, ImmutableString)>); impl Imports { /// Get the length of this stack of imported modules. pub fn len(&self) -> usize { self.0.len() } + /// Is this stack of imported modules empty? + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } /// Get the imported module at a particular index. - pub fn get(&self, index: usize) -> Option<&Module> { - self.0.get(index) + pub fn get(&self, index: usize) -> Option> { + self.0.get(index).map(|(m, _)| m).cloned() } /// Get the index of an imported module by name. pub fn find(&self, name: &str) -> Option { - self.1 + self.0 .iter() .enumerate() .rev() - .find(|(_, key)| key.as_str() == name) + .find(|(_, (_, key))| key.as_str() == name) .map(|(index, _)| index) } /// Push an imported module onto the stack. - pub fn push(&mut self, name: impl Into, module: Module) { - self.0.push(module); - self.1.push(name.into()); + pub fn push(&mut self, name: impl Into, module: impl Into>) { + self.0.push((module.into(), name.into())); } /// Truncate the stack of imported modules to a particular length. pub fn truncate(&mut self, size: usize) { self.0.truncate(size); - self.1.truncate(size); } /// Get an iterator to this stack of imported modules. #[allow(dead_code)] - pub fn iter(&self) -> impl Iterator { - self.1.iter().map(|name| name.as_str()).zip(self.0.iter()) + pub fn iter(&self) -> impl Iterator)> { + self.0 + .iter() + .map(|(module, name)| (name.as_str(), module.clone())) } /// Get a consuming iterator to this stack of imported modules. - pub fn into_iter(self) -> impl Iterator { - self.1.into_iter().zip(self.0.into_iter()) + pub fn into_iter(self) -> impl Iterator)> { + self.0.into_iter().map(|(module, name)| (name, module)) } } @@ -630,11 +631,11 @@ fn default_print(_s: &str) { /// Search for a module within an imports stack. /// Position in `EvalAltResult` is `None` and must be set afterwards. -pub fn search_imports<'s>( - mods: &'s Imports, +pub fn search_imports( + mods: &Imports, state: &mut State, modules: &ModuleRef, -) -> Result<&'s Module, Box> { +) -> Result, Box> { let Ident { name: root, pos } = &modules[0]; // Qualified - check if the root module is directly indexed @@ -1487,7 +1488,9 @@ impl Engine { calc_native_fn_hash(empty(), OP_FUNC, args.iter().map(|a| a.type_id())); if self - .call_native_fn(state, lib, OP_FUNC, hash, args, false, false, def_value) + .call_native_fn( + mods, state, lib, OP_FUNC, hash, args, false, false, def_value, + ) .map_err(|err| err.fill_position(rhs.position()))? .0 .as_bool() @@ -1805,9 +1808,9 @@ impl Engine { // Overriding exact implementation if func.is_plugin_fn() { - func.get_plugin_fn().call((self, lib).into(), args)?; + func.get_plugin_fn().call((self, mods, lib).into(), args)?; } else { - func.get_native_fn()((self, lib).into(), args)?; + func.get_native_fn()((self, mods, lib).into(), args)?; } } // Built-in op-assignment function @@ -2126,10 +2129,9 @@ impl Engine { .try_cast::() { if let Some(resolver) = &self.module_resolver { - let mut module = resolver.resolve(self, &path, expr.position())?; + let module = resolver.resolve(self, &path, expr.position())?; if let Some(name_def) = alias { - module.index_all_sub_modules(); mods.push(name_def.name.clone(), module); } diff --git a/src/engine_api.rs b/src/engine_api.rs index 135fec21..fd1ac1c8 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1662,7 +1662,7 @@ impl Engine { ast.lib() .iter_fn() .filter(|f| f.func.is_script()) - .map(|f| f.func.get_fn_def().clone()) + .map(|f| (**f.func.get_fn_def()).clone()) .collect() } else { Default::default() diff --git a/src/fn_call.rs b/src/fn_call.rs index d1fb23d3..5df04795 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -178,6 +178,7 @@ impl Engine { /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! pub(crate) fn call_native_fn( &self, + mods: &mut Imports, state: &mut State, lib: &[&Module], fn_name: &str, @@ -205,9 +206,9 @@ impl Engine { // Run external function let result = if func.is_plugin_fn() { - func.get_plugin_fn().call((self, lib).into(), args) + func.get_plugin_fn().call((self, mods, lib).into(), args) } else { - func.get_native_fn()((self, lib).into(), args) + func.get_native_fn()((self, mods, lib).into(), args) }; // Restore the original reference @@ -588,6 +589,7 @@ impl Engine { } else { // If it is a native function, redirect it self.call_native_fn( + mods, state, lib, fn_name, @@ -602,7 +604,7 @@ impl Engine { // Normal native function call _ => self.call_native_fn( - state, lib, fn_name, hash_fn, args, is_ref, pub_only, def_val, + mods, state, lib, fn_name, hash_fn, args, is_ref, pub_only, def_val, ), } } @@ -1178,15 +1180,14 @@ impl Engine { } let args = args.as_mut(); - let fn_def = f.get_shared_fn_def().clone(); - let new_scope = &mut Default::default(); - + let fn_def = f.get_fn_def().clone(); self.call_script_fn(new_scope, mods, state, lib, &mut None, &fn_def, args, level) } - Some(f) if f.is_plugin_fn() => { - f.get_plugin_fn().call((self, lib).into(), args.as_mut()) - } + Some(f) if f.is_plugin_fn() => f + .get_plugin_fn() + .clone() + .call((self, mods, lib).into(), args.as_mut()), Some(f) if f.is_native() => { if !f.is_method() { // Clone first argument @@ -1197,7 +1198,7 @@ impl Engine { } } - f.get_native_fn()((self, lib).into(), args.as_mut()) + f.get_native_fn().clone()((self, mods, lib).into(), args.as_mut()) } Some(_) => unreachable!(), None if def_val.is_some() => Ok(def_val.unwrap().into()), diff --git a/src/fn_native.rs b/src/fn_native.rs index 9b80d59a..ef66eff3 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -2,7 +2,7 @@ use crate::ast::{FnAccess, ScriptFnDef}; use crate::dynamic::Dynamic; -use crate::engine::{Engine, EvalContext}; +use crate::engine::{Engine, EvalContext, Imports}; use crate::module::Module; use crate::plugin::PluginFunction; use crate::result::EvalAltResult; @@ -50,28 +50,50 @@ pub type Locked = RwLock; /// Context of native Rust function call. #[derive(Debug, Copy, Clone)] -pub struct NativeCallContext<'e, 'm, 'pm: 'm> { +pub struct NativeCallContext<'e, 'a, 'm, 'pm: 'm> { engine: &'e Engine, + mods: Option<&'a Imports>, lib: &'m [&'pm Module], } +impl<'e, 'a, 'm, 'pm: 'm, M: AsRef<[&'pm Module]> + ?Sized> + From<(&'e Engine, &'a mut Imports, &'m M)> for NativeCallContext<'e, 'a, 'm, 'pm> +{ + fn from(value: (&'e Engine, &'a mut Imports, &'m M)) -> Self { + Self { + engine: value.0, + mods: Some(value.1), + lib: value.2.as_ref(), + } + } +} + impl<'e, 'm, 'pm: 'm, M: AsRef<[&'pm Module]> + ?Sized> From<(&'e Engine, &'m M)> - for NativeCallContext<'e, 'm, 'pm> + for NativeCallContext<'e, '_, 'm, 'pm> { fn from(value: (&'e Engine, &'m M)) -> Self { Self { engine: value.0, + mods: None, lib: value.1.as_ref(), } } } -impl<'e, 'm, 'pm> NativeCallContext<'e, 'm, 'pm> { +impl<'e, 'a, 'm, 'pm> NativeCallContext<'e, 'a, 'm, 'pm> { /// The current `Engine`. #[inline(always)] pub fn engine(&self) -> &'e Engine { self.engine } + /// _[INTERNALS]_ The current set of modules imported via `import` statements. + /// Available under the `internals` feature only. + #[cfg(feature = "internals")] + #[cfg(not(feature = "no_module"))] + #[inline(always)] + pub fn imports(&self) -> Option<&Imports> { + self.mods + } /// Get an iterator over the namespaces containing definition of all script-defined functions. #[inline(always)] pub fn iter_namespaces(&self) -> impl Iterator + 'm { @@ -181,9 +203,11 @@ impl FnPtr { args.insert(0, obj); } + let mut mods = ctx.mods.cloned().unwrap_or_default(); + ctx.engine() .exec_fn_call( - &mut Default::default(), + &mut mods, &mut Default::default(), ctx.lib, fn_name, @@ -396,14 +420,14 @@ impl CallableFunction { Self::Script(f) => f.access, } } - /// Get a reference to a native Rust function. + /// Get a shared reference to a native Rust function. /// /// # Panics /// /// Panics if the `CallableFunction` is not `Pure` or `Method`. - pub fn get_native_fn(&self) -> &FnAny { + pub fn get_native_fn(&self) -> &Shared { match self { - Self::Pure(f) | Self::Method(f) => f.as_ref(), + Self::Pure(f) | Self::Method(f) => f, Self::Iterator(_) | Self::Plugin(_) => unreachable!(), #[cfg(not(feature = "no_function"))] @@ -416,25 +440,12 @@ 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_fn_def(&self) -> &Shared { match self { Self::Pure(_) | Self::Method(_) | Self::Iterator(_) | Self::Plugin(_) => unreachable!(), Self::Script(f) => f, } } - /// Get a reference to a script-defined function definition. - /// - /// # Panics - /// - /// Panics if the `CallableFunction` is not `Script`. - pub fn get_fn_def(&self) -> &ScriptFnDef { - match self { - Self::Pure(_) | Self::Method(_) | Self::Iterator(_) | Self::Plugin(_) => unreachable!(), - - #[cfg(not(feature = "no_function"))] - Self::Script(f) => f, - } - } /// Get a reference to an iterator function. /// /// # Panics @@ -449,14 +460,14 @@ impl CallableFunction { Self::Script(_) => unreachable!(), } } - /// Get a reference to a plugin function. + /// Get a shared reference to a plugin function. /// /// # Panics /// /// Panics if the `CallableFunction` is not `Plugin`. - pub fn get_plugin_fn<'s>(&'s self) -> &FnPlugin { + pub fn get_plugin_fn<'s>(&'s self) -> &Shared { match self { - Self::Plugin(f) => f.as_ref(), + Self::Plugin(f) => f, Self::Pure(_) | Self::Method(_) | Self::Iterator(_) => unreachable!(), #[cfg(not(feature = "no_function"))] diff --git a/src/module/mod.rs b/src/module/mod.rs index b0226d37..6188b64f 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -329,7 +329,7 @@ impl Module { && fn_name == name }, ) - .map(|FuncInfo { func, .. }| func.get_shared_fn_def()) + .map(|FuncInfo { func, .. }| func.get_fn_def()) } /// Does a sub-module exist in the module? @@ -1286,7 +1286,7 @@ impl Module { .values() .map(|f| &f.func) .filter(|f| f.is_script()) - .map(CallableFunction::get_shared_fn_def) + .map(CallableFunction::get_fn_def) .map(|f| { let func = f.clone(); (f.access, f.name.as_str(), f.params.len(), func) @@ -1374,7 +1374,7 @@ impl Module { // Modules left in the scope become sub-modules mods.into_iter().for_each(|(alias, m)| { - module.modules.insert(alias.to_string(), m); + module.modules.insert(alias.to_string(), m.as_ref().clone()); }); // Non-private functions defined become module functions diff --git a/src/module/resolvers/collection.rs b/src/module/resolvers/collection.rs index ceffee33..f1f76e70 100644 --- a/src/module/resolvers/collection.rs +++ b/src/module/resolvers/collection.rs @@ -1,4 +1,5 @@ use crate::engine::Engine; +use crate::fn_native::Shared; use crate::module::{Module, ModuleResolver}; use crate::result::EvalAltResult; use crate::token::Position; @@ -90,7 +91,7 @@ impl ModuleResolver for ModuleResolversCollection { engine: &Engine, path: &str, pos: Position, - ) -> Result> { + ) -> Result, Box> { for resolver in self.0.iter() { match resolver.resolve(engine, path, pos) { Ok(module) => return Ok(module), diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index a77bcc57..b5bb346e 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -1,6 +1,5 @@ -use crate::ast::AST; use crate::engine::Engine; -use crate::fn_native::Locked; +use crate::fn_native::{Locked, Shared}; use crate::module::{Module, ModuleResolver}; use crate::result::EvalAltResult; use crate::token::Position; @@ -40,7 +39,7 @@ use crate::stdlib::{boxed::Box, collections::HashMap, path::PathBuf, string::Str pub struct FileModuleResolver { path: PathBuf, extension: String, - cache: Locked>, + cache: Locked>>, } impl Default for FileModuleResolver { @@ -119,16 +118,6 @@ impl FileModuleResolver { pub fn new() -> Self { Default::default() } - - /// Create a `Module` from a file path. - #[inline(always)] - pub fn create_module>( - &self, - engine: &Engine, - path: &str, - ) -> Result> { - self.resolve(engine, path, Default::default()) - } } impl ModuleResolver for FileModuleResolver { @@ -137,48 +126,51 @@ impl ModuleResolver for FileModuleResolver { engine: &Engine, path: &str, pos: Position, - ) -> Result> { + ) -> Result, Box> { // 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 = { + let mut module = None; + + let module_ref = { #[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, engine).map_err(|err| { - Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) - })?; - None + if let Some(module) = c.get(&file_path) { + module.clone() } 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, engine).map_err(|err| { + let mut m = Module::eval_ast_as_new(scope, &ast, engine).map_err(|err| { Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) })?; - Some(ast) + + m.index_all_sub_modules(); + + let m: Shared = m.into(); + module = Some(m.clone()); + m } }; - if let Some(ast) = ast { + if let Some(module) = module { // Put it into the cache #[cfg(not(feature = "sync"))] - self.cache.borrow_mut().insert(file_path, ast); + self.cache.borrow_mut().insert(file_path, module); #[cfg(feature = "sync")] - self.cache.write().unwrap().insert(file_path, ast); + self.cache.write().unwrap().insert(file_path, module); } - Ok(module) + Ok(module_ref) } } diff --git a/src/module/resolvers/mod.rs b/src/module/resolvers/mod.rs index 386d9e05..62638130 100644 --- a/src/module/resolvers/mod.rs +++ b/src/module/resolvers/mod.rs @@ -1,5 +1,5 @@ use crate::engine::Engine; -use crate::fn_native::SendSync; +use crate::fn_native::{SendSync, Shared}; use crate::module::Module; use crate::result::EvalAltResult; use crate::token::Position; @@ -28,5 +28,5 @@ pub trait ModuleResolver: SendSync { engine: &Engine, path: &str, pos: Position, - ) -> Result>; + ) -> Result, Box>; } diff --git a/src/module/resolvers/stat.rs b/src/module/resolvers/stat.rs index 5d895e9d..0eeeb8e8 100644 --- a/src/module/resolvers/stat.rs +++ b/src/module/resolvers/stat.rs @@ -1,4 +1,5 @@ use crate::engine::Engine; +use crate::fn_native::Shared; use crate::module::{Module, ModuleResolver}; use crate::result::EvalAltResult; use crate::token::Position; @@ -23,7 +24,7 @@ use crate::stdlib::{boxed::Box, collections::HashMap, ops::AddAssign, string::St /// engine.set_module_resolver(Some(resolver)); /// ``` #[derive(Debug, Clone, Default)] -pub struct StaticModuleResolver(HashMap); +pub struct StaticModuleResolver(HashMap>); impl StaticModuleResolver { /// Create a new `StaticModuleResolver`. @@ -48,12 +49,13 @@ impl StaticModuleResolver { } /// Add a module keyed by its path. #[inline(always)] - pub fn insert(&mut self, path: impl Into, module: Module) { - self.0.insert(path.into(), module); + pub fn insert(&mut self, path: impl Into, mut module: Module) { + module.index_all_sub_modules(); + self.0.insert(path.into(), module.into()); } /// Remove a module given its path. #[inline(always)] - pub fn remove(&mut self, path: &str) -> Option { + pub fn remove(&mut self, path: &str) -> Option> { self.0.remove(path) } /// Does the path exist? @@ -63,17 +65,12 @@ impl StaticModuleResolver { } /// Get an iterator of all the modules. #[inline(always)] - pub fn iter(&self) -> impl Iterator { - self.0.iter().map(|(k, v)| (k.as_str(), v)) + pub fn iter(&self) -> impl Iterator)> { + self.0.iter().map(|(k, v)| (k.as_str(), v.clone())) } /// Get a mutable iterator of all the modules. #[inline(always)] - pub fn iter_mut(&mut self) -> impl Iterator { - self.0.iter_mut().map(|(k, v)| (k.as_str(), v)) - } - /// Get a mutable iterator of all the modules. - #[inline(always)] - pub fn into_iter(self) -> impl Iterator { + pub fn into_iter(self) -> impl Iterator)> { self.0.into_iter() } /// Get an iterator of all the module paths. @@ -83,13 +80,8 @@ impl StaticModuleResolver { } /// Get an iterator of all the modules. #[inline(always)] - pub fn values(&self) -> impl Iterator { - self.0.values() - } - /// Get a mutable iterator of all the modules. - #[inline(always)] - pub fn values_mut(&mut self) -> impl Iterator { - self.0.values_mut() + pub fn values<'a>(&'a self) -> impl Iterator> + 'a { + self.0.values().map(|m| m.clone()) } /// Remove all modules. #[inline(always)] @@ -118,7 +110,12 @@ impl StaticModuleResolver { impl ModuleResolver for StaticModuleResolver { #[inline(always)] - fn resolve(&self, _: &Engine, path: &str, pos: Position) -> Result> { + fn resolve( + &self, + _: &Engine, + path: &str, + pos: Position, + ) -> Result, Box> { self.0 .get(path) .cloned() diff --git a/src/optimize.rs b/src/optimize.rs index 4b023627..08d23518 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -140,6 +140,7 @@ fn call_fn_with_constant_arguments( state .engine .call_native_fn( + &mut Default::default(), &mut Default::default(), state.lib, fn_name, From 760f6c3678353a6c4bcb92b2c8c5fcfaee1ac76b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 8 Nov 2020 10:56:33 +0800 Subject: [PATCH 2/5] Fix no_function build. --- src/engine_api.rs | 19 ++++++++++--------- src/module/mod.rs | 1 + src/parser.rs | 5 ++++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/engine_api.rs b/src/engine_api.rs index fd1ac1c8..6636481f 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -1658,15 +1658,16 @@ impl Engine { mut ast: AST, optimization_level: OptimizationLevel, ) -> AST { - let lib = if cfg!(not(feature = "no_function")) { - ast.lib() - .iter_fn() - .filter(|f| f.func.is_script()) - .map(|f| (**f.func.get_fn_def()).clone()) - .collect() - } else { - Default::default() - }; + #[cfg(not(feature = "no_function"))] + let lib = ast + .lib() + .iter_fn() + .filter(|f| f.func.is_script()) + .map(|f| (**f.func.get_fn_def()).clone()) + .collect(); + + #[cfg(feature = "no_function")] + let lib = Default::default(); let stmt = mem::take(ast.statements_mut()); optimize_into_ast(self, scope, stmt, lib, optimization_level) diff --git a/src/module/mod.rs b/src/module/mod.rs index 6188b64f..20b56455 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1265,6 +1265,7 @@ impl Module { /// Get an iterator to the functions in the module. #[cfg(not(feature = "no_optimize"))] + #[cfg(not(feature = "no_function"))] #[inline(always)] pub(crate) fn iter_fn(&self) -> impl Iterator { self.functions.values() diff --git a/src/parser.rs b/src/parser.rs index f4f08b81..55819dd8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -32,7 +32,7 @@ use crate::{ use crate::stdlib::{ borrow::Cow, boxed::Box, - collections::{HashMap, HashSet}, + collections::HashMap, format, hash::Hash, iter::empty, @@ -45,6 +45,9 @@ use crate::stdlib::{ #[cfg(not(feature = "no_function"))] use crate::stdlib::hash::Hasher; +#[cfg(not(feature = "no_closure"))] +use crate::stdlib::collections::HashSet; + #[cfg(not(feature = "no_std"))] #[cfg(not(feature = "no_function"))] use crate::stdlib::collections::hash_map::DefaultHasher; From 9a669ffe29469d24589338ebe7b7cb00d3207f4d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 8 Nov 2020 11:02:20 +0800 Subject: [PATCH 3/5] Test internal builds. --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 04b3be8e..e8c9cafa 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,7 +18,7 @@ jobs: os: [ubuntu-latest] flags: - "" - - "--features serde" + - "--features serde,internals" - "--features unchecked" - "--features sync" - "--features no_optimize" From 1e07e4356e2296feaed50a3e8d62d480515dec42 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 8 Nov 2020 14:29:54 +0800 Subject: [PATCH 4/5] Re-index imported modules if they are not yet indexed. --- RELEASES.md | 8 +++++++ doc/src/rust/modules/imp-resolver.md | 18 ++++++++++----- src/engine.rs | 12 ++++++++-- src/module/mod.rs | 33 +++++++++++++++++++--------- src/module/resolvers/file.rs | 2 +- src/module/resolvers/stat.rs | 2 +- 6 files changed, 55 insertions(+), 20 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index f1d034d8..e1452780 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,6 +4,14 @@ Rhai Release Notes Version 0.19.5 ============== +This version fixes a bug that prevents compilation with the `internals` feature. +It also speeds up importing modules. + +Bug fixes +--------- + +* Fixes compilation error when using the `internals` feature. Bug introduced in `0.19.4`. + Breaking changes ---------------- diff --git a/doc/src/rust/modules/imp-resolver.md b/doc/src/rust/modules/imp-resolver.md index 2c6dd001..ff6b04d3 100644 --- a/doc/src/rust/modules/imp-resolver.md +++ b/doc/src/rust/modules/imp-resolver.md @@ -14,6 +14,10 @@ When Rhai prepares to load a module, `ModuleResolver::resolve` is called with th of the _module path_ (i.e. the path specified in the [`import`] statement). * Upon success, it should return an [`Rc`][module] (or `Arc` under [`sync`]). + + The module should call `Module::build_index` on the target module before returning. + This method flattens the entire module tree and _indexes_ it for fast function name resolution. + If the module is already indexed, calling this method has no effect. * If the path does not resolve to a valid module, return `EvalAltResult::ErrorModuleNotFound`. @@ -40,12 +44,14 @@ impl ModuleResolver for MyModuleResolver { ) -> Result, Box> { // Check module path. if is_valid_module_path(path) { - load_secret_module(path) // load the custom module - .map(Rc::new) // share it - .map_err(|err| - // Return EvalAltResult::ErrorInModule upon loading error - EvalAltResult::ErrorInModule(err.to_string(), pos).into() - ) + let mut my_module = + load_secret_module(path) // load the custom module + .map_err(|err| + // Return EvalAltResult::ErrorInModule upon loading error + EvalAltResult::ErrorInModule(path.into(), Box::new(err), pos).into() + )?; + my_module.build_index(); // index it + Rc::new(my_module) // make it shared } else { // Return EvalAltResult::ErrorModuleNotFound if the path is invalid Err(EvalAltResult::ErrorModuleNotFound(path.into(), pos).into()) diff --git a/src/engine.rs b/src/engine.rs index 4e563dd1..6e47be93 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -3,7 +3,7 @@ use crate::ast::{BinaryExpr, Expr, FnCallInfo, Ident, IdentX, ReturnType, Stmt}; use crate::dynamic::{map_std_type_name, Dynamic, Union, Variant}; use crate::fn_call::run_builtin_op_assignment; -use crate::fn_native::{Callback, FnPtr, OnVarCallback, Shared}; +use crate::fn_native::{shared_try_take, Callback, FnPtr, OnVarCallback, Shared}; use crate::module::{Module, ModuleRef}; use crate::optimize::OptimizationLevel; use crate::packages::{Package, PackagesCollection, StandardPackage}; @@ -2132,7 +2132,15 @@ impl Engine { let module = resolver.resolve(self, &path, expr.position())?; if let Some(name_def) = alias { - mods.push(name_def.name.clone(), module); + if !module.is_indexed() { + // Index the module (making a clone copy if necessary) if it is not indexed + let mut module = + shared_try_take(module).unwrap_or_else(|m| m.as_ref().clone()); + module.build_index(); + mods.push(name_def.name.clone(), module); + } else { + mods.push(name_def.name.clone(), module); + } } state.modules += 1; diff --git a/src/module/mod.rs b/src/module/mod.rs index 20b56455..c0e3d457 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -60,23 +60,17 @@ pub struct FuncInfo { pub struct Module { /// Sub-modules. modules: HashMap, - /// Module variables. variables: HashMap, - /// Flattened collection of all module variables, including those in sub-modules. all_variables: HashMap, - /// External Rust functions. functions: HashMap, - /// Iterator functions, keyed by the type producing the iterator. type_iterators: HashMap, - /// Flattened collection of all external Rust functions, native or scripted, /// including those in sub-modules. all_functions: HashMap, - /// Is the module indexed? indexed: bool, } @@ -181,6 +175,23 @@ impl Module { && self.type_iterators.is_empty() } + /// Is the module indexed? + /// + /// # Example + /// + /// ``` + /// use rhai::Module; + /// + /// let mut module = Module::new(); + /// assert!(!module.is_indexed()); + /// + /// module.build_index(); + /// assert!(module.is_indexed()); + /// ``` + pub fn is_indexed(&self) -> bool { + self.indexed + } + /// Clone the module, optionally skipping the index. #[inline(always)] fn do_clone(&self, clone_index: bool) -> Self { @@ -1120,7 +1131,7 @@ impl Module { /// Name and Position in `EvalAltResult` are None and must be set afterwards. /// /// The `u64` hash is calculated by the function `crate::calc_native_fn_hash` and must match - /// the hash calculated by `index_all_sub_modules`. + /// the hash calculated by `build_index`. #[inline(always)] pub(crate) fn get_qualified_fn(&self, hash_qualified_fn: u64) -> Option<&CallableFunction> { self.all_functions.get(&hash_qualified_fn) @@ -1396,10 +1407,12 @@ impl Module { Ok(module) } - /// Scan through all the sub-modules in the module build an index of all - /// variables and external Rust functions via hashing. + /// Scan through all the sub-modules in the module and build a hash index of all + /// variables and functions as one flattened namespace. + /// + /// If the module is already indexed, this method has no effect. #[cfg(not(feature = "no_module"))] - pub(crate) fn index_all_sub_modules(&mut self) { + pub fn build_index(&mut self) { // Collect a particular module. fn index_module<'a>( module: &'a Module, diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index b5bb346e..77756247 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -155,7 +155,7 @@ impl ModuleResolver for FileModuleResolver { Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) })?; - m.index_all_sub_modules(); + m.build_index(); let m: Shared = m.into(); module = Some(m.clone()); diff --git a/src/module/resolvers/stat.rs b/src/module/resolvers/stat.rs index 0eeeb8e8..eabfd3e3 100644 --- a/src/module/resolvers/stat.rs +++ b/src/module/resolvers/stat.rs @@ -50,7 +50,7 @@ impl StaticModuleResolver { /// Add a module keyed by its path. #[inline(always)] pub fn insert(&mut self, path: impl Into, mut module: Module) { - module.index_all_sub_modules(); + module.build_index(); self.0.insert(path.into(), module.into()); } /// Remove a module given its path. From b926eba50172601d84a7d77a440ff32a162139ef Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 8 Nov 2020 16:49:59 +0800 Subject: [PATCH 5/5] Fix doc test. --- src/module/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/module/mod.rs b/src/module/mod.rs index c0e3d457..1474d8fc 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -185,8 +185,11 @@ impl Module { /// let mut module = Module::new(); /// assert!(!module.is_indexed()); /// + /// # #[cfg(not(feature = "no_module"))] + /// # { /// module.build_index(); /// assert!(module.is_indexed()); + /// # } /// ``` pub fn is_indexed(&self) -> bool { self.indexed