From 1e07e4356e2296feaed50a3e8d62d480515dec42 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 8 Nov 2020 14:29:54 +0800 Subject: [PATCH] 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.