From 14fe4f9f1be4ba68bf91e6ebc35eeb385427b2ae Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 9 Jan 2021 23:26:50 +0800 Subject: [PATCH 1/3] Change resolve_ast return type. --- src/engine_api.rs | 12 +++++--- src/module/mod.rs | 55 +++++++++++++++++++++++++----------- src/module/resolvers/file.rs | 16 ++++++----- src/module/resolvers/mod.rs | 4 +-- 4 files changed, 57 insertions(+), 30 deletions(-) diff --git a/src/engine_api.rs b/src/engine_api.rs index db3e3efa..4f899fd7 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -936,11 +936,15 @@ impl Engine { while let Some(path) = imports.iter().next() { let path = path.clone(); - if let Some(module_ast) = - self.module_resolver - .resolve_ast(self, &path, Position::NONE)? + match self + .module_resolver + .resolve_ast(self, &path, Position::NONE) { - collect_imports(&module_ast, &mut resolver, &mut imports); + Some(Ok(module_ast)) => { + collect_imports(&module_ast, &mut resolver, &mut imports) + } + Some(err @ Err(_)) => return err, + None => (), } let module = shared_take_or_clone(self.module_resolver.resolve( diff --git a/src/module/mod.rs b/src/module/mod.rs index e7cda5d4..fab754e1 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -168,27 +168,48 @@ impl fmt::Debug for Module { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "Module({}\n modules: {}\n vars: {}\n functions: {}\n)", + "Module({}\n{}{}{})", if let Some(ref id) = self.id { - format!("id: {:?}", id) + format!("id: {:?},", id) } else { "".to_string() }, - self.modules - .keys() - .map(|m| m.as_str()) - .collect::>() - .join(", "), - self.variables - .iter() - .map(|(k, v)| format!("{}={:?}", k, v)) - .collect::>() - .join(", "), - self.functions - .values() - .map(|FuncInfo { func, .. }| func.to_string()) - .collect::>() - .join(", "), + if !self.modules.is_empty() { + format!( + " modules: {}\n", + self.modules + .keys() + .map(|m| m.as_str()) + .collect::>() + .join(", ") + ) + } else { + "".to_string() + }, + if !self.variables.is_empty() { + format!( + " vars: {}\n", + self.variables + .iter() + .map(|(k, v)| format!("{}={:?}", k, v)) + .collect::>() + .join(", ") + ) + } else { + "".to_string() + }, + if !self.functions.is_empty() { + format!( + " functions: {}\n", + self.functions + .values() + .map(|FuncInfo { func, .. }| func.to_string()) + .collect::>() + .join(", ") + ) + } else { + "".to_string() + } ) } } diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index f7e41386..4226eb71 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -233,22 +233,24 @@ impl ModuleResolver for FileModuleResolver { engine: &Engine, path: &str, pos: Position, - ) -> Result, Box> { + ) -> Option>> { // Construct the script file path let mut file_path = self.base_path.clone(); file_path.push(path); file_path.set_extension(&self.extension); // Force extension // Load the script file and compile it - let mut ast = engine.compile_file(file_path).map_err(|err| match *err { + match engine.compile_file(file_path).map_err(|err| match *err { EvalAltResult::ErrorSystem(_, err) if err.is::() => { Box::new(EvalAltResult::ErrorModuleNotFound(path.to_string(), pos)) } _ => Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)), - })?; - - ast.set_source(path); - - Ok(Some(ast)) + }) { + Ok(mut ast) => { + ast.set_source(path); + Some(Ok(ast)) + } + err @ Err(_) => Some(err), + } } } diff --git a/src/module/resolvers/mod.rs b/src/module/resolvers/mod.rs index 17e0c0ac..917c4941 100644 --- a/src/module/resolvers/mod.rs +++ b/src/module/resolvers/mod.rs @@ -44,7 +44,7 @@ pub trait ModuleResolver: SendSync { engine: &Engine, path: &str, pos: Position, - ) -> Result, Box> { - Ok(None) + ) -> Option>> { + None } } From 5b9a18f5b8e9fc90fcb7379a24f51969287e8a7e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 10 Jan 2021 19:34:26 +0800 Subject: [PATCH 2/3] Fix FileModuleResolver::clear_cache_for_path. --- src/module/resolvers/file.rs | 55 +++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index 4226eb71..5d71e0a2 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -7,15 +7,21 @@ use crate::stdlib::{ }; use crate::{Engine, EvalAltResult, Module, ModuleResolver, Position, Shared}; -/// [Module] resolution service that loads [module][Module] script files from the file system. +/// A [module][Module] resolution service that loads [module][Module] script files from the file system. /// -/// Script files are cached so they are are not reloaded and recompiled in subsequent requests. +/// ## Caching /// -/// # Function Namespace +/// Resolved [Modules][Module] are cached internally so script files are not reloaded and recompiled +/// for subsequent requests. /// -/// When a function within a script file module is called, all functions in the _global_ namespace -/// plus all those defined within the same module are _merged_ into a _unified_ namespace before -/// the call. Therefore, functions in a module script can always cross-call each other. +/// Use [`clear_cache`][FileModuleResolver::clear_cache] or +/// [`clear_cache_for_path`][FileModuleResolver::clear_cache_for_path] to clear the internal cache. +/// +/// ## Namespace +/// +/// When a function within a script file module is called, all functions defined within the same +/// script are available, evan `private` ones. In other words, functions defined in a module script +/// can always cross-call each other. /// /// # Example /// @@ -146,6 +152,16 @@ impl FileModuleResolver { self } + /// Is a particular path cached? + #[inline(always)] + pub fn is_cached(&self, path: &str) -> bool { + let file_path = self.get_file_path(path); + + #[cfg(not(feature = "sync"))] + return self.cache.borrow_mut().contains_key(&file_path); + #[cfg(feature = "sync")] + return self.cache.write().unwrap().contains_key(&file_path); + } /// Empty the internal cache. #[inline(always)] pub fn clear_cache(&mut self) { @@ -154,26 +170,34 @@ impl FileModuleResolver { #[cfg(feature = "sync")] self.cache.write().unwrap().clear(); } - /// Remove the specified path from internal cache. /// /// The next time this path is resolved, the script file will be loaded once again. #[inline(always)] - pub fn clear_cache_for_path(&mut self, path: impl AsRef) -> Option> { + pub fn clear_cache_for_path(&mut self, path: &str) -> Option> { + let file_path = self.get_file_path(path); + #[cfg(not(feature = "sync"))] return self .cache .borrow_mut() - .remove_entry(path.as_ref()) + .remove_entry(&file_path) .map(|(_, v)| v); #[cfg(feature = "sync")] return self .cache .write() .unwrap() - .remove_entry(path.as_ref()) + .remove_entry(&file_path) .map(|(_, v)| v); } + /// Construct a full file path. + fn get_file_path(&self, path: &str) -> PathBuf { + let mut file_path = self.base_path.clone(); + file_path.push(path); + file_path.set_extension(&self.extension); // Force extension + file_path + } } impl ModuleResolver for FileModuleResolver { @@ -184,9 +208,7 @@ impl ModuleResolver for FileModuleResolver { pos: Position, ) -> Result, Box> { // Construct the script file path - let mut file_path = self.base_path.clone(); - file_path.push(path); - file_path.set_extension(&self.extension); // Force extension + let file_path = self.get_file_path(path); // See if it is cached { @@ -228,6 +250,9 @@ impl ModuleResolver for FileModuleResolver { Ok(m) } + /// Resolve an `AST` based on a path string. + /// + /// The file system is accessed during each call; the internal cache is by-passed. fn resolve_ast( &self, engine: &Engine, @@ -235,9 +260,7 @@ impl ModuleResolver for FileModuleResolver { pos: Position, ) -> Option>> { // Construct the script file path - let mut file_path = self.base_path.clone(); - file_path.push(path); - file_path.set_extension(&self.extension); // Force extension + let file_path = self.get_file_path(path); // Load the script file and compile it match engine.compile_file(file_path).map_err(|err| match *err { From 8c47d6145690c297c92d78eb6030d6be72135183 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 11 Jan 2021 23:09:33 +0800 Subject: [PATCH 3/3] Refine documentation and comments. --- README.md | 2 +- RELEASES.md | 8 +++- codegen/src/lib.rs | 10 ++-- src/ast.rs | 12 ++--- src/dynamic.rs | 2 + src/module/mod.rs | 73 ++++++++++++++++-------------- src/module/resolvers/collection.rs | 2 +- src/module/resolvers/mod.rs | 2 +- src/module/resolvers/stat.rs | 2 +- src/packages/mod.rs | 6 +-- 10 files changed, 65 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 7a24a1ba..caf5be6c 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ Running `mdbook build` builds it. Playground ---------- -An [Online Playground](https://alvinhochun.github.io/rhai-demo/) is available with syntax-highlighting editor. +An [Online Playground](https://rhaiscript.github.io/playground) is available with syntax-highlighting editor. Scripts can be evaluated directly from the editor. diff --git a/RELEASES.md b/RELEASES.md index 5f251322..582f7273 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,6 +4,11 @@ Rhai Release Notes Version 0.19.10 =============== +Bug fixes +--------- + +* Bug in `FileModuleResolver::clear_cache_for_path` path mapping fixed. + Breaking changes ---------------- @@ -15,7 +20,7 @@ Breaking changes New features ------------ -* `Engine::compile_to_self_contained` compiles a script into an `AST` and _eagerly_ resolves all `import` statements with string literal paths. The resolved modules are directly embedded into the `AST`. When the `AST` is later evaluated, `import` statements directly yield the pre-resolved modules without going through the resolution process once again. +* `Engine::compile_into_self_contained` compiles a script into an `AST` and _eagerly_ resolves all `import` statements with string literal paths. The resolved modules are directly embedded into the `AST`. When the `AST` is later evaluated, `import` statements directly yield the pre-resolved modules without going through the resolution process once again. * `AST::walk`, `Stmt::walk` and `Expr::walk` internal API's to recursively walk an `AST`. Enhancements @@ -24,6 +29,7 @@ Enhancements * Source information is provided when there is an error within a call to a function defined in another module. * Source information is provided to the `NativeCallContext` for native Rust functions. * `EvalAltResult::clear_position` to clear the position information of an error - useful when only the message is needed and the position doesn't need to be printed out. +* A new optional function `resolve_ast` is added to the `ModuleResolver` trait for advanced usage. Version 0.19.9 diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index d71054a0..cdc54041 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -1,4 +1,4 @@ -//! This crate contains procedural macros to make creating Rhai plugin-modules much easier. +//! This crate contains procedural macros to make creating Rhai plugin modules much easier. //! //! # Export an Entire Rust Module to a Rhai `Module` //! @@ -184,7 +184,7 @@ pub fn export_module( proc_macro::TokenStream::from(tokens) } -/// Macro to generate a Rhai `Module` from a _plugin module_ defined via `#[export_module]`. +/// Macro to generate a Rhai `Module` from a _plugin module_ defined via [`#[export_module]`][export_module]. /// /// # Usage /// @@ -223,8 +223,8 @@ pub fn exported_module(module_path: proc_macro::TokenStream) -> proc_macro::Toke /// Functions and variables in the plugin module overrides any existing similarly-named /// functions and variables in the target module. /// -/// This call is intended to be used within the `def_package!` macro to define a custom -/// package based on a plugin module. +/// This call is intended to be used within the [`def_package!`][crate::def_package] macro to define +/// a custom package based on a plugin module. /// /// All sub-modules, if any, in the plugin module are _flattened_ and their functions/variables /// registered at the top level because packages require so. @@ -269,7 +269,7 @@ pub fn combine_with_exported_module(args: proc_macro::TokenStream) -> proc_macro proc_macro::TokenStream::from(tokens) } -/// Macro to register a _plugin function_ (defined via `#[export_fn]`) into an `Engine`. +/// Macro to register a _plugin function_ (defined via [`#[export_fn]`][export_fn]) into an `Engine`. /// /// # Usage /// diff --git a/src/ast.rs b/src/ast.rs index 3f4e273f..e374c83f 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -295,7 +295,7 @@ impl AST { ) -> Option> { self.resolver.clone() } - /// _(INTERNALS)_ Get the embedded [module resolver][`ModuleResolver`]. + /// _(INTERNALS)_ Get the embedded [module resolver][crate::ModuleResolver]. /// Exported under the `internals` feature only. #[cfg(not(feature = "no_module"))] #[cfg(feature = "internals")] @@ -318,7 +318,7 @@ impl AST { /// /// This operation is cheap because functions are shared. /// - /// Not available under [`no_function`]. + /// Not available under `no_function`. #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn clone_functions_only(&self) -> Self { @@ -329,7 +329,7 @@ impl AST { /// /// This operation is cheap because functions are shared. /// - /// Not available under [`no_function`]. + /// Not available under `no_function`. #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn clone_functions_only_filtered( @@ -614,7 +614,7 @@ impl AST { } /// Filter out the functions, retaining only some based on a filter predicate. /// - /// Not available under [`no_function`]. + /// Not available under `no_function`. /// /// # Example /// @@ -661,7 +661,7 @@ impl AST { } /// Iterate through all function definitions. /// - /// Not available under [`no_function`]. + /// Not available under `no_function`. #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn iter_functions<'a>(&'a self) -> impl Iterator + 'a { @@ -671,7 +671,7 @@ impl AST { } /// Clear all function definitions in the [`AST`]. /// - /// Not available under [`no_function`]. + /// Not available under `no_function`. #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn clear_functions(&mut self) { diff --git a/src/dynamic.rs b/src/dynamic.rs index fc5ab807..07346620 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -266,6 +266,8 @@ impl Dynamic { } } /// Is the value held by this [`Dynamic`] shared? + /// + /// Always [`false`] under the `no_closure` feature. #[inline(always)] pub fn is_shared(&self) -> bool { match self.0 { diff --git a/src/module/mod.rs b/src/module/mod.rs index fab754e1..0d9046fe 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -221,6 +221,31 @@ impl AsRef for Module { } } +impl> Add for &Module { + type Output = Module; + + fn add(self, rhs: M) -> Self::Output { + let mut module = self.clone(); + module.merge(rhs.as_ref()); + module + } +} + +impl> Add for Module { + type Output = Self; + + fn add(mut self, rhs: M) -> Self::Output { + self.merge(rhs.as_ref()); + self + } +} + +impl> AddAssign for Module { + fn add_assign(&mut self, rhs: M) { + self.combine(rhs.into()); + } +} + impl Module { /// Create a new [`Module`]. /// @@ -1983,13 +2008,16 @@ impl Module { /// /// This type is volatile and may change. #[derive(Clone, Eq, PartialEq, Default, Hash)] -pub struct NamespaceRef(Option, StaticVec); +pub struct NamespaceRef { + index: Option, + path: StaticVec, +} impl fmt::Debug for NamespaceRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.1, f)?; + fmt::Debug::fmt(&self.path, f)?; - if let Some(index) = self.0 { + if let Some(index) = self.index { write!(f, " -> {}", index) } else { Ok(()) @@ -2001,19 +2029,19 @@ impl Deref for NamespaceRef { type Target = StaticVec; fn deref(&self) -> &Self::Target { - &self.1 + &self.path } } impl DerefMut for NamespaceRef { fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.1 + &mut self.path } } impl fmt::Display for NamespaceRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for Ident { name, .. } in self.1.iter() { + for Ident { name, .. } in self.path.iter() { write!(f, "{}{}", name, Token::DoubleColon.syntax())?; } Ok(()) @@ -2021,45 +2049,20 @@ impl fmt::Display for NamespaceRef { } impl From> for NamespaceRef { - fn from(modules: StaticVec) -> Self { - Self(None, modules) - } -} - -impl> Add for &Module { - type Output = Module; - - fn add(self, rhs: M) -> Self::Output { - let mut module = self.clone(); - module.merge(rhs.as_ref()); - module - } -} - -impl> Add for Module { - type Output = Self; - - fn add(mut self, rhs: M) -> Self::Output { - self.merge(rhs.as_ref()); - self - } -} - -impl> AddAssign for Module { - fn add_assign(&mut self, rhs: M) { - self.combine(rhs.into()); + fn from(path: StaticVec) -> Self { + Self { index: None, path } } } impl NamespaceRef { /// Get the [`Scope`][crate::Scope] index offset. pub(crate) fn index(&self) -> Option { - self.0 + self.index } /// Set the [`Scope`][crate::Scope] index offset. #[cfg(not(feature = "no_module"))] pub(crate) fn set_index(&mut self, index: Option) { - self.0 = index + self.index = index } } diff --git a/src/module/resolvers/collection.rs b/src/module/resolvers/collection.rs index 6a6009c3..03a967b3 100644 --- a/src/module/resolvers/collection.rs +++ b/src/module/resolvers/collection.rs @@ -1,7 +1,7 @@ use crate::stdlib::{boxed::Box, ops::AddAssign, vec::Vec}; use crate::{Engine, EvalAltResult, Module, ModuleResolver, Position, Shared}; -/// [Module] resolution service that holds a collection of [module][Module] resolves, +/// [Module] resolution service that holds a collection of module resolvers, /// to be searched in sequential order. /// /// # Example diff --git a/src/module/resolvers/mod.rs b/src/module/resolvers/mod.rs index 917c4941..bd94e19e 100644 --- a/src/module/resolvers/mod.rs +++ b/src/module/resolvers/mod.rs @@ -29,7 +29,7 @@ pub trait ModuleResolver: SendSync { pos: Position, ) -> Result, Box>; - /// Resolve a module into an `AST` based on a path string. + /// Resolve an `AST` based on a path string. /// /// Returns [`None`] (default) if such resolution is not supported /// (e.g. if the module is Rust-based). diff --git a/src/module/resolvers/stat.rs b/src/module/resolvers/stat.rs index f4705108..4b0c0f36 100644 --- a/src/module/resolvers/stat.rs +++ b/src/module/resolvers/stat.rs @@ -1,7 +1,7 @@ use crate::stdlib::{boxed::Box, collections::HashMap, ops::AddAssign, string::String}; use crate::{Engine, EvalAltResult, Module, ModuleResolver, Position, Shared}; -/// [Module] resolution service that serves [modules][Module] added into it. +/// A static [module][Module] resolution service that serves [modules][Module] added into it. /// /// # Example /// diff --git a/src/packages/mod.rs b/src/packages/mod.rs index a2f41ada..8e8fe21a 100644 --- a/src/packages/mod.rs +++ b/src/packages/mod.rs @@ -50,7 +50,7 @@ pub trait Package { } } -/// Macro that makes it easy to define a _package_ (which is basically a shared module) +/// Macro that makes it easy to define a _package_ (which is basically a shared [module][Module]) /// and register functions into it. /// /// Functions can be added to the package using the standard module methods such as @@ -58,6 +58,8 @@ pub trait Package { /// /// # Example /// +/// Define a package named `MyPackage` with a single function named `my_add`: +/// /// ``` /// use rhai::{Dynamic, EvalAltResult}; /// use rhai::def_package; @@ -70,8 +72,6 @@ pub trait Package { /// lib.set_fn_2("my_add", add); /// }); /// ``` -/// -/// The above defines a package named 'MyPackage' with a single function named 'my_add'. #[macro_export] macro_rules! def_package { ($root:ident : $package:ident : $comment:expr , $lib:ident , $block:stmt) => {