Private global functions are still exposed.

src/ast.rs

@@ -38,25 +38,6 @@ pub enum FnAccess {
     Private,
 }
 
-impl FnAccess {
-    /// Is this access mode [private][FnAccess::Private]?
-    #[inline(always)]
-    pub fn is_private(self) -> bool {
-        match self {
-            Self::Private => true,
-            Self::Public => false,
-        }
-    }
-    /// Is this access mode [public][FnAccess::Public]?
-    #[inline(always)]
-    pub fn is_public(self) -> bool {
-        match self {
-            Self::Private => false,
-            Self::Public => true,
-        }
-    }
-}
-
 /// _(INTERNALS)_ A type containing information on a scripted function.
 /// Exported under the `internals` feature only.
 ///
@@ -91,10 +72,9 @@ impl fmt::Display for ScriptFnDef {
         write!(
             f,
             "{}{}({})",
-            if self.access.is_private() {
+            match self.access {
-                "private "
+                FnAccess::Public => "",
-            } else {
+                FnAccess::Private => "private",
-                ""
             },
             self.name,
             self.params
@@ -134,10 +114,9 @@ impl fmt::Display for ScriptFnMetadata<'_> {
         write!(
             f,
             "{}{}({})",
-            if self.access.is_private() {
+            match self.access {
-                "private "
+                FnAccess::Public => "",
-            } else {
+                FnAccess::Private => "private",
-                ""
             },
             self.name,
             self.params.iter().cloned().collect::<Vec<_>>().join(", ")

src/engine.rs

@@ -107,6 +107,11 @@ impl Imports {
     pub(crate) fn iter_raw(&self) -> impl Iterator<Item = (&ImmutableString, &Shared<Module>)> {
         self.0.iter().rev().map(|(n, m)| (n, m))
     }
+    /// Get an iterator to this stack of imported [modules][Module] in forward order.
+    #[inline(always)]
+    pub(crate) fn scan_raw(&self) -> impl Iterator<Item = (&ImmutableString, &Shared<Module>)> {
+        self.0.iter().map(|(n, m)| (n, m))
+    }
     /// Get a consuming iterator to this stack of imported [modules][Module] in reverse order.
     #[inline(always)]
     pub fn into_iter(self) -> impl Iterator<Item = (ImmutableString, Shared<Module>)> {
@@ -1874,39 +1879,51 @@ impl Engine {
         lib: &[&Module],
         this_ptr: &mut Option<&mut Dynamic>,
         statements: &[Stmt],
-        restore: bool,
+        restore_prev_state: bool,
         level: usize,
     ) -> RhaiResult {
-        let mut _has_imports = false;
+        let mut _restore_fn_resolution_cache = false;
         let prev_always_search = state.always_search;
         let prev_scope_len = scope.len();
         let prev_mods_len = mods.len();
 
-        if restore {
+        if restore_prev_state {
             state.scope_level += 1;
         }
 
         let result = statements.iter().try_fold(Dynamic::UNIT, |_, stmt| {
+            let _mods_len = mods.len();
+
+            let r = self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level)?;
+
             #[cfg(not(feature = "no_module"))]
-            match stmt {
+            if matches!(stmt, Stmt::Import(_, _, _)) {
-                Stmt::Import(_, _, _) => {
+                // Get the extra modules - see if any functions are marked global.
-                    // When imports list is modified, clear the functions lookup cache
+                // Without global functions, the extra modules never affect function resolution.
-                    if _has_imports {
+                if mods
+                    .scan_raw()
+                    .skip(_mods_len)
+                    .any(|(_, m)| m.has_namespace(crate::FnNamespace::Global, true))
+                {
+                    if _restore_fn_resolution_cache {
+                        // When new module is imported with global functions and there is already
+                        // a new cache, clear it - notice that this is expensive as all function
+                        // resolutions must start again
                         state.clear_fn_resolution_cache();
-                    } else if restore {
+                    } else if restore_prev_state {
+                        // When new module is imported with global functions, push a new cache
                         state.push_fn_resolution_cache();
-                        _has_imports = true;
+                        _restore_fn_resolution_cache = true;
                     }
                 }
-                _ => (),
             }
 
-            self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level)
+            Ok(r)
         });
 
-        if restore {
+        if restore_prev_state {
             scope.rewind(prev_scope_len);
-            if _has_imports {
+            if _restore_fn_resolution_cache {
                 // If imports list is modified, pop the functions lookup cache
                 state.pop_fn_resolution_cache();
             }

src/engine_api.rs

@@ -33,9 +33,9 @@ impl Engine {
     /// Arguments are simply passed in as a mutable array of [`&mut Dynamic`][Dynamic],
     /// The arguments are guaranteed to be of the correct types matching the [`TypeId`][std::any::TypeId]'s.
     ///
-    /// To access a primary parameter value (i.e. cloning is cheap), use: `args[n].clone().cast::<T>()`
+    /// To access a primary argument value (i.e. cloning is cheap), use: `args[n].clone().cast::<T>()`
     ///
-    /// To access a parameter value and avoid cloning, use `std::mem::take(args[n]).cast::<T>()`.
+    /// To access an argument value and avoid cloning, use `std::mem::take(args[n]).cast::<T>()`.
     /// Notice that this will _consume_ the argument, replacing it with `()`.
     ///
     /// To access the first mutable parameter, use `args.get_mut(0).unwrap()`

src/fn_register.rs

@@ -131,7 +131,7 @@ macro_rules! make_func {
 			$($let)*
 			$($par = ($convert)(_drain.next().unwrap()); )*
 
-            // Call the function with each parameter value
+            // Call the function with each argument value
 			let r = $fn($($arg),*);
 
             // Map the result

src/module/mod.rs

@@ -47,25 +47,6 @@ impl Default for FnNamespace {
     }
 }
 
-impl FnNamespace {
-    /// Is this namespace [global][FnNamespace::Global]?
-    #[inline(always)]
-    pub fn is_global(self) -> bool {
-        match self {
-            Self::Global => true,
-            Self::Internal => false,
-        }
-    }
-    /// Is this namespace [internal][FnNamespace::Internal]?
-    #[inline(always)]
-    pub fn is_internal(self) -> bool {
-        match self {
-            Self::Global => false,
-            Self::Internal => true,
-        }
-    }
-}
-
 /// Data structure containing a single registered function.
 #[derive(Debug, Clone)]
 pub struct FuncInfo {
@@ -372,12 +353,15 @@ impl Module {
         self.indexed
     }
 
-    /// Generate signatures for all the functions in the [`Module`].
+    /// Generate signatures for all the non-private functions in the [`Module`].
     #[inline(always)]
     pub fn gen_fn_signatures(&self) -> impl Iterator<Item = String> + '_ {
         self.functions
             .values()
-            .filter(|FuncInfo { access, .. }| !access.is_private())
+            .filter(|FuncInfo { access, .. }| match access {
+                FnAccess::Public => true,
+                FnAccess::Private => false,
+            })
             .map(FuncInfo::gen_signature)
     }
 
@@ -619,7 +603,10 @@ impl Module {
         if public_only {
             self.functions
                 .get(&hash_fn)
-                .map_or(false, |FuncInfo { access, .. }| access.is_public())
+                .map_or(false, |FuncInfo { access, .. }| match access {
+                    FnAccess::Public => true,
+                    FnAccess::Private => false,
+                })
         } else {
             self.functions.contains_key(&hash_fn)
         }
@@ -735,6 +722,8 @@ impl Module {
     ///
     /// This function is very low level.
     ///
+    /// # Arguments
+    ///
     /// A list of [`TypeId`]'s is taken as the argument types.
     ///
     /// Arguments are simply passed in as a mutable array of [`&mut Dynamic`][Dynamic],
@@ -743,12 +732,12 @@ impl Module {
     /// The function is assumed to be a _method_, meaning that the first argument should not be consumed.
     /// All other arguments can be consumed.
     ///
-    /// To access a primary parameter value (i.e. cloning is cheap), use: `args[n].clone().cast::<T>()`
+    /// To access a primary argument value (i.e. cloning is cheap), use: `args[n].clone().cast::<T>()`
     ///
-    /// To access a parameter value and avoid cloning, use `std::mem::take(args[n]).cast::<T>()`.
+    /// To access an argument value and avoid cloning, use `std::mem::take(args[n]).cast::<T>()`.
     /// Notice that this will _consume_ the argument, replacing it with `()`.
     ///
-    /// To access the first mutable parameter, use `args.get_mut(0).unwrap()`
+    /// To access the first mutable argument, use `args.get_mut(0).unwrap()`
     ///
     /// # Function Metadata
     ///
@@ -1812,7 +1801,11 @@ impl Module {
         ast.lib()
             .functions
             .values()
-            .filter(|FuncInfo { access, func, .. }| !access.is_private() && func.is_script())
+            .filter(|FuncInfo { access, .. }| match access {
+                FnAccess::Public => true,
+                FnAccess::Private => false,
+            })
+            .filter(|FuncInfo { func, .. }| func.is_script())
             .for_each(|FuncInfo { func, .. }| {
                 // Encapsulate AST environment
                 let mut func = func.get_fn_def().clone();
@@ -1827,6 +1820,31 @@ impl Module {
         Ok(module)
     }
 
+    /// Are there functions (or type iterators) marked for the specified namespace?
+    pub fn has_namespace(&self, namespace: FnNamespace, recursive: bool) -> bool {
+        // Type iterators are default global
+        if !self.type_iterators.is_empty() {
+            return true;
+        }
+        // Any function marked global?
+        if self.functions.values().any(|f| f.namespace == namespace) {
+            return true;
+        }
+
+        // Scan sub-modules
+        if recursive {
+            if self
+                .modules
+                .values()
+                .any(|m| m.has_namespace(namespace, recursive))
+            {
+                return true;
+            }
+        }
+
+        false
+    }
+
     /// Scan through all the sub-modules in the [`Module`] and build a hash index of all
     /// variables and functions as one flattened namespace.
     ///
@@ -1860,26 +1878,30 @@ impl Module {
             });
 
             // Index all Rust functions
-            module
+            module.functions.iter().for_each(
-                .functions
-                .iter()
-                .filter(|(_, FuncInfo { access, .. })| access.is_public())
-                .for_each(
                 |(
                     &hash,
                     FuncInfo {
                         name,
                         namespace,
+                        access,
                         params,
                         param_types,
                         func,
                         ..
                     },
                 )| {
+                    match namespace {
+                        FnNamespace::Global => {
                             // Flatten all functions with global namespace
-                        if namespace.is_global() {
                             functions.insert(hash, func.clone());
                         }
+                        FnNamespace::Internal => (),
+                    }
+                    match access {
+                        FnAccess::Public => (),
+                        FnAccess::Private => return, // Do not index private functions
+                    }
 
                     let hash_qualified_script =
                         crate::calc_script_fn_hash(qualifiers.iter().cloned(), name, *params)
@@ -1893,15 +1915,11 @@ impl Module {
                         //    i.e. qualifiers + function name + number of arguments.
                         // 2) Calculate a second hash with no qualifiers, empty function name,
                         //    and the actual list of argument [`TypeId`]'.s
-                            let hash_fn_args = crate::calc_native_fn_hash(
+                        let hash_fn_args =
-                                empty(),
+                            crate::calc_native_fn_hash(empty(), "", param_types.iter().cloned())
-                                "",
-                                param_types.iter().cloned(),
-                            )
                                 .unwrap();
                         // 3) The two hashes are combined.
-                            let hash_qualified_fn =
+                        let hash_qualified_fn = combine_hashes(hash_qualified_script, hash_fn_args);
-                                combine_hashes(hash_qualified_script, hash_fn_args);
 
                         functions.insert(hash_qualified_fn, func.clone());
                     } else if cfg!(not(feature = "no_function")) {

tests/modules.rs

@@ -24,7 +24,10 @@ fn test_module_sub_module() -> Result<(), Box<EvalAltResult>> {
     sub_module2.set_var("answer", 41 as INT);
 
     let hash_inc = sub_module2.set_fn_1_mut("inc", FnNamespace::Internal, |x: &mut INT| Ok(*x + 1));
+    assert!(!sub_module2.has_namespace(FnNamespace::Global, true));
+
     sub_module2.set_fn_1_mut("super_inc", FnNamespace::Global, |x: &mut INT| Ok(*x + 1));
+    assert!(sub_module2.has_namespace(FnNamespace::Global, true));
 
     #[cfg(not(feature = "no_object"))]
     sub_module2.set_getter_fn("doubled", |x: &mut INT| Ok(*x * 2));
@@ -33,6 +36,9 @@ fn test_module_sub_module() -> Result<(), Box<EvalAltResult>> {
     module.set_sub_module("life", sub_module);
     module.set_var("MYSTIC_NUMBER", Dynamic::from(42 as INT));
 
+    assert!(module.has_namespace(FnNamespace::Global, true));
+    assert!(!module.has_namespace(FnNamespace::Global, false));
+
     assert!(module.contains_sub_module("life"));
     let m = module.get_sub_module("life").unwrap();