From 599fe846cb78dafdc70233cb5d538a1d795bd393 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 25 Sep 2020 18:07:39 +0800 Subject: [PATCH] Add complete_namespace to Module::eval_ast_as_new. --- RELEASES.md | 3 + doc/src/language/modules/ast.md | 8 +- doc/src/language/modules/export.md | 14 ++- src/module/mod.rs | 156 ++++++++++++++++++++++------ src/module/resolvers/file.rs | 78 +++++--------- src/module/resolvers/global_file.rs | 27 +++-- src/result.rs | 7 ++ 7 files changed, 191 insertions(+), 102 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 78716df6..a6e8e522 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -18,6 +18,9 @@ Breaking changes * `Engine::register_set_result` and `Engine::register_indexer_set_result` now take a function that returns `Result<(), Box>`. * `Engine::register_indexer_XXX` and `Module::set_indexer_XXX` panic when the type is `Arrary`, `Map` or `String`. +* `EvalAltResult` has a new variant `ErrorInModule` which holds errors when loading an external module. +* `Module::eval_ast_as_new` now takes an extra boolean parameter, indicating whether to encapsulate the entire module into a separate namespace. +* Functions in `FileModuleResolver` loaded modules now can cross-call each other, but cannot access the global namespace. For the old behavior, use `MergingFileModuleResolver` instead. New features ------------ diff --git a/doc/src/language/modules/ast.md b/doc/src/language/modules/ast.md index 4bf39ace..56cde70d 100644 --- a/doc/src/language/modules/ast.md +++ b/doc/src/language/modules/ast.md @@ -44,7 +44,13 @@ let ast = engine.compile(r#" "#)?; // Convert the 'AST' into a module, using the 'Engine' to evaluate it first -let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine)?; +// +// The second parameter ('private_namespace'), 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. Otherwise module script functions access the global namespace. +// +// This incurs additional overhead, avoidable by setting 'private_namespace' to false. +let module = Module::eval_ast_as_new(Scope::new(), &ast, true, &engine)?; // 'module' now can be loaded into a custom 'Scope' for future use. It contains: // - sub-module: 'foobar' (renamed from 'extra') diff --git a/doc/src/language/modules/export.md b/doc/src/language/modules/export.md index 546b5952..46416bcc 100644 --- a/doc/src/language/modules/export.md +++ b/doc/src/language/modules/export.md @@ -3,10 +3,10 @@ Export Variables, Functions and Sub-Modules in Module {{#include ../../links.md}} -A _module_ is a single script (or pre-compiled [`AST`]) containing global variables, functions and sub-modules. +A _module_ can be created from a single script (or pre-compiled [`AST`]) containing global variables, +functions and sub-modules via the `Module::eval_ast_as_new` method. -A module can be created from a script via the `Module::eval_ast_as_new` method. When given an [`AST`], -it is first evaluated, then the following items are exposed as members of the new module: +When given an [`AST`], it is first evaluated, then the following items are exposed as members of the new module: * Global variables - essentially all variables that remain in the [`Scope`] at the end of a script run - that are exported. Variables not exported (via the `export` statement) remain hidden. @@ -14,6 +14,14 @@ it is first evaluated, then the following items are exposed as members of the ne * Global modules that remain in the [`Scope`] at the end of a script run. +The parameter `private_namespace` in `Module::eval_ast_as_new` determines the exact behavior of +functions exposed by the module and the namespace that they can access: + +| `private_namespace` value | Behavior of module functions | Namespace | Call global functions | Call functions in same module | +| :-----------------------: | ---------------------------------------------------- | :-------: | :-------------------: | :---------------------------: | +| `true` | encapsulate the entire `AST` into each function call | module | no | yes | +| `false` | register each function independently | global | yes | no | + Global Variables ---------------- diff --git a/src/module/mod.rs b/src/module/mod.rs index ea773abc..75abda36 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -3,7 +3,7 @@ use crate::any::{Dynamic, Variant}; use crate::calc_fn_hash; use crate::engine::Engine; -use crate::fn_native::{CallableFunction as Func, FnCallArgs, IteratorFn, SendSync}; +use crate::fn_native::{CallableFunction, FnCallArgs, IteratorFn, SendSync}; use crate::fn_register::by_value as cast_arg; use crate::parser::{FnAccess, FnAccess::Public}; use crate::result::EvalAltResult; @@ -41,6 +41,14 @@ use crate::stdlib::{ /// Return type of module-level Rust function. pub type FuncReturn = Result>; +pub type FuncInfo = ( + String, + FnAccess, + usize, + Option>, + CallableFunction, +); + /// An imported module, which may contain variables, sub-modules, /// external Rust functions, and script-defined functions. /// @@ -57,18 +65,14 @@ pub struct Module { all_variables: HashMap, /// External Rust functions. - functions: HashMap< - u64, - (String, FnAccess, usize, Option>, Func), - StraightHasherBuilder, - >, + 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, + all_functions: HashMap, /// Is the module indexed? indexed: bool, @@ -381,7 +385,7 @@ impl Module { name: impl Into, access: FnAccess, arg_types: &[TypeId], - func: Func, + func: CallableFunction, ) -> u64 { let name = name.into(); @@ -481,7 +485,12 @@ impl Module { let f = move |engine: &Engine, lib: &Module, args: &mut FnCallArgs| { func(engine, lib, args).map(Dynamic::from) }; - self.set_fn(name, Public, arg_types, Func::from_method(Box::new(f))) + self.set_fn( + name, + Public, + arg_types, + CallableFunction::from_method(Box::new(f)), + ) } /// Set a raw function but with a signature that is a scripted function (meaning that the types @@ -507,7 +516,7 @@ impl Module { FnAccess::Public, num_args, None, - Func::from_pure(Box::new(f)), + CallableFunction::from_pure(Box::new(f)), ), ); self.indexed = false; @@ -534,7 +543,12 @@ impl Module { ) -> u64 { let f = move |_: &Engine, _: &Module, _: &mut FnCallArgs| func().map(Dynamic::from); let arg_types = []; - self.set_fn(name, Public, &arg_types, Func::from_pure(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_pure(Box::new(f)), + ) } /// Set a Rust function taking one parameter into the module, returning a hash key. @@ -559,7 +573,12 @@ impl Module { func(cast_arg::(&mut args[0])).map(Dynamic::from) }; let arg_types = [TypeId::of::()]; - self.set_fn(name, Public, &arg_types, Func::from_pure(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_pure(Box::new(f)), + ) } /// Set a Rust function taking one mutable parameter into the module, returning a hash key. @@ -584,7 +603,12 @@ impl Module { func(&mut args[0].write_lock::().unwrap()).map(Dynamic::from) }; let arg_types = [TypeId::of::()]; - self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_method(Box::new(f)), + ) } /// Set a Rust getter function taking one mutable parameter, returning a hash key. @@ -636,7 +660,12 @@ impl Module { func(a, b).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::()]; - self.set_fn(name, Public, &arg_types, Func::from_pure(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_pure(Box::new(f)), + ) } /// Set a Rust function taking two parameters (the first one mutable) into the module, @@ -667,7 +696,12 @@ impl Module { func(a, b).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::()]; - self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_method(Box::new(f)), + ) } /// Set a Rust setter function taking two parameters (the first one mutable) into the module, @@ -772,7 +806,12 @@ impl Module { func(a, b, c).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; - self.set_fn(name, Public, &arg_types, Func::from_pure(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_pure(Box::new(f)), + ) } /// Set a Rust function taking three parameters (the first one mutable) into the module, @@ -809,7 +848,12 @@ impl Module { func(a, b, c).map(Dynamic::from) }; let arg_types = [TypeId::of::(), TypeId::of::(), TypeId::of::()]; - self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_method(Box::new(f)), + ) } /// Set a Rust index setter taking three parameters (the first one mutable) into the module, @@ -865,7 +909,7 @@ impl Module { FN_IDX_SET, Public, &arg_types, - Func::from_method(Box::new(f)), + CallableFunction::from_method(Box::new(f)), ) } @@ -949,7 +993,12 @@ impl Module { TypeId::of::(), TypeId::of::(), ]; - self.set_fn(name, Public, &arg_types, Func::from_pure(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_pure(Box::new(f)), + ) } /// Set a Rust function taking four parameters (the first one mutable) into the module, @@ -993,14 +1042,19 @@ impl Module { TypeId::of::(), TypeId::of::(), ]; - self.set_fn(name, Public, &arg_types, Func::from_method(Box::new(f))) + self.set_fn( + name, + Public, + &arg_types, + CallableFunction::from_method(Box::new(f)), + ) } /// Get a Rust function. /// /// The `u64` hash is calculated by the function `crate::calc_fn_hash`. /// It is also returned by the `set_fn_XXX` calls. - pub(crate) fn get_fn(&self, hash_fn: u64, public_only: bool) -> Option<&Func> { + pub(crate) fn get_fn(&self, hash_fn: u64, public_only: bool) -> Option<&CallableFunction> { if hash_fn == 0 { None } else { @@ -1019,7 +1073,7 @@ impl Module { /// /// The `u64` hash is calculated by the function `crate::calc_fn_hash` and must match /// the hash calculated by `index_all_sub_modules`. - pub(crate) fn get_qualified_fn(&self, hash_qualified_fn: u64) -> Option<&Func> { + pub(crate) fn get_qualified_fn(&self, hash_qualified_fn: u64) -> Option<&CallableFunction> { self.all_functions.get(&hash_qualified_fn) } @@ -1084,7 +1138,9 @@ impl Module { .iter() .filter(|(_, (_, _, _, _, v))| match v { #[cfg(not(feature = "no_function"))] - Func::Script(ref f) => _filter(f.access, f.name.as_str(), f.params.len()), + CallableFunction::Script(f) => { + _filter(f.access, f.name.as_str(), f.params.len()) + } _ => true, }) .map(|(&k, v)| (k, v.clone())), @@ -1106,7 +1162,7 @@ impl Module { mut filter: impl FnMut(FnAccess, &str, usize) -> bool, ) -> &mut Self { self.functions.retain(|_, (_, _, _, _, v)| match v { - Func::Script(ref f) => filter(f.access, f.name.as_str(), f.params.len()), + CallableFunction::Script(f) => filter(f.access, f.name.as_str(), f.params.len()), _ => true, }); @@ -1135,9 +1191,7 @@ impl Module { } /// Get an iterator to the functions in the module. - pub(crate) fn iter_fn( - &self, - ) -> impl Iterator>, Func)> { + pub(crate) fn iter_fn(&self) -> impl Iterator { self.functions.values() } @@ -1156,13 +1210,21 @@ impl Module { self.functions .iter() .for_each(|(_, (_, _, _, _, v))| match v { - Func::Script(ref f) => action(f.access, f.name.as_str(), f.params.len()), + CallableFunction::Script(f) => action(f.access, f.name.as_str(), f.params.len()), _ => (), }); } /// Create a new `Module` by evaluating an `AST`. /// + /// ### `private_namespace` parameter + /// + /// If `true`, the entire `AST` is encapsulated into each function as a private namespace, + /// allowing functions to cross-call each other. + /// + /// If `false`, each function is registered independently and cannot cross-call + /// each other. Functions are searched in the global namespace. + /// /// # Examples /// /// ``` @@ -1171,14 +1233,19 @@ 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, &engine)?; + /// let module = Module::eval_ast_as_new(Scope::new(), &ast, &engine, true)?; /// 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, engine: &Engine) -> FuncReturn { + pub fn eval_ast_as_new( + mut scope: Scope, + ast: &AST, + private_namespace: bool, + engine: &Engine, + ) -> FuncReturn { let mut mods = Imports::new(); // Run the script @@ -1201,7 +1268,32 @@ impl Module { module.modules.insert(alias.to_string(), m); }); - module.merge(ast.lib()); + #[cfg(not(feature = "no_function"))] + if private_namespace { + ast.iter_functions(|access, name, num_args| match access { + FnAccess::Private => (), + FnAccess::Public => { + let fn_name = name.to_string(); + let ast_lib = ast.lib().clone(); + + module.set_raw_fn_as_scripted( + name, + num_args, + move |engine: &Engine, _, args: &mut [&mut Dynamic]| { + engine.call_fn_dynamic_raw( + &mut Scope::new(), + &ast_lib, + &fn_name, + &mut None, + args, + ) + }, + ); + } + }); + } else { + module.merge(ast.lib()); + } Ok(module) } @@ -1215,7 +1307,7 @@ impl Module { module: &'a Module, qualifiers: &mut Vec<&'a str>, variables: &mut Vec<(u64, Dynamic)>, - functions: &mut Vec<(u64, Func)>, + functions: &mut Vec<(u64, CallableFunction)>, ) { for (name, m) in &module.modules { // Index all the sub-modules first. diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index 92243cb3..bd49b429 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -1,17 +1,10 @@ -use crate::any::Dynamic; use crate::engine::Engine; use crate::module::{Module, ModuleResolver}; -use crate::parser::{FnAccess, AST}; +use crate::parser::AST; use crate::result::EvalAltResult; -use crate::scope::Scope; use crate::token::Position; -use crate::stdlib::{ - boxed::Box, - collections::HashMap, - path::PathBuf, - string::{String, ToString}, -}; +use crate::stdlib::{boxed::Box, collections::HashMap, path::PathBuf, string::String}; #[cfg(not(feature = "sync"))] use crate::stdlib::cell::RefCell; @@ -149,61 +142,42 @@ impl ModuleResolver for FileModuleResolver { file_path.push(path); file_path.set_extension(&self.extension); // Force extension + let scope = Default::default(); + let module; + // See if it is cached - let exists = { + let ast = { #[cfg(not(feature = "sync"))] let c = self.cache.borrow(); #[cfg(feature = "sync")] let c = self.cache.read().unwrap(); - c.contains_key(&file_path) + if let Some(ast) = c.get(&file_path) { + module = Module::eval_ast_as_new(scope, ast, true, 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, true, engine).map_err(|err| { + Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) + })?; + Some(ast) + } }; - if !exists { - // Load the file and compile it if not found - let ast = engine - .compile_file(file_path.clone()) - .map_err(|err| err.new_position(pos))?; - + if let Some(ast) = ast { // Put it into the cache #[cfg(not(feature = "sync"))] - self.cache.borrow_mut().insert(file_path.clone(), ast); + self.cache.borrow_mut().insert(file_path, ast); #[cfg(feature = "sync")] - self.cache.write().unwrap().insert(file_path.clone(), ast); + self.cache.write().unwrap().insert(file_path, ast); } - #[cfg(not(feature = "sync"))] - let c = self.cache.borrow(); - #[cfg(feature = "sync")] - let c = self.cache.read().unwrap(); - - let ast = c.get(&file_path).unwrap(); - - let mut _module = Module::eval_ast_as_new(Scope::new(), ast, engine)?; - - #[cfg(not(feature = "no_function"))] - ast.iter_functions(|access, name, num_args| match access { - FnAccess::Private => (), - FnAccess::Public => { - let fn_name = name.to_string(); - let ast_lib = ast.lib().clone(); - - _module.set_raw_fn_as_scripted( - name, - num_args, - move |engine: &Engine, _, args: &mut [&mut Dynamic]| { - engine.call_fn_dynamic_raw( - &mut Scope::new(), - &ast_lib, - &fn_name, - &mut None, - args, - ) - }, - ); - } - }); - - Ok(_module) + Ok(module) } } diff --git a/src/module/resolvers/global_file.rs b/src/module/resolvers/global_file.rs index f0b0cce8..f9445d09 100644 --- a/src/module/resolvers/global_file.rs +++ b/src/module/resolvers/global_file.rs @@ -148,31 +148,30 @@ impl ModuleResolver for GlobalFileModuleResolver { file_path.set_extension(&self.extension); // Force extension let scope = Default::default(); + let module; // See if it is cached - let (module, ast) = { + 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::eval_ast_as_new(scope, ast, engine) - .map_err(|err| err.new_position(pos))?, - None, - ) + 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| err.new_position(pos))?; + let ast = engine.compile_file(file_path.clone()).map_err(|err| { + Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) + })?; - ( - Module::eval_ast_as_new(scope, &ast, engine) - .map_err(|err| err.new_position(pos))?, - Some(ast), - ) + module = Module::eval_ast_as_new(scope, &ast, false, engine).map_err(|err| { + Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos)) + })?; + Some(ast) } }; diff --git a/src/result.rs b/src/result.rs index 40f2935c..e1f9ebf3 100644 --- a/src/result.rs +++ b/src/result.rs @@ -39,6 +39,9 @@ pub enum EvalAltResult { /// An error has occurred inside a called function. /// Wrapped values are the name of the function and the interior error. ErrorInFunctionCall(String, Box, Position), + /// An error has occurred while loading a module. + /// Wrapped value are the name of the module and the interior error. + ErrorInModule(String, Box, Position), /// Access to `this` that is not bound. ErrorUnboundThis(Position), /// Non-boolean operand encountered for boolean operator. Wrapped value is the operator. @@ -113,6 +116,7 @@ impl EvalAltResult { Self::ErrorParsing(p, _) => p.desc(), Self::ErrorInFunctionCall(_, _, _) => "Error in called function", + Self::ErrorInModule(_, _, _) => "Error in module", Self::ErrorFunctionNotFound(_, _) => "Function not found", Self::ErrorUnboundThis(_) => "'this' is not bound", Self::ErrorBooleanArgMismatch(_, _) => "Boolean operator expects boolean operands", @@ -180,6 +184,7 @@ impl fmt::Display for EvalAltResult { Self::ErrorInFunctionCall(s, err, _) => { write!(f, "Error in call to function '{}' : {}", s, err)? } + Self::ErrorInModule(s, err, _) => write!(f, "Error in module '{}' : {}", s, err)?, Self::ErrorFunctionNotFound(s, _) | Self::ErrorVariableNotFound(s, _) @@ -280,6 +285,7 @@ impl EvalAltResult { Self::ErrorParsing(_, pos) | Self::ErrorFunctionNotFound(_, pos) | Self::ErrorInFunctionCall(_, _, pos) + | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) | Self::ErrorBooleanArgMismatch(_, pos) | Self::ErrorCharMismatch(pos) @@ -321,6 +327,7 @@ impl EvalAltResult { Self::ErrorParsing(_, pos) | Self::ErrorFunctionNotFound(_, pos) | Self::ErrorInFunctionCall(_, _, pos) + | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) | Self::ErrorBooleanArgMismatch(_, pos) | Self::ErrorCharMismatch(pos)