Unify function hash calculations, pre-hash module-qualified function calls.

src/api.rs

@@ -999,7 +999,7 @@ impl Engine {
         let pos = Position::none();
 
         let fn_def = fn_lib
-            .get_function(name, args.len())
+            .get_function_by_signature(name, args.len())
             .ok_or_else(|| Box::new(EvalAltResult::ErrorFunctionNotFound(name.to_string(), pos)))?;
 
         let state = State::new(fn_lib);

src/engine.rs

@@ -11,8 +11,8 @@ use crate::packages::{
 use crate::parser::{Expr, FnDef, ReturnType, Stmt, AST};
 use crate::result::EvalAltResult;
 use crate::scope::{EntryType as ScopeEntryType, Scope};
-use crate::token::{Position, Token};
+use crate::token::Position;
-use crate::utils::{calc_fn_def, StaticVec};
+use crate::utils::{StaticVec, EMPTY_TYPE_ID};
 
 #[cfg(not(feature = "no_module"))]
 use crate::module::{resolvers, Module, ModuleResolver};
@@ -22,7 +22,7 @@ use crate::stdlib::{
     boxed::Box,
     collections::HashMap,
     format,
-    iter::{empty, once},
+    iter::{empty, once, repeat},
     mem,
     num::NonZeroUsize,
     ops::{Deref, DerefMut},
@@ -158,27 +158,39 @@ impl<'a> State<'a> {
     }
     /// Does a certain script-defined function exist in the `State`?
     pub fn has_function(&self, name: &str, params: usize) -> bool {
-        self.fn_lib.contains_key(&calc_fn_def(name, params))
+        self.fn_lib.contains_key(&calc_fn_hash(
+            empty(),
+            name,
+            repeat(EMPTY_TYPE_ID()).take(params),
+        ))
     }
     /// Get a script-defined function definition from the `State`.
     pub fn get_function(&self, name: &str, params: usize) -> Option<&FnDef> {
         self.fn_lib
-            .get(&calc_fn_def(name, params))
+            .get(&calc_fn_hash(
+                empty(),
+                name,
+                repeat(EMPTY_TYPE_ID()).take(params),
+            ))
             .map(|f| f.as_ref())
     }
 }
 
+/// A sharable script-defined function.
+#[cfg(feature = "sync")]
+pub type ScriptedFunction = Arc<FnDef>;
+#[cfg(not(feature = "sync"))]
+pub type ScriptedFunction = Rc<FnDef>;
+
 /// A type that holds a library (`HashMap`) of script-defined functions.
 ///
 /// Since script-defined functions have `Dynamic` parameters, functions with the same name
 /// and number of parameters are considered equivalent.
 ///
-/// The key of the `HashMap` is a `u64` hash calculated by the function `calc_fn_def`.
+/// The key of the `HashMap` is a `u64` hash calculated by the function `calc_fn_hash`
+/// with dummy parameter types `EMPTY_TYPE_ID()` repeated the correct number of times.
 #[derive(Debug, Clone, Default)]
-pub struct FunctionsLib(
+pub struct FunctionsLib(HashMap<u64, ScriptedFunction>);
-    #[cfg(feature = "sync")] HashMap<u64, Arc<FnDef>>,
-    #[cfg(not(feature = "sync"))] HashMap<u64, Rc<FnDef>>,
-);
 
 impl FunctionsLib {
     /// Create a new `FunctionsLib` from a collection of `FnDef`.
@@ -186,7 +198,11 @@ impl FunctionsLib {
         FunctionsLib(
             vec.into_iter()
                 .map(|f| {
-                    let hash = calc_fn_def(&f.name, f.params.len());
+                    let hash = calc_fn_hash(
+                        empty(),
+                        &f.name,
+                        repeat(EMPTY_TYPE_ID()).take(f.params.len()),
+                    );
 
                     #[cfg(feature = "sync")]
                     {
@@ -201,12 +217,21 @@ impl FunctionsLib {
         )
     }
     /// Does a certain function exist in the `FunctionsLib`?
-    pub fn has_function(&self, name: &str, params: usize) -> bool {
+    ///
-        self.contains_key(&calc_fn_def(name, params))
+    /// The `u64` hash is calculated by the function `crate::calc_fn_hash`.
+    pub fn has_function(&self, hash: u64) -> bool {
+        self.contains_key(&hash)
     }
     /// Get a function definition from the `FunctionsLib`.
-    pub fn get_function(&self, name: &str, params: usize) -> Option<&FnDef> {
+    ///
-        self.get(&calc_fn_def(name, params)).map(|f| f.as_ref())
+    /// The `u64` hash is calculated by the function `crate::calc_fn_hash`.
+    pub fn get_function(&self, hash: u64) -> Option<&FnDef> {
+        self.get(&hash).map(|fn_def| fn_def.as_ref())
+    }
+    /// Get a function definition from the `FunctionsLib`.
+    pub fn get_function_by_signature(&self, name: &str, params: usize) -> Option<&FnDef> {
+        let hash = calc_fn_hash(empty(), name, repeat(EMPTY_TYPE_ID()).take(params));
+        self.get_function(hash)
     }
     /// Merge another `FunctionsLib` into this `FunctionsLib`.
     pub fn merge(&self, other: &Self) -> Self {
@@ -222,6 +247,12 @@ impl FunctionsLib {
     }
 }
 
+impl From<Vec<(u64, ScriptedFunction)>> for FunctionsLib {
+    fn from(values: Vec<(u64, ScriptedFunction)>) -> Self {
+        FunctionsLib(values.into_iter().collect())
+    }
+}
+
 impl Deref for FunctionsLib {
     #[cfg(feature = "sync")]
     type Target = HashMap<u64, Arc<FnDef>>;
@@ -1323,17 +1354,14 @@ impl Engine {
                 })?;
 
                 // First search in script-defined functions (can override built-in)
-                if let Some(fn_def) = module.get_qualified_fn_lib(fn_name, args.len(), modules)? {
+                if let Some(fn_def) = module.get_qualified_scripted_fn(modules.key()) {
                     self.call_script_fn(None, state, fn_def, &mut args, *pos, level)
                 } else {
                     // Then search in Rust functions
-                    let hash = calc_fn_hash(
+                    let hash1 = modules.key();
-                        modules.iter().map(|(m, _)| m.as_str()),
+                    let hash2 = calc_fn_hash(empty(), "", args.iter().map(|a| a.type_id()));
-                        fn_name,
-                        args.iter().map(|a| a.type_id()),
-                    );
 
-                    match module.get_qualified_fn(fn_name, hash, *pos) {
+                    match module.get_qualified_fn(fn_name, hash1 ^ hash2, *pos) {
                         Ok(func) => func(&mut args, *pos),
                         Err(_) if def_val.is_some() => Ok(def_val.as_deref().unwrap().clone()),
                         Err(err) => Err(err),

src/module.rs

@@ -3,19 +3,19 @@
 
 use crate::any::{Dynamic, Variant};
 use crate::calc_fn_hash;
-use crate::engine::{Engine, FnAny, FnCallArgs, FunctionsLib};
+use crate::engine::{Engine, FnAny, FnCallArgs, FunctionsLib, ScriptedFunction};
 use crate::parser::{FnDef, AST};
 use crate::result::EvalAltResult;
 use crate::scope::{Entry as ScopeEntry, EntryType as ScopeEntryType, Scope};
 use crate::token::{Position, Token};
-use crate::utils::StaticVec;
+use crate::utils::{StaticVec, EMPTY_TYPE_ID};
 
 use crate::stdlib::{
     any::TypeId,
     boxed::Box,
     collections::HashMap,
     fmt,
-    iter::{empty, once},
+    iter::{empty, repeat},
     mem,
     ops::{Deref, DerefMut},
     rc::Rc,
@@ -67,6 +67,9 @@ pub struct Module {
 
     /// Script-defined functions.
     fn_lib: FunctionsLib,
+
+    /// Flattened collection of all script-defined functions, including those in sub-modules.
+    all_fn_lib: FunctionsLib,
 }
 
 impl fmt::Debug for Module {
@@ -239,26 +242,6 @@ impl Module {
         self.modules.insert(name.into(), sub_module.into());
     }
 
-    /// Get a mutable reference to a modules chain.
-    /// The first module is always skipped and assumed to be the same as `self`.
-    pub(crate) fn get_qualified_module_mut(
-        &mut self,
-        modules: &StaticVec<(String, Position)>,
-    ) -> Result<&mut Module, Box<EvalAltResult>> {
-        let mut drain = modules.iter();
-        drain.next().unwrap(); // Skip first module
-
-        let mut module = self;
-
-        for (id, id_pos) in drain {
-            module = module
-                .get_sub_module_mut(id)
-                .ok_or_else(|| Box::new(EvalAltResult::ErrorModuleNotFound(id.into(), *id_pos)))?;
-        }
-
-        Ok(module)
-    }
-
     /// Does the particular Rust function exist in the module?
     ///
     /// The `u64` hash is calculated by the function `crate::calc_fn_hash`.
@@ -576,17 +559,11 @@ impl Module {
         &self.fn_lib
     }
 
-    /// Get a modules-qualified functions library.
+    /// Get a modules-qualified script-defined functions.
-    pub(crate) fn get_qualified_fn_lib(
+    ///
-        &mut self,
+    /// The `u64` hash is calculated by the function `crate::calc_fn_hash`.
-        name: &str,
+    pub(crate) fn get_qualified_scripted_fn(&mut self, hash: u64) -> Option<&FnDef> {
-        args: usize,
+        self.all_fn_lib.get_function(hash)
-        modules: &StaticVec<(String, Position)>,
-    ) -> Result<Option<&FnDef>, Box<EvalAltResult>> {
-        Ok(self
-            .get_qualified_module_mut(modules)?
-            .fn_lib
-            .get_function(name, args))
     }
 
     /// Create a new `Module` by evaluating an `AST`.
@@ -648,11 +625,12 @@ impl Module {
             names: &mut Vec<&'a str>,
             variables: &mut Vec<(u64, Dynamic)>,
             functions: &mut Vec<(u64, NativeFunction)>,
+            fn_lib: &mut Vec<(u64, ScriptedFunction)>,
         ) {
             for (n, m) in module.modules.iter_mut() {
                 // Collect all the sub-modules first.
                 names.push(n);
-                collect(m, names, variables, functions);
+                collect(m, names, variables, functions, fn_lib);
                 names.pop();
             }
 
@@ -661,28 +639,42 @@ impl Module {
                 let hash = calc_fn_hash(names.iter().map(|v| *v), var_name, empty());
                 variables.push((hash, value.clone()));
             }
-            // Collect all functions
+            // Collect all Rust functions
             for (fn_name, params, func) in module.functions.values() {
-                let hash = calc_fn_hash(names.iter().map(|v| *v), fn_name, params.iter().cloned());
+                let hash1 = calc_fn_hash(
-                functions.push((hash, func.clone()));
+                    names.iter().map(|v| *v),
+                    fn_name,
+                    repeat(EMPTY_TYPE_ID()).take(params.len()),
+                );
+                let hash2 = calc_fn_hash(empty(), "", params.iter().cloned());
+                functions.push((hash1 ^ hash2, func.clone()));
+            }
+            // Collect all script-defined functions
+            for fn_def in module.fn_lib.values() {
+                let hash = calc_fn_hash(
+                    names.iter().map(|v| *v),
+                    &fn_def.name,
+                    repeat(EMPTY_TYPE_ID()).take(fn_def.params.len()),
+                );
+                fn_lib.push((hash, fn_def.clone()));
             }
         }
 
         let mut variables = Vec::new();
         let mut functions = Vec::new();
+        let mut fn_lib = Vec::new();
 
-        collect(self, &mut vec!["root"], &mut variables, &mut functions);
+        collect(
+            self,
+            &mut vec!["root"],
+            &mut variables,
+            &mut functions,
+            &mut fn_lib,
+        );
 
-        self.all_variables.clear();
+        self.all_variables = variables.into_iter().collect();
-        self.all_functions.clear();
+        self.all_functions = functions.into_iter().collect();
-
+        self.all_fn_lib = fn_lib.into();
-        for (key, value) in variables {
-            self.all_variables.insert(key, value);
-        }
-
-        for (key, value) in functions {
-            self.all_functions.insert(key, value);
-        }
     }
 }
 
@@ -843,7 +835,7 @@ impl fmt::Debug for ModuleRef {
         fmt::Debug::fmt(&self.0, f)?;
 
         if self.1 > 0 {
-            write!(f, " -> {}", self.1)
+            write!(f, " -> {:0>16x}", self.1)
         } else {
             Ok(())
         }

src/parser.rs

@@ -8,7 +8,7 @@ use crate::module::ModuleRef;
 use crate::optimize::{optimize_into_ast, OptimizationLevel};
 use crate::scope::{EntryType as ScopeEntryType, Scope};
 use crate::token::{Position, Token, TokenIterator};
-use crate::utils::{calc_fn_def, StaticVec};
+use crate::utils::{StaticVec, EMPTY_TYPE_ID};
 
 use crate::stdlib::{
     borrow::Cow,
@@ -16,7 +16,7 @@ use crate::stdlib::{
     char,
     collections::HashMap,
     format,
-    iter::{empty, Peekable},
+    iter::{empty, repeat, Peekable},
     num::NonZeroUsize,
     ops::{Add, Deref, DerefMut},
     rc::Rc,
@@ -677,7 +677,7 @@ fn parse_call_expr<'a>(
     input: &mut Peekable<TokenIterator<'a>>,
     stack: &mut Stack,
     id: String,
-    modules: Option<Box<ModuleRef>>,
+    mut modules: Option<Box<ModuleRef>>,
     begin: Position,
     allow_stmt_expr: bool,
 ) -> Result<Expr, Box<ParseError>> {
@@ -697,6 +697,13 @@ fn parse_call_expr<'a>(
         // id()
         (Token::RightParen, _) => {
             eat_token(input, Token::RightParen);
+
+            if let Some(modules) = modules.as_mut() {
+                // Calculate hash
+                let hash = calc_fn_hash(modules.iter().map(|(m, _)| m.as_str()), &id, empty());
+                modules.set_key(hash);
+            }
+
             return Ok(Expr::FnCall(
                 Box::new(id.into()),
                 modules,
@@ -713,9 +720,20 @@ fn parse_call_expr<'a>(
         args.push(parse_expr(input, stack, allow_stmt_expr)?);
 
         match input.peek().unwrap() {
+            // id(...args)
             (Token::RightParen, _) => {
                 eat_token(input, Token::RightParen);
 
+                if let Some(modules) = modules.as_mut() {
+                    // Calculate hash
+                    let hash = calc_fn_hash(
+                        modules.iter().map(|(m, _)| m.as_str()),
+                        &id,
+                        repeat(EMPTY_TYPE_ID()).take(args.len()),
+                    );
+                    modules.set_key(hash);
+                }
+
                 return Ok(Expr::FnCall(
                     Box::new(id.into()),
                     modules,
@@ -724,9 +742,11 @@ fn parse_call_expr<'a>(
                     begin,
                 ));
             }
+            // id(...args,
             (Token::Comma, _) => {
                 eat_token(input, Token::Comma);
             }
+            // id(...args <EOF>
             (Token::EOF, pos) => {
                 return Err(PERR::MissingToken(
                     Token::RightParen.into(),
@@ -734,9 +754,11 @@ fn parse_call_expr<'a>(
                 )
                 .into_err(*pos))
             }
+            // id(...args <error>
             (Token::LexError(err), pos) => {
                 return Err(PERR::BadInput(err.to_string()).into_err(*pos))
             }
+            // id(...args ???
             (_, pos) => {
                 return Err(PERR::MissingToken(
                     Token::Comma.into(),
@@ -2207,7 +2229,14 @@ fn parse_global_level<'a>(
             if let (Token::Fn, _) = input.peek().unwrap() {
                 let mut stack = Stack::new();
                 let f = parse_fn(input, &mut stack, true)?;
-                functions.insert(calc_fn_def(&f.name, f.params.len()), f);
+                functions.insert(
+                    calc_fn_hash(
+                        empty(),
+                        &f.name,
+                        repeat(EMPTY_TYPE_ID()).take(f.params.len()),
+                    ),
+                    f,
+                );
                 continue;
             }
         }

src/utils.rs

@@ -14,6 +14,10 @@ use crate::stdlib::collections::hash_map::DefaultHasher;
 #[cfg(feature = "no_std")]
 use ahash::AHasher;
 
+pub fn EMPTY_TYPE_ID() -> TypeId {
+    TypeId::of::<()>()
+}
+
 /// Calculate a `u64` hash key from a module-qualified function name and parameter types.
 ///
 /// Module names are passed in via `&str` references from an iterator.
@@ -39,18 +43,6 @@ pub fn calc_fn_spec<'a>(
     s.finish()
 }
 
-/// Calculate a `u64` hash key from a function name and number of parameters (without regard to types).
-pub(crate) fn calc_fn_def(fn_name: &str, num_params: usize) -> u64 {
-    #[cfg(feature = "no_std")]
-    let mut s: AHasher = Default::default();
-    #[cfg(not(feature = "no_std"))]
-    let mut s = DefaultHasher::new();
-
-    s.write(fn_name.as_bytes());
-    s.write_usize(num_params);
-    s.finish()
-}
-
 /// A type to hold a number of values in static storage for speed, and any spill-overs in a `Vec`.
 ///
 /// This is essentially a knock-off of the [`staticvec`](https://crates.io/crates/staticvec) crate.