Add complete_namespace to Module::eval_ast_as_new.

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<EvalAltResult>>`.
 * `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
 ------------

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')

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`],
+When given an [`AST`], it is first evaluated, then the following items are exposed as members of the new module:
-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
 ----------------

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<T> = Result<T, Box<EvalAltResult>>;
 
+pub type FuncInfo = (
+    String,
+    FnAccess,
+    usize,
+    Option<StaticVec<TypeId>>,
+    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<u64, Dynamic, StraightHasherBuilder>,
 
     /// External Rust functions.
-    functions: HashMap<
+    functions: HashMap<u64, FuncInfo, StraightHasherBuilder>,
-        u64,
-        (String, FnAccess, usize, Option<StaticVec<TypeId>>, Func),
-        StraightHasherBuilder,
-    >,
 
     /// Iterator functions, keyed by the type producing the iterator.
     type_iterators: HashMap<TypeId, IteratorFn>,
 
     /// Flattened collection of all external Rust functions, native or scripted,
     /// including those in sub-modules.
-    all_functions: HashMap<u64, Func, StraightHasherBuilder>,
+    all_functions: HashMap<u64, CallableFunction, StraightHasherBuilder>,
 
     /// Is the module indexed?
     indexed: bool,
@@ -381,7 +385,7 @@ impl Module {
         name: impl Into<String>,
         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::<A>(&mut args[0])).map(Dynamic::from)
         };
         let arg_types = [TypeId::of::<A>()];
-        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::<A>().unwrap()).map(Dynamic::from)
         };
         let arg_types = [TypeId::of::<A>()];
-        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::<A>(), TypeId::of::<B>()];
-        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::<A>(), TypeId::of::<B>()];
-        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::<A>(), TypeId::of::<B>(), TypeId::of::<C>()];
-        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::<A>(), TypeId::of::<B>(), TypeId::of::<C>()];
-        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::<C>(),
             TypeId::of::<D>(),
         ];
-        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::<C>(),
             TypeId::of::<D>(),
         ];
-        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(
+    pub(crate) fn iter_fn(&self) -> impl Iterator<Item = &FuncInfo> {
-        &self,
-    ) -> impl Iterator<Item = &(String, FnAccess, usize, Option<StaticVec<TypeId>>, Func)> {
         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::<i64>("answer").unwrap(), 42);
     /// # Ok(())
     /// # }
     /// ```
     #[cfg(not(feature = "no_module"))]
-    pub fn eval_ast_as_new(mut scope: Scope, ast: &AST, engine: &Engine) -> FuncReturn<Self> {
+    pub fn eval_ast_as_new(
+        mut scope: Scope,
+        ast: &AST,
+        private_namespace: bool,
+        engine: &Engine,
+    ) -> FuncReturn<Self> {
         let mut mods = Imports::new();
 
         // Run the script
@@ -1201,7 +1268,32 @@ impl Module {
             module.modules.insert(alias.to_string(), m);
         });
 
+        #[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.

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::{
+use crate::stdlib::{boxed::Box, collections::HashMap, path::PathBuf, string::String};
-    boxed::Box,
-    collections::HashMap,
-    path::PathBuf,
-    string::{String, ToString},
-};
 
 #[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 {
+        if let Some(ast) = ast {
-            // Load the file and compile it if not found
-            let ast = engine
-                .compile_file(file_path.clone())
-                .map_err(|err| err.new_position(pos))?;
-
             // 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"))]
+        Ok(module)
-        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)
     }
 }

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 = Module::eval_ast_as_new(scope, ast, false, engine).map_err(|err| {
-                    Module::eval_ast_as_new(scope, ast, engine)
+                    Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos))
-                        .map_err(|err| err.new_position(pos))?,
+                })?;
-                    None,
+                None
-                )
             } else {
                 // Load the file and compile it if not found
-                let ast = engine
+                let ast = engine.compile_file(file_path.clone()).map_err(|err| {
-                    .compile_file(file_path.clone())
+                    Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos))
-                    .map_err(|err| err.new_position(pos))?;
+                })?;
 
-                (
+                module = Module::eval_ast_as_new(scope, &ast, false, engine).map_err(|err| {
-                    Module::eval_ast_as_new(scope, &ast, engine)
+                    Box::new(EvalAltResult::ErrorInModule(path.to_string(), err, pos))
-                        .map_err(|err| err.new_position(pos))?,
+                })?;
-                    Some(ast),
+                Some(ast)
-                )
             }
         };
 

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<EvalAltResult>, Position),
+    /// An error has occurred while loading a module.
+    /// Wrapped value are the name of the module and the interior error.
+    ErrorInModule(String, Box<EvalAltResult>, 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)