From 2c8b15c74022bd42340203d3326a3530b5ce02d2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 3 Mar 2021 11:40:27 +0800 Subject: [PATCH] Private global functions are still exposed. --- src/ast.rs | 33 ++-------- src/engine.rs | 43 ++++++++---- src/engine_api.rs | 4 +- src/fn_register.rs | 2 +- src/module/mod.rs | 158 +++++++++++++++++++++++++-------------------- tests/modules.rs | 6 ++ 6 files changed, 133 insertions(+), 113 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index b43e1c8c..050ea197 100644 --- a/src/ast.rs +++ b/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() { - "private " - } else { - "" + match self.access { + FnAccess::Public => "", + FnAccess::Private => "private", }, self.name, self.params @@ -134,10 +114,9 @@ impl fmt::Display for ScriptFnMetadata<'_> { write!( f, "{}{}({})", - if self.access.is_private() { - "private " - } else { - "" + match self.access { + FnAccess::Public => "", + FnAccess::Private => "private", }, self.name, self.params.iter().cloned().collect::>().join(", ") diff --git a/src/engine.rs b/src/engine.rs index a3d09842..bca6b01f 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -107,6 +107,11 @@ impl Imports { pub(crate) fn iter_raw(&self) -> impl Iterator)> { 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)> { + 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)> { @@ -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 { - Stmt::Import(_, _, _) => { - // When imports list is modified, clear the functions lookup cache - if _has_imports { + if matches!(stmt, Stmt::Import(_, _, _)) { + // Get the extra modules - see if any functions are marked global. + // Without global functions, the extra modules never affect function resolution. + 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(); } diff --git a/src/engine_api.rs b/src/engine_api.rs index 45edace6..69a2ff0e 100644 --- a/src/engine_api.rs +++ b/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::()` + /// To access a primary argument value (i.e. cloning is cheap), use: `args[n].clone().cast::()` /// - /// To access a parameter value and avoid cloning, use `std::mem::take(args[n]).cast::()`. + /// To access an argument value and avoid cloning, use `std::mem::take(args[n]).cast::()`. /// Notice that this will _consume_ the argument, replacing it with `()`. /// /// To access the first mutable parameter, use `args.get_mut(0).unwrap()` diff --git a/src/fn_register.rs b/src/fn_register.rs index 77329412..f35a26f6 100644 --- a/src/fn_register.rs +++ b/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 diff --git a/src/module/mod.rs b/src/module/mod.rs index 1317eec6..4d066db5 100644 --- a/src/module/mod.rs +++ b/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 + '_ { 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::()` + /// To access a primary argument value (i.e. cloning is cheap), use: `args[n].clone().cast::()` /// - /// To access a parameter value and avoid cloning, use `std::mem::take(args[n]).cast::()`. + /// To access an argument value and avoid cloning, use `std::mem::take(args[n]).cast::()`. /// 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,55 +1878,55 @@ impl Module { }); // Index all Rust functions - module - .functions - .iter() - .filter(|(_, FuncInfo { access, .. })| access.is_public()) - .for_each( - |( - &hash, - FuncInfo { - name, - namespace, - params, - param_types, - func, - .. - }, - )| { - // Flatten all functions with global namespace - if namespace.is_global() { + module.functions.iter().for_each( + |( + &hash, + FuncInfo { + name, + namespace, + access, + params, + param_types, + func, + .. + }, + )| { + match namespace { + FnNamespace::Global => { + // Flatten all functions with global namespace 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) - .unwrap(); - - if !func.is_script() { - assert_eq!(*params, param_types.len()); - - // Namespace-qualified Rust functions are indexed in two steps: - // 1) Calculate a hash in a similar manner to script-defined functions, - // 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( - empty(), - "", - param_types.iter().cloned(), - ) + let hash_qualified_script = + crate::calc_script_fn_hash(qualifiers.iter().cloned(), name, *params) .unwrap(); - // 3) The two hashes are combined. - let hash_qualified_fn = - combine_hashes(hash_qualified_script, hash_fn_args); - functions.insert(hash_qualified_fn, func.clone()); - } else if cfg!(not(feature = "no_function")) { - functions.insert(hash_qualified_script, func.clone()); - } - }, - ); + if !func.is_script() { + assert_eq!(*params, param_types.len()); + + // Namespace-qualified Rust functions are indexed in two steps: + // 1) Calculate a hash in a similar manner to script-defined functions, + // 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(empty(), "", param_types.iter().cloned()) + .unwrap(); + // 3) The two hashes are combined. + let hash_qualified_fn = combine_hashes(hash_qualified_script, hash_fn_args); + + functions.insert(hash_qualified_fn, func.clone()); + } else if cfg!(not(feature = "no_function")) { + functions.insert(hash_qualified_script, func.clone()); + } + }, + ); } if !self.indexed { diff --git a/tests/modules.rs b/tests/modules.rs index aadf2a5d..c42111b6 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -24,7 +24,10 @@ fn test_module_sub_module() -> Result<(), Box> { 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> { 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();