From fe633ea7d3aeda4826be62d8c2416b0eaf50c4da Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 6 Mar 2021 10:44:55 +0800 Subject: [PATCH 01/11] Fix bug when passing shared string variable to &str parameter. --- CHANGELOG.md | 7 ++++--- src/dynamic.rs | 6 ++++-- src/fn_call.rs | 50 +++++++++++++++++++++++++++------------------- src/fn_register.rs | 19 ++++++++++++------ tests/closures.rs | 45 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b77d3dea..6523076d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,12 @@ Version 0.19.14 Bug fixes --------- +* Panic when passing a shared string into a registered function as `&str` argument is fixed. +* Panic when calling `switch` statements on custom types is fixed. +* Potential overflow panics in `range(from, to, step)` is fixed. +* Some expressions involving shared variables now work properly, for example `x in shared_value`, `return shared_value`, `obj.field = shared_value` etc. Previously, the resultant value is still shared which is counter-intuitive. * Errors in native Rust functions now contain the correct function call positions. * Fixed error types in `EvalAltResult::ErrorMismatchDataType` which were swapped. -* Some expressions involving shared variables now work properly, for example `x in shared_value`, `return shared_value`, `obj.field = shared_value` etc. Previously, the resultant value is still shared which is counter-intuitive. -* Potential overflow panics in `range(from, to, step)` is fixed. -* `switch` statements no longer panic on custom types. Breaking changes ---------------- diff --git a/src/dynamic.rs b/src/dynamic.rs index 020ed7e8..ae7ecd9a 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -284,11 +284,13 @@ impl Dynamic { /// Always [`false`] under the `no_closure` feature. #[inline(always)] pub fn is_shared(&self) -> bool { + #[cfg(not(feature = "no_closure"))] match self.0 { - #[cfg(not(feature = "no_closure"))] Union::Shared(_, _) => true, _ => false, } + #[cfg(feature = "no_closure")] + false } /// Is the value held by this [`Dynamic`] a particular type? /// @@ -1533,7 +1535,7 @@ impl Dynamic { /// /// Panics if the value is shared. #[inline(always)] - pub(crate) fn as_str(&self) -> Result<&str, &'static str> { + pub(crate) fn as_str_ref(&self) -> Result<&str, &'static str> { match &self.0 { Union::Str(s, _) => Ok(s), #[cfg(not(feature = "no_closure"))] diff --git a/src/fn_call.rs b/src/fn_call.rs index 2250b5df..585a8273 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -334,6 +334,12 @@ impl Engine { backup.as_mut().unwrap().change_first_arg_to_copy(args); } + // Flatten arguments passed by value + args.iter_mut() + .skip(if is_ref { 1 } else { 0 }) + .filter(|v| v.is_shared()) + .for_each(|v| **v = mem::take(*v).flatten()); + // Run external function let source = src.as_ref().or_else(|| source.as_ref()).map(|s| s.as_str()); let result = if func.is_plugin_fn() { @@ -1129,16 +1135,18 @@ impl Engine { // Handle is_def_fn() #[cfg(not(feature = "no_function"))] crate::engine::KEYWORD_IS_DEF_FN if args_expr.len() == 2 => { - let fn_name = - self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; - let fn_name = fn_name.take_immutable_string().map_err(|err| { - self.make_type_mismatch_err::(err, args_expr[0].position()) - })?; - let num_params = - self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[1], level)?; - let num_params = num_params.as_int().map_err(|err| { - self.make_type_mismatch_err::(err, args_expr[0].position()) - })?; + let fn_name = self + .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)? + .take_immutable_string() + .map_err(|err| { + self.make_type_mismatch_err::(err, args_expr[0].position()) + })?; + let num_params = self + .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[1], level)? + .as_int() + .map_err(|err| { + self.make_type_mismatch_err::(err, args_expr[0].position()) + })?; return Ok(if num_params < 0 { Dynamic::FALSE @@ -1151,11 +1159,12 @@ impl Engine { // Handle is_def_var() KEYWORD_IS_DEF_VAR if args_expr.len() == 1 => { - let var_name = - self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; - let var_name = var_name.take_immutable_string().map_err(|err| { - self.make_type_mismatch_err::(err, args_expr[0].position()) - })?; + let var_name = self + .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)? + .take_immutable_string() + .map_err(|err| { + self.make_type_mismatch_err::(err, args_expr[0].position()) + })?; return Ok(scope.contains(&var_name).into()); } @@ -1166,11 +1175,12 @@ impl Engine { // eval - only in function call style let prev_len = scope.len(); - let script = - self.eval_expr(scope, mods, state, lib, this_ptr, script_expr, level)?; - let script = script.take_immutable_string().map_err(|typ| { - self.make_type_mismatch_err::(typ, script_pos) - })?; + let script = self + .eval_expr(scope, mods, state, lib, this_ptr, script_expr, level)? + .take_immutable_string() + .map_err(|typ| { + self.make_type_mismatch_err::(typ, script_pos) + })?; let result = self.eval_script_expr_in_place( scope, mods, diff --git a/src/fn_register.rs b/src/fn_register.rs index 48ed4bab..74ca32be 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -100,13 +100,20 @@ pub fn by_ref(data: &mut Dynamic) -> DynamicWriteLock { #[inline(always)] pub fn by_value(data: &mut Dynamic) -> T { if TypeId::of::() == TypeId::of::<&str>() { - // If T is &str, data must be ImmutableString, so map directly to it - let ref_str = data.as_str().unwrap(); - let ref_T = unsafe { mem::transmute::<_, &T>(&ref_str) }; - ref_T.clone() + // If T is `&str`, data must be `ImmutableString`, so map directly to it + + // Beware - `Dynamic::as_str_ref` panics if `data` is shared, + // but this should not happen for argument which is passed by value + assert!(!data.is_shared()); + + let ref_str = data + .as_str_ref() + .expect("argument passed by value should not be shared"); + let ref_t = unsafe { mem::transmute::<_, &T>(&ref_str) }; + ref_t.clone() } else if TypeId::of::() == TypeId::of::() { - // If T is String, data must be ImmutableString, so map directly to it - *unsafe_cast_box(Box::new(data.clone().take_string().unwrap())).unwrap() + // If T is `String`, data must be `ImmutableString`, so map directly to it + *unsafe_cast_box(Box::new(mem::take(data).take_string().unwrap())).unwrap() } else { // We consume the argument and then replace it with () - the argument is not supposed to be used again. // This way, we avoid having to clone the argument again, because it is already a clone when passed here. diff --git a/tests/closures.rs b/tests/closures.rs index d64cccd7..25f81c8a 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -179,6 +179,51 @@ fn test_closures() -> Result<(), Box> { Ok(()) } +#[test] +#[cfg(not(feature = "no_closure"))] +fn test_closures_sharing() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine.register_fn("foo", |x: INT, s: &str| s.len() as INT + x); + engine.register_fn("bar", |x: INT, s: String| s.len() as INT + x); + + assert_eq!( + engine.eval::( + r#" + let s = "hello"; + let f = || s; + foo(1, s) + "# + )?, + 6 + ); + + assert_eq!( + engine.eval::( + r#" + let s = "hello"; + let f = || s; + let n = foo(1, s); + s + "# + )?, + "hello" + ); + + assert_eq!( + engine.eval::( + r#" + let s = "hello"; + let f = || s; + bar(1, s) + "# + )?, + 6 + ); + + Ok(()) +} + #[test] #[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_object"))] From e14bef4b10a304ac8f7072d5737f850f731cd8e5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 6 Mar 2021 14:41:35 +0800 Subject: [PATCH 02/11] Trap &mut String parameters. --- CHANGELOG.md | 1 + src/fn_register.rs | 14 +++++++------- tests/string.rs | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6523076d..85615fb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Bug fixes * Panic when passing a shared string into a registered function as `&str` argument is fixed. * Panic when calling `switch` statements on custom types is fixed. * Potential overflow panics in `range(from, to, step)` is fixed. +* `&mut String` parameters in registered functions no longer panic when passed a string. * Some expressions involving shared variables now work properly, for example `x in shared_value`, `return shared_value`, `obj.field = shared_value` etc. Previously, the resultant value is still shared which is counter-intuitive. * Errors in native Rust functions now contain the correct function call positions. * Fixed error types in `EvalAltResult::ErrorMismatchDataType` which were swapped. diff --git a/src/fn_register.rs b/src/fn_register.rs index 74ca32be..5751f0d3 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -161,15 +161,15 @@ pub fn map_result(data: RhaiResult) -> RhaiResult { /// Remap `&str` | `String` to `ImmutableString`. #[inline(always)] -fn map_type_id() -> TypeId { - let id = TypeId::of::(); +fn map_type_id() -> TypeId { + let ref_id = TypeId::of::(); - if id == TypeId::of::<&str>() { + if ref_id == TypeId::of::<&str>() { TypeId::of::() - } else if id == TypeId::of::() { + } else if ref_id == TypeId::of::() { TypeId::of::() } else { - id + TypeId::of::() } } @@ -193,7 +193,7 @@ macro_rules! def_register { #[inline(always)] fn register_fn(&mut self, name: &str, f: FN) -> &mut Self { self.global_namespace.set_fn(name, FnNamespace::Global, FnAccess::Public, None, - &[$(map_type_id::<$par>()),*], + &[$(map_type_id::<$param, $par>()),*], CallableFunction::$abi(make_func!(f : map_dynamic ; $($par => $let => $clone => $arg),*)) ); self @@ -208,7 +208,7 @@ macro_rules! def_register { #[inline(always)] fn register_result_fn(&mut self, name: &str, f: FN) -> &mut Self { self.global_namespace.set_fn(name, FnNamespace::Global, FnAccess::Public, None, - &[$(map_type_id::<$par>()),*], + &[$(map_type_id::<$param, $par>()),*], CallableFunction::$abi(make_func!(f : map_result ; $($par => $let => $clone => $arg),*)) ); self diff --git a/tests/string.rs b/tests/string.rs index a5328fad..0db10eb0 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -64,6 +64,25 @@ fn test_string_dynamic() -> Result<(), Box> { Ok(()) } +#[test] +fn test_string_mut() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine.register_fn("foo", |x: INT, s: &str| s.len() as INT + x); + engine.register_fn("bar", |x: INT, s: String| s.len() as INT + x); + engine.register_fn("baz", |s: &mut String| s.len()); + + assert_eq!(engine.eval::(r#"foo(1, "hello")"#)?, 6); + assert_eq!(engine.eval::(r#"bar(1, "hello")"#)?, 6); + assert!( + matches!(*engine.eval::(r#"baz("hello")"#).expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(f, _) if f == "baz (&str | ImmutableString | String)" + ) + ); + + Ok(()) +} + #[cfg(not(feature = "no_object"))] #[test] fn test_string_substring() -> Result<(), Box> { From a126d05c3fd52b44afc45551adb51f5139c4c675 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 6 Mar 2021 16:05:22 +0800 Subject: [PATCH 03/11] Skip evaluate condition for loop statement. --- src/ast.rs | 8 +++++--- src/engine.rs | 49 ++++++++++++++++++++++++++----------------------- src/optimize.rs | 21 ++++++++++++++------- src/parser.rs | 5 +++-- 4 files changed, 48 insertions(+), 35 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 13127fe4..06d3b714 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -823,7 +823,7 @@ pub enum Stmt { Position, ), /// `while` expr `{` stmt `}` - While(Expr, Box, Position), + While(Option, Box, Position), /// `do` `{` stmt `}` `while`|`until` expr Do(Box, Expr, bool, Position), /// `for` id `in` expr `{` stmt `}` @@ -981,9 +981,10 @@ impl Stmt { && x.0.values().all(Stmt::is_pure) && x.1.as_ref().map(Stmt::is_pure).unwrap_or(true) } - Self::While(condition, block, _) | Self::Do(block, condition, _, _) => { + Self::While(Some(condition), block, _) | Self::Do(block, condition, _, _) => { condition.is_pure() && block.is_pure() } + Self::While(None, block, _) => block.is_pure(), Self::For(iterable, x, _) => iterable.is_pure() && x.1.is_pure(), Self::Let(_, _, _, _) | Self::Const(_, _, _, _) | Self::Assignment(_, _) => false, Self::Block(block, _) => block.iter().all(|stmt| stmt.is_pure()), @@ -1021,10 +1022,11 @@ impl Stmt { s.walk(path, on_node); } } - Self::While(e, s, _) | Self::Do(s, e, _, _) => { + Self::While(Some(e), s, _) | Self::Do(s, e, _, _) => { e.walk(path, on_node); s.walk(path, on_node); } + Self::While(None, s, _) => s.walk(path, on_node), Self::For(e, x, _) => { e.walk(path, on_node); x.1.walk(path, on_node); diff --git a/src/engine.rs b/src/engine.rs index 8c710c69..113150f9 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -2138,24 +2138,25 @@ impl Engine { // While loop Stmt::While(expr, body, _) => loop { - match self - .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? - .as_bool() - { - Ok(true) => { - match self.eval_stmt(scope, mods, state, lib, this_ptr, body, level) { - Ok(_) => (), - Err(err) => match *err { - EvalAltResult::LoopBreak(false, _) => (), - EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), - _ => return Err(err), - }, - } - } - Ok(false) => return Ok(Dynamic::UNIT), - Err(err) => { - return Err(self.make_type_mismatch_err::(err, expr.position())) + let condition = if let Some(expr) = expr { + self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)? + .as_bool() + .map_err(|err| self.make_type_mismatch_err::(err, expr.position()))? + } else { + true + }; + + if condition { + match self.eval_stmt(scope, mods, state, lib, this_ptr, body, level) { + Ok(_) => (), + Err(err) => match *err { + EvalAltResult::LoopBreak(false, _) => (), + EvalAltResult::LoopBreak(true, _) => return Ok(Dynamic::UNIT), + _ => return Err(err), + }, } + } else { + return Ok(Dynamic::UNIT); } }, @@ -2170,15 +2171,17 @@ impl Engine { }, } - match self + if self .eval_expr(scope, mods, state, lib, this_ptr, expr, level)? .as_bool() + .map_err(|err| self.make_type_mismatch_err::(err, expr.position()))? { - Ok(true) if !*is_while => return Ok(Dynamic::UNIT), - Ok(false) if *is_while => return Ok(Dynamic::UNIT), - Ok(_) => (), - Err(err) => { - return Err(self.make_type_mismatch_err::(err, expr.position())) + if !*is_while { + return Ok(Dynamic::UNIT); + } + } else { + if *is_while { + return Ok(Dynamic::UNIT); } } }, diff --git a/src/optimize.rs b/src/optimize.rs index cff6580b..2f367a05 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -393,25 +393,32 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { } // while false { block } -> Noop - Stmt::While(Expr::BoolConstant(false, pos), _, _) => { + Stmt::While(Some(Expr::BoolConstant(false, pos)), _, _) => { state.set_dirty(); *stmt = Stmt::Noop(*pos) } // while expr { block } Stmt::While(condition, block, _) => { optimize_stmt(block, state, false); - optimize_expr(condition, state); + + if let Some(condition) = condition { + optimize_expr(condition, state); + } match **block { // while expr { break; } -> { expr; } Stmt::Break(pos) => { // Only a single break statement - turn into running the guard expression once state.set_dirty(); - let mut statements = vec![Stmt::Expr(mem::take(condition))]; - if preserve_result { - statements.push(Stmt::Noop(pos)) - } - *stmt = Stmt::Block(statements, pos); + if let Some(condition) = condition { + let mut statements = vec![Stmt::Expr(mem::take(condition))]; + if preserve_result { + statements.push(Stmt::Noop(pos)) + } + *stmt = Stmt::Block(statements, pos); + } else { + *stmt = Stmt::Noop(pos); + }; } _ => (), } diff --git a/src/parser.rs b/src/parser.rs index 96e1da4e..cb549a41 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2122,9 +2122,10 @@ fn parse_while_loop( let (guard, token_pos) = match input.next().unwrap() { (Token::While, pos) => { ensure_not_statement_expr(input, "a boolean")?; - (parse_expr(input, state, lib, settings.level_up())?, pos) + let expr = parse_expr(input, state, lib, settings.level_up())?; + (Some(expr), pos) } - (Token::Loop, pos) => (Expr::BoolConstant(true, pos), pos), + (Token::Loop, pos) => (None, pos), (t, _) => unreachable!("expecting Token::While or Token::Loop, but gets {:?}", t), }; settings.pos = token_pos; From 4da5af8aaea48c54e29d4eacd376bd4b1720b9ee Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 6 Mar 2021 21:25:49 +0800 Subject: [PATCH 04/11] Do not flatten arguments passed by value. --- src/dynamic.rs | 134 +++++++++++++++++++++++++++------------------ src/fn_call.rs | 8 +-- src/fn_register.rs | 6 +- tests/modules.rs | 18 +++--- 4 files changed, 93 insertions(+), 73 deletions(-) diff --git a/src/dynamic.rs b/src/dynamic.rs index ae7ecd9a..5478efa2 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -286,10 +286,10 @@ impl Dynamic { pub fn is_shared(&self) -> bool { #[cfg(not(feature = "no_closure"))] match self.0 { - Union::Shared(_, _) => true, - _ => false, + Union::Shared(_, _) => return true, + _ => (), } - #[cfg(feature = "no_closure")] + false } /// Is the value held by this [`Dynamic`] a particular type? @@ -749,27 +749,26 @@ impl Dynamic { /// if its value is going to be modified. This safe-guards constant values from being modified /// from within Rust functions. pub fn is_read_only(&self) -> bool { + #[cfg(not(feature = "no_closure"))] match self.0 { - #[cfg(not(feature = "no_closure"))] - Union::Shared(_, AccessMode::ReadOnly) => true, - - #[cfg(not(feature = "no_closure"))] + Union::Shared(_, AccessMode::ReadOnly) => return true, Union::Shared(ref cell, _) => { #[cfg(not(feature = "sync"))] let value = cell.borrow(); #[cfg(feature = "sync")] let value = cell.read().unwrap(); - match value.access_mode() { + return match value.access_mode() { AccessMode::ReadWrite => false, AccessMode::ReadOnly => true, - } + }; } + _ => (), + } - _ => match self.access_mode() { - AccessMode::ReadWrite => false, - AccessMode::ReadOnly => true, - }, + match self.access_mode() { + AccessMode::ReadWrite => false, + AccessMode::ReadOnly => true, } } /// Can this [`Dynamic`] be hashed? @@ -949,13 +948,10 @@ impl Dynamic { pub fn into_shared(self) -> Self { let _access = self.access_mode(); - return match self.0 { + match self.0 { Union::Shared(_, _) => self, _ => Self(Union::Shared(crate::Locked::new(self).into(), _access)), - }; - - #[cfg(feature = "no_closure")] - panic!("converting into a shared value is not supported under 'no_closure'"); + } } /// Convert the [`Dynamic`] value into specific type. /// @@ -1139,18 +1135,20 @@ impl Dynamic { /// If the [`Dynamic`] is a shared value, it a cloned copy of the shared value. #[inline(always)] pub fn flatten_clone(&self) -> Self { + #[cfg(not(feature = "no_closure"))] match &self.0 { - #[cfg(not(feature = "no_closure"))] Union::Shared(cell, _) => { #[cfg(not(feature = "sync"))] let value = cell.borrow(); #[cfg(feature = "sync")] let value = cell.read().unwrap(); - value.clone() + return value.clone(); } - _ => self.clone(), + _ => (), } + + self.clone() } /// Flatten the [`Dynamic`]. /// @@ -1160,25 +1158,57 @@ impl Dynamic { /// outstanding references, or a cloned copy. #[inline(always)] pub fn flatten(self) -> Self { + #[cfg(not(feature = "no_closure"))] match self.0 { - #[cfg(not(feature = "no_closure"))] - Union::Shared(cell, _) => crate::fn_native::shared_try_take(cell).map_or_else( - |cell| { - #[cfg(not(feature = "sync"))] - let value = cell.borrow(); - #[cfg(feature = "sync")] - let value = cell.read().unwrap(); + Union::Shared(cell, _) => { + return crate::fn_native::shared_try_take(cell).map_or_else( + |cell| { + #[cfg(not(feature = "sync"))] + let value = cell.borrow(); + #[cfg(feature = "sync")] + let value = cell.read().unwrap(); + + value.clone() + }, + |value| { + #[cfg(not(feature = "sync"))] + return value.into_inner(); + #[cfg(feature = "sync")] + return value.into_inner().unwrap(); + }, + ) + } + _ => (), + } + + self + } + /// Flatten the [`Dynamic`] in place. + /// + /// If the [`Dynamic`] is not a shared value, it does nothing. + /// + /// If the [`Dynamic`] is a shared value, it is set to the shared value if there are no + /// outstanding references, or a cloned copy otherwise. + #[inline(always)] + pub(crate) fn flatten_in_place(&mut self) { + #[cfg(not(feature = "no_closure"))] + match self.0 { + Union::Shared(_, _) => match crate::stdlib::mem::take(self).0 { + Union::Shared(cell, _) => { + let value = crate::fn_native::shared_take_or_clone(cell); - value.clone() - }, - |value| { #[cfg(not(feature = "sync"))] - return value.into_inner(); + { + *self = value.into_inner(); + } #[cfg(feature = "sync")] - return value.into_inner().unwrap(); - }, - ), - _ => self, + { + *self = value.into_inner().unwrap(); + } + } + _ => unreachable!(), + }, + _ => (), } } /// Is the [`Dynamic`] a shared value that is locked? @@ -1190,8 +1220,8 @@ impl Dynamic { /// So this method always returns [`false`] under [`Sync`]. #[inline(always)] pub fn is_locked(&self) -> bool { + #[cfg(not(feature = "no_closure"))] match self.0 { - #[cfg(not(feature = "no_closure"))] Union::Shared(ref _cell, _) => { #[cfg(not(feature = "sync"))] return _cell.try_borrow().is_err(); @@ -1199,8 +1229,10 @@ impl Dynamic { #[cfg(feature = "sync")] return false; } - _ => false, + _ => (), } + + false } /// Get a reference of a specific type to the [`Dynamic`]. /// Casting to [`Dynamic`] just returns a reference to it. @@ -1224,15 +1256,16 @@ impl Dynamic { let type_id = (*value).type_id(); if type_id != TypeId::of::() && TypeId::of::() != TypeId::of::() { - None + return None; } else { - Some(DynamicReadLock(DynamicReadLockInner::Guard(value))) + return Some(DynamicReadLock(DynamicReadLockInner::Guard(value))); } } - _ => self - .downcast_ref() - .map(|r| DynamicReadLock(DynamicReadLockInner::Reference(r))), + _ => (), } + + self.downcast_ref() + .map(|r| DynamicReadLock(DynamicReadLockInner::Reference(r))) } /// Get a mutable reference of a specific type to the [`Dynamic`]. /// Casting to [`Dynamic`] just returns a mutable reference to it. @@ -1256,15 +1289,16 @@ impl Dynamic { let type_id = (*value).type_id(); if type_id != TypeId::of::() && TypeId::of::() != TypeId::of::() { - None + return None; } else { - Some(DynamicWriteLock(DynamicWriteLockInner::Guard(value))) + return Some(DynamicWriteLock(DynamicWriteLockInner::Guard(value))); } } - _ => self - .downcast_mut() - .map(|r| DynamicWriteLock(DynamicWriteLockInner::Reference(r))), + _ => (), } + + self.downcast_mut() + .map(|r| DynamicWriteLock(DynamicWriteLockInner::Reference(r))) } /// Get a reference of a specific type to the [`Dynamic`]. /// Casting to [`Dynamic`] just returns a reference to it. @@ -1357,10 +1391,8 @@ impl Dynamic { match &self.0 { Union::Variant(value, _) => value.as_ref().as_ref().as_any().downcast_ref::(), - #[cfg(not(feature = "no_closure"))] Union::Shared(_, _) => None, - _ => None, } } @@ -1449,10 +1481,8 @@ impl Dynamic { match &mut self.0 { Union::Variant(value, _) => value.as_mut().as_mut_any().downcast_mut::(), - #[cfg(not(feature = "no_closure"))] Union::Shared(_, _) => None, - _ => None, } } diff --git a/src/fn_call.rs b/src/fn_call.rs index 585a8273..f539ae3e 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -334,12 +334,6 @@ impl Engine { backup.as_mut().unwrap().change_first_arg_to_copy(args); } - // Flatten arguments passed by value - args.iter_mut() - .skip(if is_ref { 1 } else { 0 }) - .filter(|v| v.is_shared()) - .for_each(|v| **v = mem::take(*v).flatten()); - // Run external function let source = src.as_ref().or_else(|| source.as_ref()).map(|s| s.as_str()); let result = if func.is_plugin_fn() { @@ -683,7 +677,7 @@ impl Engine { crate::engine::KEYWORD_IS_DEF_FN if args.len() == 2 && args[0].is::() && args[1].is::() => { - let fn_name = mem::take(args[0]).take_immutable_string().unwrap(); + let fn_name = args[0].read_lock::().unwrap(); let num_params = args[1].as_int().unwrap(); return Ok(( diff --git a/src/fn_register.rs b/src/fn_register.rs index 5751f0d3..2d8989e0 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -101,11 +101,7 @@ pub fn by_ref(data: &mut Dynamic) -> DynamicWriteLock { pub fn by_value(data: &mut Dynamic) -> T { if TypeId::of::() == TypeId::of::<&str>() { // If T is `&str`, data must be `ImmutableString`, so map directly to it - - // Beware - `Dynamic::as_str_ref` panics if `data` is shared, - // but this should not happen for argument which is passed by value - assert!(!data.is_shared()); - + data.flatten_in_place(); let ref_str = data .as_str_ref() .expect("argument passed by value should not be shared"); diff --git a/tests/modules.rs b/tests/modules.rs index 44f277e6..934b430d 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -395,14 +395,14 @@ fn test_module_export() -> Result<(), Box> { #[test] fn test_module_str() -> Result<(), Box> { - fn test_fn(_input: ImmutableString) -> Result> { - Ok(42) + fn test_fn(input: ImmutableString) -> Result> { + Ok(input.len() as INT) } - fn test_fn2(_input: &str) -> Result> { - Ok(42) + fn test_fn2(input: &str) -> Result> { + Ok(input.len() as INT) } - fn test_fn3(_input: String) -> Result> { - Ok(42) + fn test_fn3(input: String) -> Result> { + Ok(input.len() as INT) } let mut engine = rhai::Engine::new(); @@ -417,15 +417,15 @@ fn test_module_str() -> Result<(), Box> { assert_eq!( engine.eval::(r#"import "test" as test; test::test("test");"#)?, - 42 + 4 ); assert_eq!( engine.eval::(r#"import "test" as test; test::test2("test");"#)?, - 42 + 4 ); assert_eq!( engine.eval::(r#"import "test" as test; test::test3("test");"#)?, - 42 + 4 ); Ok(()) From e87f981674b2204547443bb669f62af6a669d702 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 6 Mar 2021 22:07:20 +0800 Subject: [PATCH 05/11] Fix sync build. --- src/dynamic.rs | 26 ++++++++++++++++---------- src/engine.rs | 9 ++++++--- src/serde/metadata.rs | 2 +- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/dynamic.rs b/src/dynamic.rs index 5478efa2..abafc404 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -1195,18 +1195,24 @@ impl Dynamic { match self.0 { Union::Shared(_, _) => match crate::stdlib::mem::take(self).0 { Union::Shared(cell, _) => { - let value = crate::fn_native::shared_take_or_clone(cell); + *self = crate::fn_native::shared_try_take(cell).map_or_else( + |cell| { + #[cfg(not(feature = "sync"))] + let value = cell.borrow(); + #[cfg(feature = "sync")] + let value = cell.read().unwrap(); - #[cfg(not(feature = "sync"))] - { - *self = value.into_inner(); - } - #[cfg(feature = "sync")] - { - *self = value.into_inner().unwrap(); - } + value.clone() + }, + |value| { + #[cfg(not(feature = "sync"))] + return value.into_inner(); + #[cfg(feature = "sync")] + return value.into_inner().unwrap(); + }, + ) } - _ => unreachable!(), + _ => unreachable!("self should be Shared"), }, _ => (), } diff --git a/src/engine.rs b/src/engine.rs index 113150f9..b4ac13c2 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -405,8 +405,8 @@ impl<'a> Target<'a> { Self::LockGuard((r, _)) => **r = new_val, Self::Value(_) => panic!("cannot update a value"), #[cfg(not(feature = "no_index"))] - Self::StringChar(string, index, _) if string.is::() => { - let mut s = string.write_lock::().unwrap(); + Self::StringChar(s, index, _) if s.is::() => { + let mut s = s.write_lock::().unwrap(); // Replace the character at the specified index position let new_ch = new_val.as_char().map_err(|err| { @@ -426,7 +426,10 @@ impl<'a> Target<'a> { } } #[cfg(not(feature = "no_index"))] - Self::StringChar(_, _, _) => unreachable!(), + Self::StringChar(s, _, _) => unreachable!( + "Target::StringChar should contain only a string, not {}", + s.type_name() + ), } Ok(()) diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index 94fc32c6..6531ef39 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -147,7 +147,7 @@ impl From<&crate::module::FuncInfo> for FnMetadata { doc_comments: if info.func.is_script() { #[cfg(feature = "no_function")] { - unreachable!() + unreachable!("scripted functions should not exist under no_function") } #[cfg(not(feature = "no_function"))] { From 330d3f87af4cc414ac91adf0f5f8f678e991ea7d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 7 Mar 2021 22:10:54 +0800 Subject: [PATCH 06/11] Use namespace for ScriptFnDef. --- src/ast.rs | 3 +++ src/dynamic.rs | 12 ++++++++- src/engine.rs | 55 +++++++++++++++++----------------------- src/fn_call.rs | 10 ++++---- src/fn_native.rs | 25 ++++++++---------- src/fn_register.rs | 9 +++---- src/lib.rs | 5 +++- src/module/mod.rs | 15 ++++++----- src/optimize.rs | 6 ++--- src/packages/fn_basic.rs | 49 ++++++++++++++++++----------------- src/parser.rs | 17 +++++-------- src/token.rs | 8 +++--- 12 files changed, 106 insertions(+), 108 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 06d3b714..46789cbd 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -89,6 +89,7 @@ impl fmt::Display for ScriptFnDef { /// A type containing the metadata of a script-defined function. /// /// Created by [`AST::iter_functions`]. +#[cfg(not(feature = "no_function"))] #[derive(Debug, Eq, PartialEq, Clone, Hash)] pub struct ScriptFnMetadata<'a> { /// Function doc-comments (if any). @@ -108,6 +109,7 @@ pub struct ScriptFnMetadata<'a> { pub params: Vec<&'a str>, } +#[cfg(not(feature = "no_function"))] impl fmt::Display for ScriptFnMetadata<'_> { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -124,6 +126,7 @@ impl fmt::Display for ScriptFnMetadata<'_> { } } +#[cfg(not(feature = "no_function"))] impl<'a> Into> for &'a ScriptFnDef { #[inline(always)] fn into(self) -> ScriptFnMetadata<'a> { diff --git a/src/dynamic.rs b/src/dynamic.rs index abafc404..b6bb5e71 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -1212,7 +1212,7 @@ impl Dynamic { }, ) } - _ => unreachable!("self should be Shared"), + _ => unreachable!(), }, _ => (), } @@ -1684,6 +1684,16 @@ impl From<&[T]> for Dynamic { )) } } +#[cfg(not(feature = "no_index"))] +impl crate::stdlib::iter::FromIterator for Dynamic { + #[inline(always)] + fn from_iter>(iter: X) -> Self { + Self(Union::Array( + Box::new(iter.into_iter().map(Dynamic::from).collect()), + AccessMode::ReadWrite, + )) + } +} #[cfg(not(feature = "no_object"))] impl, T: Variant + Clone> From> for Dynamic diff --git a/src/engine.rs b/src/engine.rs index b4ac13c2..d8ffd7bb 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -108,6 +108,7 @@ impl Imports { self.0.iter().rev().map(|(n, m)| (n, m)) } /// Get an iterator to this stack of imported [modules][Module] in forward order. + #[allow(dead_code)] #[inline(always)] pub(crate) fn scan_raw(&self) -> impl Iterator)> { self.0.iter().map(|(n, m)| (n, m)) @@ -405,8 +406,8 @@ impl<'a> Target<'a> { Self::LockGuard((r, _)) => **r = new_val, Self::Value(_) => panic!("cannot update a value"), #[cfg(not(feature = "no_index"))] - Self::StringChar(s, index, _) if s.is::() => { - let mut s = s.write_lock::().unwrap(); + Self::StringChar(s, index, _) => { + let s = &mut *s.write_lock::().unwrap(); // Replace the character at the specified index position let new_ch = new_val.as_char().map_err(|err| { @@ -417,19 +418,14 @@ impl<'a> Target<'a> { )) })?; - let mut chars = s.chars().collect::>(); + let index = *index; - // See if changed - if so, update the String - if chars[*index] != new_ch { - chars[*index] = new_ch; - *s = chars.iter().collect::().into(); - } + *s = s + .chars() + .enumerate() + .map(|(i, ch)| if i == index { new_ch } else { ch }) + .collect(); } - #[cfg(not(feature = "no_index"))] - Self::StringChar(s, _, _) => unreachable!( - "Target::StringChar should contain only a string, not {}", - s.type_name() - ), } Ok(()) @@ -543,6 +539,7 @@ impl State { self.fn_resolution_caches.last_mut().unwrap() } /// Push an empty functions resolution cache onto the stack and make it current. + #[allow(dead_code)] pub fn push_fn_resolution_cache(&mut self) { self.fn_resolution_caches.push(Default::default()); } @@ -550,14 +547,6 @@ impl State { pub fn pop_fn_resolution_cache(&mut self) { self.fn_resolution_caches.pop(); } - /// Clear the current functions resolution cache. - /// - /// # Panics - /// - /// Panics if there is no current functions resolution cache. - pub fn clear_fn_resolution_cache(&mut self) { - self.fn_resolution_caches.last_mut().unwrap().clear(); - } } /// _(INTERNALS)_ A type containing all the limits imposed by the [`Engine`]. @@ -1066,9 +1055,7 @@ impl Engine { level: usize, new_val: Option<((Dynamic, Position), (&str, Position))>, ) -> Result<(Dynamic, bool), Box> { - if chain_type == ChainType::NonChaining { - unreachable!("should not be ChainType::NonChaining"); - } + assert!(chain_type != ChainType::NonChaining); let is_ref = target.is_ref(); @@ -1875,7 +1862,7 @@ impl Engine { restore_prev_state: bool, level: usize, ) -> RhaiResult { - let mut _restore_fn_resolution_cache = false; + let mut _extra_fn_resolution_cache = false; let prev_always_search = state.always_search; let prev_scope_len = scope.len(); let prev_mods_len = mods.len(); @@ -1898,15 +1885,18 @@ impl Engine { .skip(_mods_len) .any(|(_, m)| m.contains_indexed_global_functions()) { - if _restore_fn_resolution_cache { + if _extra_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(); + state.fn_resolution_cache_mut().clear(); } else if restore_prev_state { // When new module is imported with global functions, push a new cache state.push_fn_resolution_cache(); - _restore_fn_resolution_cache = true; + _extra_fn_resolution_cache = true; + } else { + // When the block is to be evaluated in-place, just clear the current cache + state.fn_resolution_cache_mut().clear(); } } } @@ -1914,12 +1904,13 @@ impl Engine { Ok(r) }); + if _extra_fn_resolution_cache { + // If imports list is modified, pop the functions lookup cache + state.pop_fn_resolution_cache(); + } + if restore_prev_state { scope.rewind(prev_scope_len); - if _restore_fn_resolution_cache { - // If imports list is modified, pop the functions lookup cache - state.pop_fn_resolution_cache(); - } mods.truncate(prev_mods_len); state.scope_level -= 1; diff --git a/src/fn_call.rs b/src/fn_call.rs index f539ae3e..35c7a768 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -28,7 +28,7 @@ use crate::{ }; use crate::{ calc_native_fn_hash, calc_script_fn_hash, Dynamic, Engine, EvalAltResult, FnPtr, - ImmutableString, Module, ParseErrorType, Position, Scope, StaticVec, INT, + ImmutableString, Module, ParseErrorType, Position, Scope, StaticVec, }; #[cfg(not(feature = "no_object"))] @@ -649,7 +649,7 @@ impl Engine { state: &mut State, lib: &[&Module], fn_name: &str, - hash_script: Option, + _hash_script: Option, args: &mut FnCallArgs, is_ref: bool, _is_method: bool, @@ -675,7 +675,7 @@ impl Engine { // Handle is_def_fn() #[cfg(not(feature = "no_function"))] crate::engine::KEYWORD_IS_DEF_FN - if args.len() == 2 && args[0].is::() && args[1].is::() => + if args.len() == 2 && args[0].is::() && args[1].is::() => { let fn_name = args[0].read_lock::().unwrap(); let num_params = args[1].as_int().unwrap(); @@ -732,7 +732,7 @@ impl Engine { } #[cfg(not(feature = "no_function"))] - if let Some((func, source)) = hash_script.and_then(|hash| { + if let Some((func, source)) = _hash_script.and_then(|hash| { self.resolve_function(mods, state, lib, fn_name, hash, args, false, false) .as_ref() .map(|(f, s)| (f.clone(), s.clone())) @@ -1139,7 +1139,7 @@ impl Engine { .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[1], level)? .as_int() .map_err(|err| { - self.make_type_mismatch_err::(err, args_expr[0].position()) + self.make_type_mismatch_err::(err, args_expr[0].position()) })?; return Ok(if num_params < 0 { diff --git a/src/fn_native.rs b/src/fn_native.rs index 9296da62..d7c299dd 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -1,6 +1,6 @@ //! Module defining interfaces to native-Rust functions. -use crate::ast::{FnAccess, ScriptFnDef}; +use crate::ast::FnAccess; use crate::engine::Imports; use crate::plugin::PluginFunction; use crate::stdlib::{ @@ -143,6 +143,7 @@ impl<'a> NativeCallContext<'a> { } /// Get an iterator over the current set of modules imported via `import` statements. #[cfg(not(feature = "no_module"))] + #[allow(dead_code)] #[inline(always)] pub(crate) fn iter_imports_raw( &self, @@ -438,7 +439,7 @@ pub enum CallableFunction { Plugin(Shared), /// A script-defined function. #[cfg(not(feature = "no_function"))] - Script(Shared), + Script(Shared), } impl fmt::Debug for CallableFunction { @@ -576,7 +577,7 @@ impl CallableFunction { /// Panics if the [`CallableFunction`] is not [`Script`][CallableFunction::Script]. #[cfg(not(feature = "no_function"))] #[inline(always)] - pub fn get_fn_def(&self) -> &ScriptFnDef { + pub fn get_fn_def(&self) -> &crate::ast::ScriptFnDef { match self { Self::Pure(_) | Self::Method(_) | Self::Iterator(_) | Self::Plugin(_) => { panic!("function should be scripted") @@ -642,24 +643,18 @@ impl From for CallableFunction { } } -impl From for CallableFunction { +#[cfg(not(feature = "no_function"))] +impl From for CallableFunction { #[inline(always)] - fn from(_func: ScriptFnDef) -> Self { - #[cfg(feature = "no_function")] - unreachable!("no_function active"); - - #[cfg(not(feature = "no_function"))] + fn from(_func: crate::ast::ScriptFnDef) -> Self { Self::Script(_func.into()) } } -impl From> for CallableFunction { +#[cfg(not(feature = "no_function"))] +impl From> for CallableFunction { #[inline(always)] - fn from(_func: Shared) -> Self { - #[cfg(feature = "no_function")] - unreachable!("no_function active"); - - #[cfg(not(feature = "no_function"))] + fn from(_func: Shared) -> Self { Self::Script(_func) } } diff --git a/src/fn_register.rs b/src/fn_register.rs index 2d8989e0..ea7dc9b0 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -131,8 +131,7 @@ macro_rules! make_func { // The arguments are assumed to be of the correct number and types! let mut _drain = args.iter_mut(); - $($let)* - $($par = ($convert)(_drain.next().unwrap()); )* + $($let $par = ($convert)(_drain.next().unwrap()); )* // Call the function with each argument value let r = $fn($($arg),*); @@ -215,9 +214,9 @@ macro_rules! def_register { }; ($p0:ident $(, $p:ident)*) => { def_register!(imp from_pure : $p0 => $p0 => $p0 => $p0 => let $p0 => by_value $(, $p => $p => $p => $p => let $p => by_value)*); - def_register!(imp from_method : $p0 => &mut $p0 => Mut<$p0> => &mut $p0 => let mut $p0 => by_ref $(, $p => $p => $p => $p => let $p => by_value)*); - // ^ CallableFunction - // handle the first parameter ^ first parameter passed through + def_register!(imp from_method : $p0 => &mut $p0 => Mut<$p0> => &mut $p0 => let mut $p0 => by_ref $(, $p => $p => $p => $p => let $p => by_value)*); + // ^ CallableFunction constructor + // ^ first parameter passed through // ^ others passed by value (by_value) // Currently does not support first argument which is a reference, as there will be diff --git a/src/lib.rs b/src/lib.rs index 26b7b421..01d02910 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,7 +122,7 @@ pub type FLOAT = f64; #[cfg(feature = "f32_float")] pub type FLOAT = f32; -pub use ast::{FnAccess, ScriptFnMetadata, AST}; +pub use ast::{FnAccess, AST}; pub use dynamic::Dynamic; pub use engine::{Engine, EvalContext}; pub use fn_native::{FnPtr, NativeCallContext}; @@ -155,6 +155,9 @@ pub use fn_func::Func; #[cfg(not(feature = "no_function"))] pub use fn_args::FuncArgs; +#[cfg(not(feature = "no_function"))] +pub use ast::ScriptFnMetadata; + /// Variable-sized array of [`Dynamic`] values. /// /// Not available under `no_index`. diff --git a/src/module/mod.rs b/src/module/mod.rs index 6bcdf3d8..ed9f2dab 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -22,9 +22,6 @@ use crate::{ Dynamic, EvalAltResult, ImmutableString, NativeCallContext, Position, Shared, StaticVec, }; -#[cfg(not(feature = "no_function"))] -use crate::ast::ScriptFnDef; - #[cfg(not(feature = "no_index"))] use crate::Array; @@ -460,7 +457,10 @@ impl Module { /// If there is an existing function of the same name and number of arguments, it is replaced. #[cfg(not(feature = "no_function"))] #[inline] - pub(crate) fn set_script_fn(&mut self, fn_def: impl Into>) -> NonZeroU64 { + pub(crate) fn set_script_fn( + &mut self, + fn_def: impl Into>, + ) -> NonZeroU64 { let fn_def = fn_def.into(); // None + function name + number of arguments. @@ -493,7 +493,7 @@ impl Module { name: &str, num_params: usize, public_only: bool, - ) -> Option<&ScriptFnDef> { + ) -> Option<&crate::ast::ScriptFnDef> { self.functions .values() .find( @@ -1692,7 +1692,8 @@ impl Module { #[inline(always)] pub(crate) fn iter_script_fn( &self, - ) -> impl Iterator + '_ { + ) -> impl Iterator + '_ + { self.functions.values().filter(|f| f.func.is_script()).map( |FuncInfo { namespace, @@ -1751,7 +1752,7 @@ impl Module { #[inline(always)] pub fn iter_script_fn_info( &self, - ) -> impl Iterator { + ) -> impl Iterator { self.iter_script_fn() } diff --git a/src/optimize.rs b/src/optimize.rs index 2f367a05..278f2588 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -1,6 +1,6 @@ //! Module implementing the [`AST`] optimizer. -use crate::ast::{Expr, ScriptFnDef, Stmt}; +use crate::ast::{Expr, Stmt}; use crate::dynamic::AccessMode; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::fn_builtin::get_builtin_binary_op_fn; @@ -872,7 +872,7 @@ pub fn optimize_into_ast( engine: &Engine, scope: &Scope, mut statements: Vec, - _functions: Vec, + _functions: Vec, level: OptimizationLevel, ) -> AST { let level = if cfg!(feature = "no_optimize") { @@ -891,7 +891,7 @@ pub fn optimize_into_ast( _functions .iter() - .map(|fn_def| ScriptFnDef { + .map(|fn_def| crate::ast::ScriptFnDef { name: fn_def.name.clone(), access: fn_def.access, body: Default::default(), diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs index e9bd49ff..3a6e489d 100644 --- a/src/packages/fn_basic.rs +++ b/src/packages/fn_basic.rs @@ -1,11 +1,6 @@ use crate::plugin::*; use crate::{def_package, FnPtr, ImmutableString, NativeCallContext}; -#[cfg(not(feature = "no_function"))] -#[cfg(not(feature = "no_index"))] -#[cfg(not(feature = "no_object"))] -use crate::{ast::ScriptFnDef, stdlib::collections::HashMap, Array, Map}; - def_package!(crate:BasicFnPackage:"Basic Fn functions.", lib, { combine_with_exported_module!(lib, "FnPtr", fn_ptr_functions); }); @@ -29,7 +24,7 @@ mod fn_ptr_functions { #[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_object"))] pub mod functions_and_maps { - pub fn get_fn_metadata_list(ctx: NativeCallContext) -> Array { + pub fn get_fn_metadata_list(ctx: NativeCallContext) -> crate::Array { collect_fn_metadata(ctx) } } @@ -38,7 +33,9 @@ mod fn_ptr_functions { #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_object"))] -fn collect_fn_metadata(ctx: NativeCallContext) -> Array { +fn collect_fn_metadata(ctx: NativeCallContext) -> crate::Array { + use crate::{ast::ScriptFnDef, stdlib::collections::HashMap, Array, Map}; + // Create a metadata record for a function. fn make_metadata( dict: &HashMap<&str, ImmutableString>, @@ -76,22 +73,6 @@ fn collect_fn_metadata(ctx: NativeCallContext) -> Array { map.into() } - // Recursively scan modules for script-defined functions. - fn scan_module( - list: &mut Array, - dict: &HashMap<&str, ImmutableString>, - namespace: ImmutableString, - module: &Module, - ) { - module.iter_script_fn().for_each(|(_, _, _, _, f)| { - list.push(make_metadata(dict, Some(namespace.clone()), f).into()) - }); - module.iter_sub_modules().for_each(|(ns, m)| { - let ns: ImmutableString = format!("{}::{}", namespace, ns).into(); - scan_module(list, dict, ns, m.as_ref()) - }); - } - // Intern strings let mut dict = HashMap::<&str, ImmutableString>::with_capacity(8); [ @@ -115,8 +96,26 @@ fn collect_fn_metadata(ctx: NativeCallContext) -> Array { .for_each(|(_, _, _, _, f)| list.push(make_metadata(&dict, None, f).into())); #[cfg(not(feature = "no_module"))] - ctx.iter_imports_raw() - .for_each(|(ns, m)| scan_module(&mut list, &dict, ns.clone(), m.as_ref())); + { + // Recursively scan modules for script-defined functions. + fn scan_module( + list: &mut Array, + dict: &HashMap<&str, ImmutableString>, + namespace: ImmutableString, + module: &Module, + ) { + module.iter_script_fn().for_each(|(_, _, _, _, f)| { + list.push(make_metadata(dict, Some(namespace.clone()), f).into()) + }); + module.iter_sub_modules().for_each(|(ns, m)| { + let ns: ImmutableString = format!("{}::{}", namespace, ns).into(); + scan_module(list, dict, ns, m.as_ref()) + }); + } + + ctx.iter_imports_raw() + .for_each(|(ns, m)| scan_module(&mut list, &dict, ns.clone(), m.as_ref())); + } list } diff --git a/src/parser.rs b/src/parser.rs index cb549a41..dd73136d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -948,7 +948,7 @@ fn parse_primary( } Token::True => Expr::BoolConstant(true, settings.pos), Token::False => Expr::BoolConstant(false, settings.pos), - t => unreachable!("unexpected token: {:?}", t), + _ => unreachable!(), }, #[cfg(not(feature = "no_float"))] Token::FloatConstant(x) => { @@ -1033,7 +1033,7 @@ fn parse_primary( Token::Identifier(_) => { let s = match input.next().unwrap().0 { Token::Identifier(s) => s, - t => unreachable!("expecting Token::Identifier, but gets {:?}", t), + _ => unreachable!(), }; match input.peek().unwrap().0 { @@ -1080,7 +1080,7 @@ fn parse_primary( Token::Reserved(_) => { let s = match input.next().unwrap().0 { Token::Reserved(s) => s, - t => unreachable!("expecting Token::Reserved, but gets {:?}", t), + _ => unreachable!(), }; match input.peek().unwrap().0 { @@ -1115,7 +1115,7 @@ fn parse_primary( Token::LexError(_) => { let err = match input.next().unwrap().0 { Token::LexError(err) => err, - t => unreachable!("expecting Token::LexError, but gets {:?}", t), + _ => unreachable!(), }; return Err(err.into_err(settings.pos)); @@ -2126,7 +2126,7 @@ fn parse_while_loop( (Some(expr), pos) } (Token::Loop, pos) => (None, pos), - (t, _) => unreachable!("expecting Token::While or Token::Loop, but gets {:?}", t), + _ => unreachable!(), }; settings.pos = token_pos; @@ -2544,7 +2544,7 @@ fn parse_stmt( _ => return Err(PERR::WrongDocComment.into_err(comments_pos)), } } - t => unreachable!("expecting Token::Comment, but gets {:?}", t), + _ => unreachable!(), } } } @@ -2651,10 +2651,7 @@ fn parse_stmt( match token { Token::Return => ReturnType::Return, Token::Throw => ReturnType::Exception, - t => unreachable!( - "expecting Token::Return or Token::Throw, but gets {:?}", - t - ), + _ => unreachable!(), }, pos, ) diff --git a/src/token.rs b/src/token.rs index 5cd1cc5c..d0cdcf9e 100644 --- a/src/token.rs +++ b/src/token.rs @@ -480,7 +480,7 @@ impl Token { #[cfg(not(feature = "no_module"))] As => "as", EOF => "{EOF}", - _ => unreachable!("operator should be matched in outer scope"), + t => unreachable!("operator should be matched in outer scope: {:?}", t), } .into(), } @@ -892,7 +892,7 @@ pub fn parse_string_literal( 'x' => 2, 'u' => 4, 'U' => 8, - _ => unreachable!("expecting 'x', 'u' or 'U', but gets {}", ch), + _ => unreachable!(), }; for _ in 0..len { @@ -1190,14 +1190,14 @@ fn get_next_token_inner( 'x' | 'X' => is_hex_digit, 'o' | 'O' => is_numeric_digit, 'b' | 'B' => is_numeric_digit, - _ => unreachable!("expecting 'x', 'o' or 'b', but gets {}", ch), + _ => unreachable!(), }; radix_base = Some(match ch { 'x' | 'X' => 16, 'o' | 'O' => 8, 'b' | 'B' => 2, - _ => unreachable!("expecting 'x', 'o' or 'b', but gets {}", ch), + _ => unreachable!(), }); } From a7ff9fb24fcb33390b3c35e02c8018838ab6c817 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 7 Mar 2021 22:33:02 +0800 Subject: [PATCH 07/11] Add bytes method for strings. --- CHANGELOG.md | 1 + src/packages/string_more.rs | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85615fb3..2c0f4af9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Enhancements * More information is provided to the error variable captured by the `catch` statement in an _object map_. * Previously, `private` functions in an `AST` cannot be called with `call_fn` etc. This is inconvenient when trying to call a function inside a script which also serves as a loadable module exporting part (but not all) of the functions. Now, all functions (`private` or not) can be called in an `AST`. The `private` keyword is relegated to preventing a function from being exported. * `Dynamic::as_unit` just for completeness sake. +* `bytes` method added for strings to get length quickly (if the string is ASCII-only). Version 0.19.13 diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index b800d90d..783dc2c4 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -12,7 +12,7 @@ def_package!(crate:MoreStringPackage:"Additional string utilities, including str // Register string iterator lib.set_iter( TypeId::of::(), - |string: Dynamic| Box::new(string.cast::().chars().collect::>().into_iter().map(Into::into)) + |string| Box::new(string.cast::().chars().collect::>().into_iter().map(Into::into)) ); }); @@ -42,6 +42,10 @@ mod string_functions { pub fn len(string: &str) -> INT { string.chars().count() as INT } + #[rhai_fn(name = "bytes", get = "bytes")] + pub fn bytes(string: &str) -> INT { + string.len() as INT + } pub fn remove(string: &mut ImmutableString, sub_string: ImmutableString) { *string -= sub_string; } From 1c3a07fe86f7e85a2c364190f984b6286a18673d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 7 Mar 2021 22:37:23 +0800 Subject: [PATCH 08/11] Fix metadata build. --- src/serde/metadata.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index 6531ef39..7f4edb25 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -160,8 +160,9 @@ impl From<&crate::module::FuncInfo> for FnMetadata { } } -impl From> for FnMetadata { - fn from(info: crate::ScriptFnMetadata) -> Self { +#[cfg(not(feature = "no_function"))] +impl From> for FnMetadata { + fn from(info: crate::ast::ScriptFnMetadata) -> Self { Self { namespace: FnNamespace::Global, access: info.access.into(), From 62928f86139b692efbfe0cd4b45f2c3d9529d20d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 8 Mar 2021 15:30:32 +0800 Subject: [PATCH 09/11] Revise function hashing. --- src/ast.rs | 100 +++++++++++++++++++---- src/engine.rs | 147 +++++++++++++++------------------ src/fn_call.rs | 198 +++++++++++++++++++++++---------------------- src/fn_native.rs | 17 +++- src/fn_register.rs | 22 +---- src/lib.rs | 8 +- src/module/mod.rs | 178 +++++++++++++++++++--------------------- src/optimize.rs | 16 ++-- src/parser.rs | 180 +++++++++++++++++++++++------------------ src/utils.rs | 94 ++++++++------------- tests/bit_shift.rs | 2 +- tests/string.rs | 8 +- 12 files changed, 501 insertions(+), 469 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 46789cbd..e586caeb 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -8,7 +8,7 @@ use crate::stdlib::{ boxed::Box, fmt, hash::Hash, - num::{NonZeroU64, NonZeroUsize}, + num::NonZeroUsize, ops::{Add, AddAssign}, string::String, vec, @@ -836,7 +836,7 @@ pub enum Stmt { /// \[`export`\] `const` id `=` expr Const(Box, Option, bool, Position), /// expr op`=` expr - Assignment(Box<(Expr, Cow<'static, str>, Expr)>, Position), + Assignment(Box<(Expr, Expr, Option)>, Position), /// `{` stmt`;` ... `}` Block(Vec, Position), /// `try` `{` stmt; ... `}` `catch` `(` var `)` `{` stmt; ... `}` @@ -1036,7 +1036,7 @@ impl Stmt { } Self::Assignment(x, _) => { x.0.walk(path, on_node); - x.2.walk(path, on_node); + x.1.walk(path, on_node); } Self::Block(x, _) => x.iter().for_each(|s| s.walk(path, on_node)), Self::TryCatch(x, _, _) => { @@ -1083,6 +1083,79 @@ pub struct BinaryExpr { pub rhs: Expr, } +/// _(INTERNALS)_ An op-assignment operator. +/// Exported under the `internals` feature only. +/// +/// # Volatile Data Structure +/// +/// This type is volatile and may change. +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct OpAssignment { + pub hash_op_assign: u64, + pub hash_op: u64, + pub op: Cow<'static, str>, +} + +/// _(INTERNALS)_ An set of function call hashes. +/// Exported under the `internals` feature only. +/// +/// # Volatile Data Structure +/// +/// This type is volatile and may change. +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Default)] +pub struct FnHash { + /// Pre-calculated hash for a script-defined function ([`None`] if native functions only). + script: Option, + /// Pre-calculated hash for a native Rust function with no parameter types. + native: u64, +} + +impl FnHash { + /// Create a [`FnHash`] with only the native Rust hash. + #[inline(always)] + pub fn from_native(hash: u64) -> Self { + Self { + script: None, + native: hash, + } + } + /// Create a [`FnHash`] with both native Rust and script function hashes set to the same value. + #[inline(always)] + pub fn from_script(hash: u64) -> Self { + Self { + script: Some(hash), + native: hash, + } + } + /// Create a [`FnHash`] with both native Rust and script function hashes. + #[inline(always)] + pub fn from_script_and_native(script: u64, native: u64) -> Self { + Self { + script: Some(script), + native, + } + } + /// Is this [`FnHash`] native Rust only? + #[inline(always)] + pub fn is_native_only(&self) -> bool { + self.script.is_none() + } + /// Get the script function hash from this [`FnHash`]. + /// + /// # Panics + /// + /// Panics if the [`FnHash`] is native Rust only. + #[inline(always)] + pub fn script_hash(&self) -> u64 { + self.script.unwrap() + } + /// Get the naive Rust function hash from this [`FnHash`]. + #[inline(always)] + pub fn native_hash(&self) -> u64 { + self.native + } +} + /// _(INTERNALS)_ A function call. /// Exported under the `internals` feature only. /// @@ -1091,9 +1164,8 @@ pub struct BinaryExpr { /// This type is volatile and may change. #[derive(Debug, Clone, Default, Hash)] pub struct FnCallExpr { - /// Pre-calculated hash for a script-defined function of the same name and number of parameters. - /// None if native Rust only. - pub hash_script: Option, + /// Pre-calculated hash. + pub hash: FnHash, /// Does this function call capture the parent scope? pub capture: bool, /// Namespace of the function, if any. Boxed because it occurs rarely. @@ -1225,15 +1297,9 @@ pub enum Expr { /// () Unit(Position), /// Variable access - (optional index, optional (hash, modules), variable name) - Variable( - Box<( - Option, - Option<(NonZeroU64, NamespaceRef)>, - Ident, - )>, - ), - /// Property access - (getter, setter), prop - Property(Box<(ImmutableString, ImmutableString, Ident)>), + Variable(Box<(Option, Option<(u64, NamespaceRef)>, Ident)>), + /// Property access - (getter, hash, setter, hash, prop) + Property(Box<(ImmutableString, u64, ImmutableString, u64, Ident)>), /// { [statement][Stmt] } Stmt(Box>, Position), /// func `(` expr `,` ... `)` @@ -1326,7 +1392,7 @@ impl Expr { Self::FnPointer(_, pos) => *pos, Self::Array(_, pos) => *pos, Self::Map(_, pos) => *pos, - Self::Property(x) => (x.2).pos, + Self::Property(x) => (x.4).pos, Self::Stmt(_, pos) => *pos, Self::Variable(x) => (x.2).pos, Self::FnCall(_, pos) => *pos, @@ -1355,7 +1421,7 @@ impl Expr { Self::Array(_, pos) => *pos = new_pos, Self::Map(_, pos) => *pos = new_pos, Self::Variable(x) => (x.2).pos = new_pos, - Self::Property(x) => (x.2).pos = new_pos, + Self::Property(x) => (x.4).pos = new_pos, Self::Stmt(_, pos) => *pos = new_pos, Self::FnCall(_, pos) => *pos = new_pos, Self::And(_, pos) | Self::Or(_, pos) | Self::In(_, pos) => *pos = new_pos, diff --git a/src/engine.rs b/src/engine.rs index d8ffd7bb..30f6d1a4 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,6 +1,6 @@ //! Main module defining the script evaluation [`Engine`]. -use crate::ast::{Expr, FnCallExpr, Ident, ReturnType, Stmt}; +use crate::ast::{Expr, FnCallExpr, FnHash, Ident, OpAssignment, ReturnType, Stmt}; use crate::dynamic::{map_std_type_name, AccessMode, Union, Variant}; use crate::fn_native::{ CallableFunction, IteratorFn, OnDebugCallback, OnPrintCallback, OnProgressCallback, @@ -25,8 +25,8 @@ use crate::stdlib::{ use crate::syntax::CustomSyntax; use crate::utils::{get_hasher, StraightHasherBuilder}; use crate::{ - calc_native_fn_hash, Dynamic, EvalAltResult, FnPtr, ImmutableString, Module, Position, - RhaiResult, Scope, Shared, StaticVec, + calc_fn_hash, Dynamic, EvalAltResult, FnPtr, ImmutableString, Module, Position, RhaiResult, + Scope, Shared, StaticVec, }; #[cfg(not(feature = "no_index"))] @@ -126,15 +126,12 @@ impl Imports { /// Does the specified function hash key exist in this stack of imported [modules][Module]? #[allow(dead_code)] #[inline(always)] - pub fn contains_fn(&self, hash: NonZeroU64) -> bool { + pub fn contains_fn(&self, hash: u64) -> bool { self.0.iter().any(|(_, m)| m.contains_qualified_fn(hash)) } /// Get specified function via its hash key. #[inline(always)] - pub fn get_fn( - &self, - hash: NonZeroU64, - ) -> Option<(&CallableFunction, Option<&ImmutableString>)> { + pub fn get_fn(&self, hash: u64) -> Option<(&CallableFunction, Option<&ImmutableString>)> { self.0 .iter() .rev() @@ -510,11 +507,7 @@ pub struct State { pub resolver: Option>, /// Functions resolution cache. fn_resolution_caches: StaticVec< - HashMap< - NonZeroU64, - Option<(CallableFunction, Option)>, - StraightHasherBuilder, - >, + HashMap)>, StraightHasherBuilder>, >, } @@ -527,11 +520,8 @@ impl State { /// Get a mutable reference to the current functions resolution cache. pub fn fn_resolution_cache_mut( &mut self, - ) -> &mut HashMap< - NonZeroU64, - Option<(CallableFunction, Option)>, - StraightHasherBuilder, - > { + ) -> &mut HashMap)>, StraightHasherBuilder> + { if self.fn_resolution_caches.is_empty() { self.fn_resolution_caches .push(HashMap::with_capacity_and_hasher(16, StraightHasherBuilder)); @@ -1053,7 +1043,7 @@ impl Engine { idx_values: &mut StaticVec, chain_type: ChainType, level: usize, - new_val: Option<((Dynamic, Position), (&str, Position))>, + new_val: Option<((Dynamic, Position), (&Option, Position))>, ) -> Result<(Dynamic, bool), Box> { assert!(chain_type != ChainType::NonChaining); @@ -1102,9 +1092,9 @@ impl Engine { ) { // Indexed value is a reference - update directly Ok(obj_ptr) => { - let ((new_val, new_pos), (op, op_pos)) = new_val.unwrap(); + let ((new_val, new_pos), (op_info, op_pos)) = new_val.unwrap(); self.eval_op_assignment( - mods, state, lib, op, op_pos, obj_ptr, new_val, new_pos, + mods, state, lib, op_info, op_pos, obj_ptr, new_val, new_pos, )?; None } @@ -1122,11 +1112,13 @@ impl Engine { let val_type_name = target_val.type_name(); let ((_, val_pos), _) = new_val; + let hash_set = + FnHash::from_native(calc_fn_hash(empty(), FN_IDX_SET, 3)); let args = &mut [target_val, &mut idx_val2, &mut (new_val.0).0]; self.exec_fn_call( - mods, state, lib, FN_IDX_SET, None, args, is_ref, true, val_pos, - None, level, + mods, state, lib, FN_IDX_SET, hash_set, args, is_ref, true, + val_pos, None, level, ) .map_err(|err| match *err { EvalAltResult::ErrorFunctionNotFound(fn_sig, _) @@ -1159,11 +1151,7 @@ impl Engine { match rhs { // xxx.fn_name(arg_expr_list) Expr::FnCall(x, pos) if x.namespace.is_none() && new_val.is_none() => { - let FnCallExpr { - name, - hash_script: hash, - .. - } = x.as_ref(); + let FnCallExpr { name, hash, .. } = x.as_ref(); let args = idx_val.as_fn_call_args(); self.make_method_call( mods, state, lib, name, *hash, target, args, *pos, level, @@ -1179,20 +1167,20 @@ impl Engine { } // {xxx:map}.id op= ??? Expr::Property(x) if target_val.is::() && new_val.is_some() => { - let Ident { name, pos } = &x.2; + let Ident { name, pos } = &x.4; let index = name.clone().into(); let val = self.get_indexed_mut( mods, state, lib, target_val, index, *pos, true, is_ref, false, level, )?; - let ((new_val, new_pos), (op, op_pos)) = new_val.unwrap(); + let ((new_val, new_pos), (op_info, op_pos)) = new_val.unwrap(); self.eval_op_assignment( - mods, state, lib, op, op_pos, val, new_val, new_pos, + mods, state, lib, op_info, op_pos, val, new_val, new_pos, )?; Ok((Dynamic::UNIT, true)) } // {xxx:map}.id Expr::Property(x) if target_val.is::() => { - let Ident { name, pos } = &x.2; + let Ident { name, pos } = &x.4; let index = name.clone().into(); let val = self.get_indexed_mut( mods, state, lib, target_val, index, *pos, false, is_ref, false, level, @@ -1202,21 +1190,23 @@ impl Engine { } // xxx.id = ??? Expr::Property(x) if new_val.is_some() => { - let (_, setter, Ident { pos, .. }) = x.as_ref(); + let (_, _, setter, hash_set, Ident { pos, .. }) = x.as_ref(); + let hash = FnHash::from_native(*hash_set); let mut new_val = new_val; let mut args = [target_val, &mut (new_val.as_mut().unwrap().0).0]; self.exec_fn_call( - mods, state, lib, setter, None, &mut args, is_ref, true, *pos, None, + mods, state, lib, setter, hash, &mut args, is_ref, true, *pos, None, level, ) .map(|(v, _)| (v, true)) } // xxx.id Expr::Property(x) => { - let (getter, _, Ident { pos, .. }) = x.as_ref(); + let (getter, hash_get, _, _, Ident { pos, .. }) = x.as_ref(); + let hash = FnHash::from_native(*hash_get); let mut args = [target_val]; self.exec_fn_call( - mods, state, lib, getter, None, &mut args, is_ref, true, *pos, None, + mods, state, lib, getter, hash, &mut args, is_ref, true, *pos, None, level, ) .map(|(v, _)| (v, false)) @@ -1225,7 +1215,7 @@ impl Engine { Expr::Index(x, x_pos) | Expr::Dot(x, x_pos) if target_val.is::() => { let mut val = match &x.lhs { Expr::Property(p) => { - let Ident { name, pos } = &p.2; + let Ident { name, pos } = &p.4; let index = name.clone().into(); self.get_indexed_mut( mods, state, lib, target_val, index, *pos, false, is_ref, true, @@ -1234,11 +1224,7 @@ impl Engine { } // {xxx:map}.fn_name(arg_expr_list)[expr] | {xxx:map}.fn_name(arg_expr_list).expr Expr::FnCall(x, pos) if x.namespace.is_none() => { - let FnCallExpr { - name, - hash_script: hash, - .. - } = x.as_ref(); + let FnCallExpr { name, hash, .. } = x.as_ref(); let args = idx_val.as_fn_call_args(); let (val, _) = self.make_method_call( mods, state, lib, name, *hash, target, args, *pos, level, @@ -1264,12 +1250,15 @@ impl Engine { match &x.lhs { // xxx.prop[expr] | xxx.prop.expr Expr::Property(p) => { - let (getter, setter, Ident { pos, .. }) = p.as_ref(); + let (getter, hash_get, setter, hash_set, Ident { pos, .. }) = + p.as_ref(); + let hash_get = FnHash::from_native(*hash_get); + let hash_set = FnHash::from_native(*hash_set); let arg_values = &mut [target_val, &mut Default::default()]; let args = &mut arg_values[..1]; let (mut val, updated) = self.exec_fn_call( - mods, state, lib, getter, None, args, is_ref, true, *pos, None, - level, + mods, state, lib, getter, hash_get, args, is_ref, true, *pos, + None, level, )?; let val = &mut val; @@ -1294,8 +1283,8 @@ impl Engine { // Re-use args because the first &mut parameter will not be consumed arg_values[1] = val; self.exec_fn_call( - mods, state, lib, setter, None, arg_values, is_ref, true, - *pos, None, level, + mods, state, lib, setter, hash_set, arg_values, is_ref, + true, *pos, None, level, ) .or_else( |err| match *err { @@ -1313,11 +1302,7 @@ impl Engine { } // xxx.fn_name(arg_expr_list)[expr] | xxx.fn_name(arg_expr_list).expr Expr::FnCall(f, pos) if f.namespace.is_none() => { - let FnCallExpr { - name, - hash_script: hash, - .. - } = f.as_ref(); + let FnCallExpr { name, hash, .. } = f.as_ref(); let args = idx_val.as_fn_call_args(); let (mut val, _) = self.make_method_call( mods, state, lib, name, *hash, target, args, *pos, level, @@ -1359,7 +1344,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, expr: &Expr, level: usize, - new_val: Option<((Dynamic, Position), (&str, Position))>, + new_val: Option<((Dynamic, Position), (&Option, Position))>, ) -> RhaiResult { let (crate::ast::BinaryExpr { lhs, rhs }, chain_type, op_pos) = match expr { Expr::Index(x, pos) => (x.as_ref(), ChainType::Index, *pos), @@ -1593,8 +1578,9 @@ impl Engine { let type_name = target.type_name(); let mut idx = idx; let args = &mut [target, &mut idx]; + let hash_get = FnHash::from_native(calc_fn_hash(empty(), FN_IDX_GET, 2)); self.exec_fn_call( - _mods, state, _lib, FN_IDX_GET, None, args, _is_ref, true, idx_pos, None, + _mods, state, _lib, FN_IDX_GET, hash_get, args, _is_ref, true, idx_pos, None, _level, ) .map(|(v, _)| v.into()) @@ -1640,17 +1626,13 @@ impl Engine { #[cfg(not(feature = "no_index"))] Dynamic(Union::Array(mut rhs_value, _)) => { // Call the `==` operator to compare each value + let hash = calc_fn_hash(empty(), OP_EQUALS, 2); for value in rhs_value.iter_mut() { let args = &mut [&mut lhs_value.clone(), value]; - let hash_fn = - calc_native_fn_hash(empty(), OP_EQUALS, args.iter().map(|a| a.type_id())) - .unwrap(); let pos = rhs.position(); if self - .call_native_fn( - mods, state, lib, OP_EQUALS, hash_fn, args, false, false, pos, - )? + .call_native_fn(mods, state, lib, OP_EQUALS, hash, args, false, false, pos)? .0 .as_bool() .unwrap_or(false) @@ -1764,7 +1746,7 @@ impl Engine { let FnCallExpr { name, capture: cap_scope, - hash_script: hash, + hash, args, .. } = x.as_ref(); @@ -1778,12 +1760,12 @@ impl Engine { let FnCallExpr { name, namespace, - hash_script, + hash, args, .. } = x.as_ref(); let namespace = namespace.as_ref(); - let hash = hash_script.unwrap(); + let hash = hash.native_hash(); self.make_qualified_function_call( scope, mods, state, lib, this_ptr, namespace, name, args, hash, *pos, level, ) @@ -1927,7 +1909,7 @@ impl Engine { mods: &mut Imports, state: &mut State, lib: &[&Module], - op: &str, + op_info: &Option, op_pos: Position, mut target: Target, mut new_value: Dynamic, @@ -1937,11 +1919,12 @@ impl Engine { unreachable!("LHS should not be read-only"); } - if op.is_empty() { - // Normal assignment - target.set_value(new_value, new_value_pos)?; - Ok(()) - } else { + if let Some(OpAssignment { + hash_op_assign, + hash_op, + op, + }) = op_info + { let mut lock_guard; let lhs_ptr_inner; @@ -1952,28 +1935,30 @@ impl Engine { lhs_ptr_inner = target.as_mut(); } + let hash = *hash_op_assign; let args = &mut [lhs_ptr_inner, &mut new_value]; - let hash_fn = - calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); - match self.call_native_fn(mods, state, lib, op, hash_fn, args, true, true, op_pos) { + match self.call_native_fn(mods, state, lib, op, hash, args, true, true, op_pos) { Ok(_) => (), - Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) => + Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op.as_ref())) => { // Expand to `var = var op rhs` let op = &op[..op.len() - 1]; // extract operator without = - let hash_fn = - calc_native_fn_hash(empty(), op, args.iter().map(|a| a.type_id())).unwrap(); // Run function - let (value, _) = self - .call_native_fn(mods, state, lib, op, hash_fn, args, true, false, op_pos)?; + let (value, _) = self.call_native_fn( + mods, state, lib, op, *hash_op, args, true, false, op_pos, + )?; *args[0] = value.flatten(); } err => return err.map(|_| ()), } + Ok(()) + } else { + // Normal assignment + target.set_value(new_value, new_value_pos)?; Ok(()) } } @@ -2007,7 +1992,7 @@ impl Engine { // var op= rhs Stmt::Assignment(x, op_pos) if x.0.get_variable_access(false).is_some() => { - let (lhs_expr, op, rhs_expr) = x.as_ref(); + let (lhs_expr, rhs_expr, op_info) = x.as_ref(); let rhs_val = self .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)? .flatten(); @@ -2035,7 +2020,7 @@ impl Engine { mods, state, lib, - op, + op_info, *op_pos, lhs_ptr, rhs_val, @@ -2047,11 +2032,11 @@ impl Engine { // lhs op= rhs Stmt::Assignment(x, op_pos) => { - let (lhs_expr, op, rhs_expr) = x.as_ref(); + let (lhs_expr, rhs_expr, op_info) = x.as_ref(); let rhs_val = self .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)? .flatten(); - let _new_val = Some(((rhs_val, rhs_expr.position()), (op.as_ref(), *op_pos))); + let _new_val = Some(((rhs_val, rhs_expr.position()), (op_info, *op_pos))); // Must be either `var[index] op= val` or `var.prop op= val` match lhs_expr { diff --git a/src/fn_call.rs b/src/fn_call.rs index 35c7a768..b0d43420 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -1,5 +1,6 @@ //! Implement function-calling mechanism for [`Engine`]. +use crate::ast::FnHash; use crate::engine::{ Imports, State, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_FN_PTR_CALL, KEYWORD_FN_PTR_CURRY, KEYWORD_IS_DEF_VAR, KEYWORD_PRINT, KEYWORD_TYPE_OF, @@ -16,18 +17,16 @@ use crate::stdlib::{ format, iter::{empty, once}, mem, - num::NonZeroU64, string::{String, ToString}, vec::Vec, }; -use crate::utils::combine_hashes; use crate::{ ast::{Expr, Stmt}, fn_native::CallableFunction, RhaiResult, }; use crate::{ - calc_native_fn_hash, calc_script_fn_hash, Dynamic, Engine, EvalAltResult, FnPtr, + calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, EvalAltResult, FnPtr, ImmutableString, Module, ParseErrorType, Position, Scope, StaticVec, }; @@ -183,20 +182,27 @@ impl Engine { state: &'s mut State, lib: &[&Module], fn_name: &str, - mut hash: NonZeroU64, - args: &mut FnCallArgs, + hash_script: u64, + args: Option<&mut FnCallArgs>, allow_dynamic: bool, is_op_assignment: bool, ) -> &'s Option<(CallableFunction, Option)> { + let mut hash = if let Some(ref args) = args { + let hash_params = calc_fn_params_hash(args.iter().map(|a| a.type_id())); + combine_hashes(hash_script, hash_params) + } else { + hash_script + }; + &*state .fn_resolution_cache_mut() .entry(hash) .or_insert_with(|| { - let num_args = args.len(); + let num_args = args.as_ref().map(|a| a.len()).unwrap_or(0); let max_bitmask = if !allow_dynamic { 0 } else { - 1usize << args.len().min(MAX_DYNAMIC_PARAMETERS) + 1usize << num_args.min(MAX_DYNAMIC_PARAMETERS) }; let mut bitmask = 1usize; // Bitmask of which parameter to replace with `Dynamic` @@ -238,39 +244,41 @@ impl Engine { None if bitmask >= max_bitmask => { return if num_args != 2 { None - } else if !is_op_assignment { - if let Some(f) = - get_builtin_binary_op_fn(fn_name, &args[0], &args[1]) - { - Some(( - CallableFunction::from_method(Box::new(f) as Box), - None, - )) + } else if let Some(ref args) = args { + if !is_op_assignment { + if let Some(f) = + get_builtin_binary_op_fn(fn_name, &args[0], &args[1]) + { + Some(( + CallableFunction::from_method(Box::new(f) as Box), + None, + )) + } else { + None + } } else { - None + let (first, second) = args.split_first().unwrap(); + + if let Some(f) = + get_builtin_op_assignment_fn(fn_name, *first, second[0]) + { + Some(( + CallableFunction::from_method(Box::new(f) as Box), + None, + )) + } else { + None + } } } else { - let (first, second) = args.split_first().unwrap(); - - if let Some(f) = - get_builtin_op_assignment_fn(fn_name, *first, second[0]) - { - Some(( - CallableFunction::from_method(Box::new(f) as Box), - None, - )) - } else { - None - } + None } } // Try all permutations with `Dynamic` wildcards None => { - hash = calc_native_fn_hash( - empty(), - fn_name, - args.iter().enumerate().map(|(i, a)| { + let hash_params = calc_fn_params_hash( + args.as_ref().unwrap().iter().enumerate().map(|(i, a)| { let mask = 1usize << (num_args - i - 1); if bitmask & mask != 0 { // Replace with `Dynamic` @@ -279,8 +287,8 @@ impl Engine { a.type_id() } }), - ) - .unwrap(); + ); + hash = combine_hashes(hash_script, hash_params); bitmask += 1; } @@ -302,7 +310,7 @@ impl Engine { state: &mut State, lib: &[&Module], fn_name: &str, - hash_fn: NonZeroU64, + hash_native: u64, args: &mut FnCallArgs, is_ref: bool, is_op_assignment: bool, @@ -318,8 +326,8 @@ impl Engine { state, lib, fn_name, - hash_fn, - args, + hash_native, + Some(args), true, is_op_assignment, ); @@ -569,9 +577,9 @@ impl Engine { result } - // Has a system function an override? + // Has a system function a Rust-native override? #[inline(always)] - pub(crate) fn has_override_by_name_and_arguments( + pub(crate) fn has_native_override( &self, mods: Option<&Imports>, state: &mut State, @@ -579,15 +587,11 @@ impl Engine { fn_name: &str, arg_types: &[TypeId], ) -> bool { - let arg_types = arg_types.as_ref(); + let hash_script = calc_fn_hash(empty(), fn_name, arg_types.len()); + let hash_params = calc_fn_params_hash(arg_types.iter().cloned()); + let hash_fn = combine_hashes(hash_script, hash_params); - self.has_override( - mods, - state, - lib, - calc_native_fn_hash(empty(), fn_name, arg_types.iter().cloned()), - calc_script_fn_hash(empty(), fn_name, arg_types.len()), - ) + self.has_override(mods, state, lib, Some(hash_fn), None) } // Has a system function an override? @@ -597,8 +601,8 @@ impl Engine { mods: Option<&Imports>, state: &mut State, lib: &[&Module], - hash_fn: Option, - hash_script: Option, + hash_fn: Option, + hash_script: Option, ) -> bool { let cache = state.fn_resolution_cache_mut(); @@ -649,7 +653,7 @@ impl Engine { state: &mut State, lib: &[&Module], fn_name: &str, - _hash_script: Option, + hash: FnHash, args: &mut FnCallArgs, is_ref: bool, _is_method: bool, @@ -684,9 +688,8 @@ impl Engine { if num_params < 0 { Dynamic::FALSE } else { - let hash_script = - calc_script_fn_hash(empty(), &fn_name, num_params as usize); - self.has_override(Some(mods), state, lib, None, hash_script) + let hash_script = calc_fn_hash(empty(), &fn_name, num_params as usize); + self.has_override(Some(mods), state, lib, None, Some(hash_script)) .into() }, false, @@ -731,9 +734,16 @@ impl Engine { _ => (), } + // Scripted function call? + let hash_script = if hash.is_native_only() { + None + } else { + Some(hash.script_hash()) + }; + #[cfg(not(feature = "no_function"))] - if let Some((func, source)) = _hash_script.and_then(|hash| { - self.resolve_function(mods, state, lib, fn_name, hash, args, false, false) + if let Some((func, source)) = hash_script.and_then(|hash| { + self.resolve_function(mods, state, lib, fn_name, hash, None, false, false) .as_ref() .map(|(f, s)| (f.clone(), s.clone())) }) { @@ -815,9 +825,17 @@ impl Engine { } // Native function call - let hash_fn = - calc_native_fn_hash(empty(), fn_name, args.iter().map(|a| a.type_id())).unwrap(); - self.call_native_fn(mods, state, lib, fn_name, hash_fn, args, is_ref, false, pos) + self.call_native_fn( + mods, + state, + lib, + fn_name, + hash.native_hash(), + args, + is_ref, + false, + pos, + ) } /// Evaluate a list of statements with no `this` pointer. @@ -894,7 +912,7 @@ impl Engine { state: &mut State, lib: &[&Module], fn_name: &str, - hash_script: Option, + mut hash: FnHash, target: &mut crate::engine::Target, mut call_args: StaticVec, pos: Position, @@ -913,9 +931,8 @@ impl Engine { // Redirect function name let fn_name = fn_ptr.fn_name(); let args_len = call_args.len() + fn_ptr.curry().len(); - // Recalculate hash - let hash = - hash_script.and_then(|_| calc_script_fn_hash(empty(), fn_name, args_len)); + // Recalculate hashes + let hash = FnHash::from_script(calc_fn_hash(empty(), fn_name, args_len)); // Arguments are passed as-is, adding the curried arguments let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = curry @@ -936,8 +953,10 @@ impl Engine { let fn_name = fn_ptr.fn_name(); let args_len = call_args.len() + fn_ptr.curry().len(); // Recalculate hash - let hash = - hash_script.and_then(|_| calc_script_fn_hash(empty(), fn_name, args_len)); + let hash = FnHash::from_script_and_native( + calc_fn_hash(empty(), fn_name, args_len), + calc_fn_hash(empty(), fn_name, args_len + 1), + ); // Replace the first argument with the object pointer, adding the curried arguments let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = once(obj) @@ -977,7 +996,6 @@ impl Engine { _ => { let _redirected; - let mut hash = hash_script; // Check if it is a map method call in OOP style #[cfg(not(feature = "no_object"))] @@ -995,17 +1013,14 @@ impl Engine { .enumerate() .for_each(|(i, v)| call_args.insert(i, v)); // Recalculate the hash based on the new function name and new arguments - hash = hash_script.and_then(|_| { - calc_script_fn_hash(empty(), fn_name, call_args.len()) - }); + hash = FnHash::from_script_and_native( + calc_fn_hash(empty(), fn_name, call_args.len()), + calc_fn_hash(empty(), fn_name, call_args.len() + 1), + ); } } }; - if hash_script.is_none() { - hash = None; - } - // Attached object pointer in front of the arguments let mut arg_values = once(obj) .chain(call_args.iter_mut()) @@ -1036,7 +1051,7 @@ impl Engine { this_ptr: &mut Option<&mut Dynamic>, fn_name: &str, args_expr: &[Expr], - mut hash_script: Option, + mut hash: FnHash, pos: Position, capture_scope: bool, level: usize, @@ -1074,7 +1089,11 @@ impl Engine { // Recalculate hash let args_len = args_expr.len() + curry.len(); - hash_script = calc_script_fn_hash(empty(), name, args_len); + hash = if !hash.is_native_only() { + FnHash::from_script(calc_fn_hash(empty(), name, args_len)) + } else { + FnHash::from_native(calc_fn_hash(empty(), name, args_len)) + }; } // Handle Fn() @@ -1145,8 +1164,8 @@ impl Engine { return Ok(if num_params < 0 { Dynamic::FALSE } else { - let hash_script = calc_script_fn_hash(empty(), &fn_name, num_params as usize); - self.has_override(Some(mods), state, lib, None, hash_script) + let hash_script = calc_fn_hash(empty(), &fn_name, num_params as usize); + self.has_override(Some(mods), state, lib, None, Some(hash_script)) .into() }); } @@ -1266,17 +1285,7 @@ impl Engine { let args = args.as_mut(); self.exec_fn_call( - mods, - state, - lib, - name, - hash_script, - args, - is_ref, - false, - pos, - capture, - level, + mods, state, lib, name, hash, args, is_ref, false, pos, capture, level, ) .map(|(v, _)| v) } @@ -1292,7 +1301,7 @@ impl Engine { namespace: Option<&NamespaceRef>, fn_name: &str, args_expr: &[Expr], - hash_script: NonZeroU64, + hash: u64, pos: Position, level: usize, ) -> RhaiResult { @@ -1357,20 +1366,13 @@ impl Engine { })?; // First search in script-defined functions (can override built-in) - let func = match module.get_qualified_fn(hash_script) { + let func = match module.get_qualified_fn(hash) { // Then search in Rust functions None => { self.inc_operations(state, pos)?; - // 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 = - calc_native_fn_hash(empty(), "", args.iter().map(|a| a.type_id())).unwrap(); - // 3) The two hashes are combined. - let hash_qualified_fn = combine_hashes(hash_script, hash_fn_args); + let hash_params = calc_fn_params_hash(args.iter().map(|a| a.type_id())); + let hash_qualified_fn = combine_hashes(hash, hash_params); module.get_qualified_fn(hash_qualified_fn) } diff --git a/src/fn_native.rs b/src/fn_native.rs index d7c299dd..ace60b56 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -1,6 +1,6 @@ //! Module defining interfaces to native-Rust functions. -use crate::ast::FnAccess; +use crate::ast::{FnAccess, FnHash}; use crate::engine::Imports; use crate::plugin::PluginFunction; use crate::stdlib::{ @@ -14,8 +14,8 @@ use crate::stdlib::{ }; use crate::token::is_valid_identifier; use crate::{ - calc_script_fn_hash, Dynamic, Engine, EvalAltResult, EvalContext, ImmutableString, Module, - Position, RhaiResult, + calc_fn_hash, Dynamic, Engine, EvalAltResult, EvalContext, ImmutableString, Module, Position, + RhaiResult, }; /// Trait that maps to `Send + Sync` only under the `sync` feature. @@ -189,13 +189,22 @@ impl<'a> NativeCallContext<'a> { is_method: bool, args: &mut [&mut Dynamic], ) -> RhaiResult { + let hash = if is_method { + FnHash::from_script_and_native( + calc_fn_hash(empty(), fn_name, args.len() - 1), + calc_fn_hash(empty(), fn_name, args.len()), + ) + } else { + FnHash::from_script(calc_fn_hash(empty(), fn_name, args.len())) + }; + self.engine() .exec_fn_call( &mut self.mods.cloned().unwrap_or_default(), &mut Default::default(), self.lib, fn_name, - calc_script_fn_hash(empty(), fn_name, args.len() - if is_method { 1 } else { 0 }), + hash, args, is_method, is_method, diff --git a/src/fn_register.rs b/src/fn_register.rs index ea7dc9b0..38f2d272 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -6,9 +6,7 @@ use crate::dynamic::{DynamicWriteLock, Variant}; use crate::fn_native::{CallableFunction, FnAny, FnCallArgs, SendSync}; use crate::r#unsafe::unsafe_cast_box; use crate::stdlib::{any::TypeId, boxed::Box, mem, string::String}; -use crate::{ - Dynamic, Engine, FnAccess, FnNamespace, ImmutableString, NativeCallContext, RhaiResult, -}; +use crate::{Dynamic, Engine, FnAccess, FnNamespace, NativeCallContext, RhaiResult}; /// Trait to register custom functions with the [`Engine`]. pub trait RegisterFn { @@ -154,20 +152,6 @@ pub fn map_result(data: RhaiResult) -> RhaiResult { data } -/// Remap `&str` | `String` to `ImmutableString`. -#[inline(always)] -fn map_type_id() -> TypeId { - let ref_id = TypeId::of::(); - - if ref_id == TypeId::of::<&str>() { - TypeId::of::() - } else if ref_id == TypeId::of::() { - TypeId::of::() - } else { - TypeId::of::() - } -} - macro_rules! def_register { () => { def_register!(imp from_pure :); @@ -188,7 +172,7 @@ macro_rules! def_register { #[inline(always)] fn register_fn(&mut self, name: &str, f: FN) -> &mut Self { self.global_namespace.set_fn(name, FnNamespace::Global, FnAccess::Public, None, - &[$(map_type_id::<$param, $par>()),*], + &[$(TypeId::of::<$par>()),*], CallableFunction::$abi(make_func!(f : map_dynamic ; $($par => $let => $clone => $arg),*)) ); self @@ -203,7 +187,7 @@ macro_rules! def_register { #[inline(always)] fn register_result_fn(&mut self, name: &str, f: FN) -> &mut Self { self.global_namespace.set_fn(name, FnNamespace::Global, FnAccess::Public, None, - &[$(map_type_id::<$param, $par>()),*], + &[$(TypeId::of::<$par>()),*], CallableFunction::$abi(make_func!(f : map_result ; $($par => $let => $clone => $arg),*)) ); self diff --git a/src/lib.rs b/src/lib.rs index 01d02910..1ed55ca3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -142,10 +142,10 @@ pub use fn_native::Shared; use fn_native::Locked; #[cfg(feature = "internals")] -pub use utils::{calc_native_fn_hash, calc_script_fn_hash, HashableHashMap}; +pub use utils::{calc_fn_hash, calc_fn_params_hash, combine_hashes, HashableHashMap}; #[cfg(not(feature = "internals"))] -pub(crate) use utils::{calc_native_fn_hash, calc_script_fn_hash}; +pub(crate) use utils::{calc_fn_hash, calc_fn_params_hash, combine_hashes}; pub use rhai_codegen::*; @@ -195,8 +195,8 @@ pub use token::{get_next_token, parse_string_literal, InputStream, Token, Tokeni #[cfg(feature = "internals")] #[deprecated = "this type is volatile and may change"] pub use ast::{ - ASTNode, BinaryExpr, CustomExpr, Expr, FloatWrapper, FnCallExpr, Ident, ReturnType, - ScriptFnDef, Stmt, + ASTNode, BinaryExpr, CustomExpr, Expr, FloatWrapper, FnCallExpr, FnHash, Ident, OpAssignment, + ReturnType, ScriptFnDef, Stmt, }; #[cfg(feature = "internals")] diff --git a/src/module/mod.rs b/src/module/mod.rs index ed9f2dab..c79e2396 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -10,16 +10,16 @@ use crate::stdlib::{ collections::HashMap, fmt, format, iter::empty, - num::NonZeroU64, num::NonZeroUsize, ops::{Add, AddAssign, Deref, DerefMut}, string::{String, ToString}, vec::Vec, }; use crate::token::Token; -use crate::utils::{combine_hashes, StraightHasherBuilder}; +use crate::utils::StraightHasherBuilder; use crate::{ - Dynamic, EvalAltResult, ImmutableString, NativeCallContext, Position, Shared, StaticVec, + calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, EvalAltResult, ImmutableString, + NativeCallContext, Position, Shared, StaticVec, }; #[cfg(not(feature = "no_index"))] @@ -101,6 +101,27 @@ impl FuncInfo { } } +/// _(INTERNALS)_ Calculate a [`u64`] hash key from a namespace-qualified function name and +/// parameter types. +/// Exported under the `internals` feature only. +/// +/// Module names are passed in via `&str` references from an iterator. +/// Parameter types are passed in via [`TypeId`] values from an iterator. +/// +/// # Note +/// +/// The first module name is skipped. Hashing starts from the _second_ module in the chain. +#[inline(always)] +fn calc_native_fn_hash<'a>( + modules: impl Iterator, + fn_name: &str, + params: &[TypeId], +) -> u64 { + let hash_script = calc_fn_hash(modules, fn_name, params.len()); + let hash_params = calc_fn_params_hash(params.iter().cloned()); + combine_hashes(hash_script, hash_params) +} + /// A module which may contain variables, sub-modules, external Rust functions, /// and/or script-defined functions. #[derive(Clone)] @@ -112,12 +133,12 @@ pub struct Module { /// [`Module`] variables. variables: HashMap, /// Flattened collection of all [`Module`] variables, including those in sub-modules. - all_variables: HashMap, + all_variables: HashMap, /// External Rust functions. - functions: HashMap, + functions: HashMap, /// Flattened collection of all external Rust functions, native or scripted. /// including those in sub-modules. - all_functions: HashMap, + all_functions: HashMap, /// Iterator functions, keyed by the type producing the iterator. type_iterators: HashMap, /// Flattened collection of iterator functions, including those in sub-modules. @@ -434,19 +455,13 @@ impl Module { ) -> &mut Self { self.variables.insert(name.into(), Dynamic::from(value)); self.indexed = false; - self.contains_indexed_global_functions = false; self } /// Get a reference to a namespace-qualified variable. /// Name and Position in [`EvalAltResult`] are [`None`] and [`NONE`][Position::NONE] and must be set afterwards. - /// - /// The [`NonZeroU64`] hash is calculated by the function [`calc_native_fn_hash`][crate::calc_native_fn_hash]. #[inline(always)] - pub(crate) fn get_qualified_var( - &self, - hash_var: NonZeroU64, - ) -> Result<&Dynamic, Box> { + pub(crate) fn get_qualified_var(&self, hash_var: u64) -> Result<&Dynamic, Box> { self.all_variables.get(&hash_var).ok_or_else(|| { EvalAltResult::ErrorVariableNotFound(String::new(), Position::NONE).into() }) @@ -460,12 +475,12 @@ impl Module { pub(crate) fn set_script_fn( &mut self, fn_def: impl Into>, - ) -> NonZeroU64 { + ) -> u64 { let fn_def = fn_def.into(); // None + function name + number of arguments. let num_params = fn_def.params.len(); - let hash_script = crate::calc_script_fn_hash(empty(), &fn_def.name, num_params).unwrap(); + let hash_script = crate::calc_fn_hash(empty(), &fn_def.name, num_params); let mut param_names: StaticVec<_> = fn_def.params.iter().cloned().collect(); param_names.push("Dynamic".into()); self.functions.insert( @@ -593,8 +608,7 @@ impl Module { /// Does the particular Rust function exist in the [`Module`]? /// - /// The [`NonZeroU64`] hash is calculated by the function [`calc_native_fn_hash`][crate::calc_native_fn_hash]. - /// It is also returned by the `set_fn_XXX` calls. + /// The [`u64`] hash is returned by the `set_fn_XXX` calls. /// /// # Example /// @@ -606,7 +620,7 @@ impl Module { /// assert!(module.contains_fn(hash, true)); /// ``` #[inline(always)] - pub fn contains_fn(&self, hash_fn: NonZeroU64, public_only: bool) -> bool { + pub fn contains_fn(&self, hash_fn: u64, public_only: bool) -> bool { if public_only { self.functions .get(&hash_fn) @@ -621,9 +635,7 @@ impl Module { /// Update the metadata (parameter names/types and return type) of a registered function. /// - /// The [`NonZeroU64`] hash is calculated either by the function - /// [`calc_native_fn_hash`][crate::calc_native_fn_hash] or the function - /// [`calc_script_fn_hash`][crate::calc_script_fn_hash]. + /// The [`u64`] hash is returned by the `set_fn_XXX` calls. /// /// ## Parameter Names and Types /// @@ -634,7 +646,7 @@ impl Module { /// The _last entry_ in the list should be the _return type_ of the function. /// In other words, the number of entries should be one larger than the number of parameters. #[inline(always)] - pub fn update_fn_metadata(&mut self, hash_fn: NonZeroU64, arg_names: &[&str]) -> &mut Self { + pub fn update_fn_metadata(&mut self, hash_fn: u64, arg_names: &[&str]) -> &mut Self { if let Some(f) = self.functions.get_mut(&hash_fn) { f.param_names = arg_names.iter().map(|&n| n.into()).collect(); } @@ -643,15 +655,9 @@ impl Module { /// Update the namespace of a registered function. /// - /// The [`NonZeroU64`] hash is calculated either by the function - /// [`calc_native_fn_hash`][crate::calc_native_fn_hash] or the function - /// [`calc_script_fn_hash`][crate::calc_script_fn_hash]. + /// The [`u64`] hash is returned by the `set_fn_XXX` calls. #[inline(always)] - pub fn update_fn_namespace( - &mut self, - hash_fn: NonZeroU64, - namespace: FnNamespace, - ) -> &mut Self { + pub fn update_fn_namespace(&mut self, hash_fn: u64, namespace: FnNamespace) -> &mut Self { if let Some(f) = self.functions.get_mut(&hash_fn) { f.namespace = namespace; } @@ -676,24 +682,34 @@ impl Module { arg_names: Option<&[&str]>, arg_types: &[TypeId], func: CallableFunction, - ) -> NonZeroU64 { + ) -> u64 { let name = name.into(); - - let hash_fn = - crate::calc_native_fn_hash(empty(), &name, arg_types.iter().cloned()).unwrap(); + let is_method = func.is_method(); let param_types = arg_types - .into_iter() + .iter() .cloned() - .map(|id| { - if id == TypeId::of::<&str>() || id == TypeId::of::() { - TypeId::of::() + .enumerate() + .map(|(i, type_id)| { + if !is_method || i > 0 { + if type_id == TypeId::of::<&str>() { + // Map &str to ImmutableString + TypeId::of::() + } else if type_id == TypeId::of::() { + // Map String to ImmutableString + TypeId::of::() + } else { + type_id + } } else { - id + // Do not map &mut parameter + type_id } }) .collect::>(); + let hash_fn = calc_native_fn_hash(empty(), &name, ¶m_types); + self.functions.insert( hash_fn, FuncInfo { @@ -793,7 +809,7 @@ impl Module { func: impl Fn(NativeCallContext, &mut FnCallArgs) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |ctx: NativeCallContext, args: &mut FnCallArgs| func(ctx, args).map(Dynamic::from); @@ -829,7 +845,7 @@ impl Module { &mut self, name: impl Into, func: impl Fn() -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, _: &mut FnCallArgs| func().map(Dynamic::from); let arg_types = []; self.set_fn( @@ -864,7 +880,7 @@ impl Module { &mut self, name: impl Into, func: impl Fn(A) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { func(cast_arg::(&mut args[0])).map(Dynamic::from) }; @@ -904,7 +920,7 @@ impl Module { name: impl Into, namespace: FnNamespace, func: impl Fn(&mut A) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { func(&mut args[0].write_lock::().unwrap()).map(Dynamic::from) }; @@ -943,7 +959,7 @@ impl Module { &mut self, name: impl Into, func: impl Fn(&mut A) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { self.set_fn_1_mut( crate::engine::make_getter(&name.into()), FnNamespace::Global, @@ -975,7 +991,7 @@ impl Module { &mut self, name: impl Into, func: impl Fn(A, B) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { let a = cast_arg::(&mut args[0]); let b = cast_arg::(&mut args[1]); @@ -1022,7 +1038,7 @@ impl Module { name: impl Into, namespace: FnNamespace, func: impl Fn(&mut A, B) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { let b = cast_arg::(&mut args[1]); let a = &mut args[0].write_lock::().unwrap(); @@ -1068,7 +1084,7 @@ impl Module { &mut self, name: impl Into, func: impl Fn(&mut A, B) -> Result<(), Box> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { self.set_fn_2_mut( crate::engine::make_setter(&name.into()), FnNamespace::Global, @@ -1107,7 +1123,7 @@ impl Module { pub fn set_indexer_get_fn( &mut self, func: impl Fn(&mut A, B) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -1154,7 +1170,7 @@ impl Module { &mut self, name: impl Into, func: impl Fn(A, B, C) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { let a = cast_arg::(&mut args[0]); let b = cast_arg::(&mut args[1]); @@ -1207,7 +1223,7 @@ impl Module { name: impl Into, namespace: FnNamespace, func: impl Fn(&mut A, B, C) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { let b = cast_arg::(&mut args[2]); let c = cast_arg::(&mut args[3]); @@ -1258,7 +1274,7 @@ impl Module { pub fn set_indexer_set_fn( &mut self, func: impl Fn(&mut A, B, C) -> Result<(), Box> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -1330,7 +1346,7 @@ impl Module { &mut self, get_fn: impl Fn(&mut A, B) -> Result> + SendSync + 'static, set_fn: impl Fn(&mut A, B, T) -> Result<(), Box> + SendSync + 'static, - ) -> (NonZeroU64, NonZeroU64) { + ) -> (u64, u64) { ( self.set_indexer_get_fn(get_fn), self.set_indexer_set_fn(set_fn), @@ -1367,7 +1383,7 @@ impl Module { &mut self, name: impl Into, func: impl Fn(A, B, C, D) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { let a = cast_arg::(&mut args[0]); let b = cast_arg::(&mut args[1]); @@ -1427,7 +1443,7 @@ impl Module { name: impl Into, namespace: FnNamespace, func: impl Fn(&mut A, B, C, D) -> Result> + SendSync + 'static, - ) -> NonZeroU64 { + ) -> u64 { let f = move |_: NativeCallContext, args: &mut FnCallArgs| { let b = cast_arg::(&mut args[1]); let c = cast_arg::(&mut args[2]); @@ -1454,14 +1470,9 @@ impl Module { /// Get a Rust function. /// - /// The [`NonZeroU64`] hash is calculated by the function [`calc_native_fn_hash`][crate::calc_native_fn_hash]. - /// It is also returned by the `set_fn_XXX` calls. + /// The [`u64`] hash is returned by the `set_fn_XXX` calls. #[inline(always)] - pub(crate) fn get_fn( - &self, - hash_fn: NonZeroU64, - public_only: bool, - ) -> Option<&CallableFunction> { + pub(crate) fn get_fn(&self, hash_fn: u64, public_only: bool) -> Option<&CallableFunction> { self.functions .get(&hash_fn) .and_then(|FuncInfo { access, func, .. }| match access { @@ -1473,24 +1484,17 @@ impl Module { /// Does the particular namespace-qualified function exist in the [`Module`]? /// - /// The [`NonZeroU64`] hash is calculated by the function - /// [`calc_native_fn_hash`][crate::calc_native_fn_hash] and must match - /// the hash calculated by [`build_index`][Module::build_index]. + /// The [`u64`] hash is calculated by [`build_index`][Module::build_index]. #[inline(always)] - pub fn contains_qualified_fn(&self, hash_fn: NonZeroU64) -> bool { + pub fn contains_qualified_fn(&self, hash_fn: u64) -> bool { self.all_functions.contains_key(&hash_fn) } /// Get a namespace-qualified function. /// - /// The [`NonZeroU64`] hash is calculated by the function - /// [`calc_native_fn_hash`][crate::calc_native_fn_hash] and must match - /// the hash calculated by [`build_index`][Module::build_index]. + /// The [`u64`] hash is calculated by [`build_index`][Module::build_index]. #[inline(always)] - pub(crate) fn get_qualified_fn( - &self, - hash_qualified_fn: NonZeroU64, - ) -> Option<&CallableFunction> { + pub(crate) fn get_qualified_fn(&self, hash_qualified_fn: u64) -> Option<&CallableFunction> { self.all_functions.get(&hash_qualified_fn) } @@ -1854,8 +1858,8 @@ impl Module { fn index_module<'a>( module: &'a Module, qualifiers: &mut Vec<&'a str>, - variables: &mut HashMap, - functions: &mut HashMap, + variables: &mut HashMap, + functions: &mut HashMap, type_iterators: &mut HashMap, ) -> bool { let mut contains_indexed_global_functions = false; @@ -1871,8 +1875,7 @@ impl Module { // Index all variables module.variables.iter().for_each(|(var_name, value)| { - let hash_var = - crate::calc_script_fn_hash(qualifiers.iter().map(|&v| v), var_name, 0).unwrap(); + let hash_var = crate::calc_fn_hash(qualifiers.iter().map(|&v| v), var_name, 0); variables.insert(hash_var, value.clone()); }); @@ -1909,26 +1912,13 @@ impl Module { 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()) - .unwrap(); - // 3) The two hashes are combined. - let hash_qualified_fn = combine_hashes(hash_qualified_script, hash_fn_args); - + let hash_qualified_fn = + calc_native_fn_hash(qualifiers.iter().cloned(), name, param_types); functions.insert(hash_qualified_fn, func.clone()); } else if cfg!(not(feature = "no_function")) { + let hash_qualified_script = + crate::calc_fn_hash(qualifiers.iter().cloned(), name, *params); functions.insert(hash_qualified_script, func.clone()); } }, @@ -2016,7 +2006,7 @@ impl Module { /// _(INTERNALS)_ A chain of [module][Module] names to namespace-qualify a variable or function call. /// Exported under the `internals` feature only. /// -/// A [`NonZeroU64`] offset to the current [`Scope`][crate::Scope] is cached for quick search purposes. +/// A [`u64`] offset to the current [`Scope`][crate::Scope] is cached for quick search purposes. /// /// A [`StaticVec`] is used because most namespace-qualified access contains only one level, /// and it is wasteful to always allocate a [`Vec`] with one element. diff --git a/src/optimize.rs b/src/optimize.rs index 278f2588..653c05f0 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -15,8 +15,8 @@ use crate::stdlib::{ vec::Vec, }; use crate::token::is_valid_identifier; -use crate::utils::get_hasher; -use crate::{calc_native_fn_hash, Dynamic, Engine, Module, Position, Scope, StaticVec, AST}; +use crate::utils::{calc_fn_hash, get_hasher}; +use crate::{Dynamic, Engine, Module, Position, Scope, StaticVec, AST}; /// Level of optimization performed. #[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] @@ -133,7 +133,7 @@ fn call_fn_with_constant_arguments( arg_values: &mut [Dynamic], ) -> Option { // Search built-in's and external functions - let hash_fn = calc_native_fn_hash(empty(), fn_name, arg_values.iter().map(|a| a.type_id())); + let hash_native = calc_fn_hash(empty(), fn_name, arg_values.len()); state .engine @@ -142,7 +142,7 @@ fn call_fn_with_constant_arguments( &mut Default::default(), state.lib, fn_name, - hash_fn.unwrap(), + hash_native, arg_values.iter_mut().collect::>().as_mut(), false, false, @@ -295,10 +295,10 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { match stmt { // expr op= expr Stmt::Assignment(x, _) => match x.0 { - Expr::Variable(_) => optimize_expr(&mut x.2, state), + Expr::Variable(_) => optimize_expr(&mut x.1, state), _ => { optimize_expr(&mut x.0, state); - optimize_expr(&mut x.2, state); + optimize_expr(&mut x.1, state); } }, @@ -518,7 +518,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { Expr::Dot(x, _) => match (&mut x.lhs, &mut x.rhs) { // map.string (Expr::Map(m, pos), Expr::Property(p)) if m.iter().all(|(_, x)| x.is_pure()) => { - let prop = &p.2.name; + let prop = &p.4.name; // Map literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); @@ -676,7 +676,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); // Search for overloaded operators (can override built-in). - if !state.engine.has_override_by_name_and_arguments(Some(&Default::default()), &mut Default::default(), state.lib, x.name.as_ref(), arg_types.as_ref()) { + if !state.engine.has_native_override(Some(&Default::default()), &mut Default::default(), state.lib, x.name.as_ref(), arg_types.as_ref()) { if let Some(result) = get_builtin_binary_op_fn(x.name.as_ref(), &arg_values[0], &arg_values[1]) .and_then(|f| { let ctx = (state.engine, x.name.as_ref(), state.lib).into(); diff --git a/src/parser.rs b/src/parser.rs index dd73136d..3361921b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,6 +1,9 @@ //! Main module defining the lexer and parser. -use crate::ast::{BinaryExpr, CustomExpr, Expr, FnCallExpr, Ident, ReturnType, ScriptFnDef, Stmt}; +use crate::ast::{ + BinaryExpr, CustomExpr, Expr, FnCallExpr, FnHash, Ident, OpAssignment, ReturnType, ScriptFnDef, + Stmt, +}; use crate::dynamic::{AccessMode, Union}; use crate::engine::KEYWORD_THIS; use crate::module::NamespaceRef; @@ -13,7 +16,7 @@ use crate::stdlib::{ format, hash::{Hash, Hasher}, iter::empty, - num::{NonZeroU64, NonZeroUsize}, + num::NonZeroUsize, string::{String, ToString}, vec, vec::Vec, @@ -22,8 +25,8 @@ use crate::syntax::{CustomSyntax, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; use crate::token::{is_keyword_function, is_valid_identifier, Token, TokenStream}; use crate::utils::{get_hasher, StraightHasherBuilder}; use crate::{ - calc_script_fn_hash, Dynamic, Engine, ImmutableString, LexError, ParseError, ParseErrorType, - Position, Scope, StaticVec, AST, + calc_fn_hash, Dynamic, Engine, ImmutableString, LexError, ParseError, ParseErrorType, Position, + Scope, StaticVec, AST, }; #[cfg(not(feature = "no_float"))] @@ -34,7 +37,7 @@ use crate::FnAccess; type PERR = ParseErrorType; -type FunctionsLib = HashMap; +type FunctionsLib = HashMap; /// A type that encapsulates the current state of the parser. #[derive(Debug)] @@ -236,8 +239,11 @@ impl Expr { Self::Variable(x) if x.1.is_none() => { let ident = x.2; let getter = state.get_interned_string(crate::engine::make_getter(&ident.name)); + let hash_get = calc_fn_hash(empty(), &getter, 1); let setter = state.get_interned_string(crate::engine::make_setter(&ident.name)); - Self::Property(Box::new((getter, setter, ident.into()))) + let hash_set = calc_fn_hash(empty(), &setter, 2); + + Self::Property(Box::new((getter, hash_get, setter, hash_set, ident.into()))) } _ => self, } @@ -334,33 +340,25 @@ fn parse_fn_call( Token::RightParen => { eat_token(input, Token::RightParen); - let mut hash_script = if let Some(ref mut modules) = namespace { + let hash = if let Some(ref mut modules) = namespace { #[cfg(not(feature = "no_module"))] modules.set_index(state.find_module(&modules[0].name)); - // 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, - // zero number of arguments, and the actual list of argument `TypeId`'s. - // 3) The final hash is the XOR of the two hashes. - let qualifiers = modules.iter().map(|m| m.name.as_str()); - calc_script_fn_hash(qualifiers, &id, 0) + calc_fn_hash(modules.iter().map(|m| m.name.as_str()), &id, 0) } else { - calc_script_fn_hash(empty(), &id, 0) + calc_fn_hash(empty(), &id, 0) }; - // script functions can only be valid identifiers - if !is_valid_identifier(id.chars()) { - hash_script = None; - } - return Ok(Expr::FnCall( Box::new(FnCallExpr { name: id.to_string().into(), capture, namespace, - hash_script, + hash: if is_valid_identifier(id.chars()) { + FnHash::from_script(hash) + } else { + FnHash::from_native(hash) + }, args, ..Default::default() }), @@ -385,33 +383,25 @@ fn parse_fn_call( (Token::RightParen, _) => { eat_token(input, Token::RightParen); - let mut hash_script = if let Some(modules) = namespace.as_mut() { + let hash = if let Some(modules) = namespace.as_mut() { #[cfg(not(feature = "no_module"))] modules.set_index(state.find_module(&modules[0].name)); - // 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, - // zero number of arguments, and the actual list of argument `TypeId`'s. - // 3) The final hash is the XOR of the two hashes. - let qualifiers = modules.iter().map(|m| m.name.as_str()); - calc_script_fn_hash(qualifiers, &id, args.len()) + calc_fn_hash(modules.iter().map(|m| m.name.as_str()), &id, args.len()) } else { - calc_script_fn_hash(empty(), &id, args.len()) + calc_fn_hash(empty(), &id, args.len()) }; - // script functions can only be valid identifiers - if !is_valid_identifier(id.chars()) { - hash_script = None; - } - return Ok(Expr::FnCall( Box::new(FnCallExpr { name: id.to_string().into(), capture, namespace, - hash_script, + hash: if is_valid_identifier(id.chars()) { + FnHash::from_script(hash) + } else { + FnHash::from_native(hash) + }, args, ..Default::default() }), @@ -1013,10 +1003,8 @@ fn parse_primary( state.access_var(closure, *pos); }); - lib.insert( - calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(), - func, - ); + let hash_script = calc_fn_hash(empty(), &func.name, func.params.len()); + lib.insert(hash_script, func); expr } @@ -1183,7 +1171,7 @@ fn parse_primary( } else { let mut ns: NamespaceRef = Default::default(); ns.push(var_name_def); - let index = NonZeroU64::new(42).unwrap(); // Dummy + let index = 42; // Dummy namespace = Some((index, ns)); } @@ -1243,8 +1231,7 @@ fn parse_primary( } .map(|x| match x.as_mut() { (_, Some((ref mut hash, ref mut namespace)), Ident { name, .. }) => { - *hash = - calc_script_fn_hash(namespace.iter().map(|v| v.name.as_str()), name, 0).unwrap(); + *hash = calc_fn_hash(namespace.iter().map(|v| v.name.as_str()), name, 0); #[cfg(not(feature = "no_module"))] namespace.set_index(state.find_module(&namespace[0].name)); @@ -1300,6 +1287,7 @@ fn parse_unary( Ok(Expr::FnCall( Box::new(FnCallExpr { name: op.into(), + hash: FnHash::from_native(calc_fn_hash(empty(), op, 1)), args, ..Default::default() }), @@ -1326,6 +1314,7 @@ fn parse_unary( Ok(Expr::FnCall( Box::new(FnCallExpr { name: op.into(), + hash: FnHash::from_native(calc_fn_hash(empty(), op, 1)), args, ..Default::default() }), @@ -1346,6 +1335,7 @@ fn parse_unary( Ok(Expr::FnCall( Box::new(FnCallExpr { name: op.into(), + hash: FnHash::from_native(calc_fn_hash(empty(), op, 1)), args, ..Default::default() }), @@ -1361,7 +1351,7 @@ fn parse_unary( /// Make an assignment statement. fn make_assignment_stmt<'a>( - fn_name: Cow<'static, str>, + op: Cow<'static, str>, state: &mut ParseState, lhs: Expr, rhs: Expr, @@ -1384,24 +1374,34 @@ fn make_assignment_stmt<'a>( } } + let op_info = if op.is_empty() { + None + } else { + let op2 = &op[..op.len() - 1]; // extract operator without = + + Some(OpAssignment { + hash_op_assign: calc_fn_hash(empty(), &op, 2), + hash_op: calc_fn_hash(empty(), op2, 2), + op, + }) + }; + match &lhs { // const_expr = rhs expr if expr.is_constant() => { Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) } // var (non-indexed) = rhs - Expr::Variable(x) if x.0.is_none() => Ok(Stmt::Assignment( - Box::new((lhs, fn_name.into(), rhs)), - op_pos, - )), + Expr::Variable(x) if x.0.is_none() => { + Ok(Stmt::Assignment(Box::new((lhs, rhs, op_info)), op_pos)) + } // var (indexed) = rhs Expr::Variable(x) => { let (index, _, Ident { name, pos }) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { - AccessMode::ReadWrite => Ok(Stmt::Assignment( - Box::new((lhs, fn_name.into(), rhs)), - op_pos, - )), + AccessMode::ReadWrite => { + Ok(Stmt::Assignment(Box::new((lhs, rhs, op_info)), op_pos)) + } // Constant values cannot be assigned to AccessMode::ReadOnly => { Err(PERR::AssignmentToConstant(name.to_string()).into_err(*pos)) @@ -1413,18 +1413,16 @@ fn make_assignment_stmt<'a>( match check_lvalue(&x.rhs, matches!(lhs, Expr::Dot(_, _))) { Position::NONE => match &x.lhs { // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs - Expr::Variable(x) if x.0.is_none() => Ok(Stmt::Assignment( - Box::new((lhs, fn_name.into(), rhs)), - op_pos, - )), + Expr::Variable(x) if x.0.is_none() => { + Ok(Stmt::Assignment(Box::new((lhs, rhs, op_info)), op_pos)) + } // var[???] (indexed) = rhs, var.??? (indexed) = rhs Expr::Variable(x) => { let (index, _, Ident { name, pos }) = x.as_ref(); match state.stack[(state.stack.len() - index.unwrap().get())].1 { - AccessMode::ReadWrite => Ok(Stmt::Assignment( - Box::new((lhs, fn_name.into(), rhs)), - op_pos, - )), + AccessMode::ReadWrite => { + Ok(Stmt::Assignment(Box::new((lhs, rhs, op_info)), op_pos)) + } // Constant values cannot be assigned to AccessMode::ReadOnly => { Err(PERR::AssignmentToConstant(name.to_string()).into_err(*pos)) @@ -1506,8 +1504,11 @@ fn make_dot_expr( (lhs, Expr::Variable(x)) if x.1.is_none() => { let ident = x.2; let getter = state.get_interned_string(crate::engine::make_getter(&ident.name)); + let hash_get = calc_fn_hash(empty(), &getter, 1); let setter = state.get_interned_string(crate::engine::make_setter(&ident.name)); - let rhs = Expr::Property(Box::new((getter, setter, ident))); + let hash_set = calc_fn_hash(empty(), &setter, 2); + + let rhs = Expr::Property(Box::new((getter, hash_get, setter, hash_set, ident))); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } @@ -1521,7 +1522,7 @@ fn make_dot_expr( } // lhs.dot_lhs.dot_rhs (lhs, Expr::Dot(x, pos)) => match x.lhs { - Expr::Variable(_) | Expr::Property(_) | Expr::FnCall(_, _) => { + Expr::Variable(_) | Expr::Property(_) => { let rhs = Expr::Dot( Box::new(BinaryExpr { lhs: x.lhs.into_property(state), @@ -1531,6 +1532,22 @@ fn make_dot_expr( ); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } + Expr::FnCall(mut func, func_pos) => { + // Recalculate hash + func.hash = FnHash::from_script_and_native( + calc_fn_hash(empty(), &func.name, func.args.len()), + calc_fn_hash(empty(), &func.name, func.args.len() + 1), + ); + + let rhs = Expr::Dot( + Box::new(BinaryExpr { + lhs: Expr::FnCall(func, func_pos), + rhs: x.rhs, + }), + pos, + ); + Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) + } _ => unreachable!("invalid dot expression: {:?}", x.lhs), }, // lhs.idx_lhs[idx_rhs] @@ -1544,6 +1561,10 @@ fn make_dot_expr( ); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } + // lhs.nnn::func(...) + (_, Expr::FnCall(x, _)) if x.namespace.is_some() => { + unreachable!("method call should not be namespace-qualified") + } // lhs.Fn() or lhs.eval() (_, Expr::FnCall(x, pos)) if x.args.len() == 0 @@ -1567,8 +1588,14 @@ fn make_dot_expr( .into_err(pos)) } // lhs.func(...) - (lhs, func @ Expr::FnCall(_, _)) => { - Expr::Dot(Box::new(BinaryExpr { lhs, rhs: func }), op_pos) + (lhs, Expr::FnCall(mut func, func_pos)) => { + // Recalculate hash + func.hash = FnHash::from_script_and_native( + calc_fn_hash(empty(), &func.name, func.args.len()), + calc_fn_hash(empty(), &func.name, func.args.len() + 1), + ); + let rhs = Expr::FnCall(func, func_pos); + Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } // lhs.rhs (_, rhs) => return Err(PERR::PropertyExpected.into_err(rhs.position())), @@ -1792,9 +1819,11 @@ fn parse_binary_op( settings.ensure_level_within_max_limit(state.max_expr_depth)?; let op = op_token.syntax(); + let hash = calc_fn_hash(empty(), &op, 2); let op_base = FnCallExpr { name: op, + hash: FnHash::from_native(hash), capture: false, ..Default::default() }; @@ -1863,16 +1892,15 @@ fn parse_binary_op( .get(&s) .map_or(false, Option::is_some) => { - let hash_script = if is_valid_identifier(s.chars()) { - // Accept non-native functions for custom operators - calc_script_fn_hash(empty(), &s, 2) - } else { - None - }; + let hash = calc_fn_hash(empty(), &s, 2); Expr::FnCall( Box::new(FnCallExpr { - hash_script, + hash: if is_valid_identifier(s.chars()) { + FnHash::from_script(hash) + } else { + FnHash::from_native(hash) + }, args, ..op_base }), @@ -2604,7 +2632,7 @@ fn parse_stmt( }; let func = parse_fn(input, &mut new_state, lib, access, settings, _comments)?; - let hash = calc_script_fn_hash(empty(), &func.name, func.params.len()).unwrap(); + let hash = calc_fn_hash(empty(), &func.name, func.params.len()); if lib.contains_key(&hash) { return Err(PERR::FnDuplicatedDefinition( @@ -2871,12 +2899,10 @@ fn make_curry_from_externals(fn_expr: Expr, externals: StaticVec, pos: Po let curry_func = crate::engine::KEYWORD_FN_PTR_CURRY; - let hash_script = calc_script_fn_hash(empty(), curry_func, num_externals + 1); - let expr = Expr::FnCall( Box::new(FnCallExpr { name: curry_func.into(), - hash_script, + hash: FnHash::from_native(calc_fn_hash(empty(), curry_func, num_externals + 1)), args, ..Default::default() }), diff --git a/src/utils.rs b/src/utils.rs index b84ea2e3..a51501e9 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -10,8 +10,7 @@ use crate::stdlib::{ fmt, fmt::{Debug, Display}, hash::{BuildHasher, Hash, Hasher}, - iter::{empty, FromIterator}, - num::NonZeroU64, + iter::FromIterator, ops::{Add, AddAssign, Deref, DerefMut, Sub, SubAssign}, str::FromStr, string::{String, ToString}, @@ -19,18 +18,18 @@ use crate::stdlib::{ }; use crate::Shared; -/// A hasher that only takes one single [`NonZeroU64`] and returns it as a hash key. +/// A hasher that only takes one single [`u64`] and returns it as a hash key. /// /// # Panics /// -/// Panics when hashing any data type other than a [`NonZeroU64`]. +/// Panics when hashing any data type other than a [`u64`]. #[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub struct StraightHasher(NonZeroU64); +pub struct StraightHasher(u64); impl Hasher for StraightHasher { #[inline(always)] fn finish(&self) -> u64 { - self.0.get() + self.0 } #[inline(always)] fn write(&mut self, bytes: &[u8]) { @@ -39,9 +38,7 @@ impl Hasher for StraightHasher { let mut key = [0_u8; 8]; key.copy_from_slice(bytes); - // HACK - If it so happens to hash directly to zero (OMG!) then change it to 42... - self.0 = NonZeroU64::new(u64::from_ne_bytes(key)) - .unwrap_or_else(|| NonZeroU64::new(42).unwrap()); + self.0 = u64::from_ne_bytes(key); } } @@ -54,7 +51,7 @@ impl BuildHasher for StraightHasherBuilder { #[inline(always)] fn build_hasher(&self) -> Self::Hasher { - StraightHasher(NonZeroU64::new(42).unwrap()) + StraightHasher(42) } } @@ -64,26 +61,7 @@ pub fn get_hasher() -> ahash::AHasher { Default::default() } -/// _(INTERNALS)_ Calculate a [`NonZeroU64`] hash key from a namespace-qualified function name and -/// parameter types. -/// Exported under the `internals` feature only. -/// -/// Module names are passed in via `&str` references from an iterator. -/// Parameter types are passed in via [`TypeId`] values from an iterator. -/// -/// # Note -/// -/// The first module name is skipped. Hashing starts from the _second_ module in the chain. -#[inline(always)] -pub fn calc_native_fn_hash<'a>( - modules: impl Iterator, - fn_name: &str, - params: impl Iterator, -) -> Option { - calc_fn_hash(modules, fn_name, None, params) -} - -/// _(INTERNALS)_ Calculate a [`NonZeroU64`] hash key from a namespace-qualified function name +/// _(INTERNALS)_ Calculate a [`u64`] hash key from a namespace-qualified function name /// and the number of parameters, but no parameter types. /// Exported under the `internals` feature only. /// @@ -94,29 +72,11 @@ pub fn calc_native_fn_hash<'a>( /// /// The first module name is skipped. Hashing starts from the _second_ module in the chain. #[inline(always)] -pub fn calc_script_fn_hash<'a>( - modules: impl Iterator, - fn_name: &str, - num: usize, -) -> Option { - calc_fn_hash(modules, fn_name, Some(num), empty()) -} - -/// Calculate a [`NonZeroU64`] hash key from a namespace-qualified function name and parameter types. -/// -/// Module names are passed in via `&str` references from an iterator. -/// Parameter types are passed in via [`TypeId`] values from an iterator. -/// -/// # Note -/// -/// The first module name is skipped. Hashing starts from the _second_ module in the chain. -#[inline(always)] -fn calc_fn_hash<'a>( +pub fn calc_fn_hash<'a>( mut modules: impl Iterator, fn_name: &str, - num: Option, - params: impl Iterator, -) -> Option { + num: usize, +) -> u64 { let s = &mut get_hasher(); // Hash a boolean indicating whether the hash is namespace-qualified. @@ -124,20 +84,30 @@ fn calc_fn_hash<'a>( // We always skip the first module modules.for_each(|m| m.hash(s)); fn_name.hash(s); - if let Some(num) = num { - num.hash(s); - } else { - params.for_each(|t| t.hash(s)); - } - // HACK - If it so happens to hash directly to zero (OMG!) then change it to 42... - NonZeroU64::new(s.finish()).or_else(|| NonZeroU64::new(42)) + num.hash(s); + s.finish() } -/// Combine two [`NonZeroU64`] hashes by taking the XOR of them. +/// _(INTERNALS)_ Calculate a [`u64`] hash key from a list of parameter types. +/// Exported under the `internals` feature only. +/// +/// Parameter types are passed in via [`TypeId`] values from an iterator. #[inline(always)] -pub(crate) fn combine_hashes(a: NonZeroU64, b: NonZeroU64) -> NonZeroU64 { - // HACK - If it so happens to hash directly to zero (OMG!) then change it to 42... - NonZeroU64::new(a.get() ^ b.get()).unwrap_or_else(|| NonZeroU64::new(42).unwrap()) +pub fn calc_fn_params_hash(params: impl Iterator) -> u64 { + let s = &mut get_hasher(); + let mut len = 0; + params.for_each(|t| { + t.hash(s); + len += 1; + }); + len.hash(s); + s.finish() +} + +/// Combine two [`u64`] hashes by taking the XOR of them. +#[inline(always)] +pub(crate) fn combine_hashes(a: u64, b: u64) -> u64 { + a ^ b } /// _(INTERNALS)_ A type that wraps a [`HashMap`] and implements [`Hash`]. diff --git a/tests/bit_shift.rs b/tests/bit_shift.rs index 0b888e32..fb318e4d 100644 --- a/tests/bit_shift.rs +++ b/tests/bit_shift.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, INT}; + use rhai::{Engine, EvalAltResult, INT}; #[test] fn test_left_shift() -> Result<(), Box> { diff --git a/tests/string.rs b/tests/string.rs index 0db10eb0..2078b90c 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -68,12 +68,12 @@ fn test_string_dynamic() -> Result<(), Box> { fn test_string_mut() -> Result<(), Box> { let mut engine = Engine::new(); - engine.register_fn("foo", |x: INT, s: &str| s.len() as INT + x); - engine.register_fn("bar", |x: INT, s: String| s.len() as INT + x); + engine.register_fn("foo", |s: &str| s.len() as INT); + engine.register_fn("bar", |s: String| s.len() as INT); engine.register_fn("baz", |s: &mut String| s.len()); - assert_eq!(engine.eval::(r#"foo(1, "hello")"#)?, 6); - assert_eq!(engine.eval::(r#"bar(1, "hello")"#)?, 6); + assert_eq!(engine.eval::(r#"foo("hello")"#)?, 5); + assert_eq!(engine.eval::(r#"bar("hello")"#)?, 5); assert!( matches!(*engine.eval::(r#"baz("hello")"#).expect_err("should error"), EvalAltResult::ErrorFunctionNotFound(f, _) if f == "baz (&str | ImmutableString | String)" From 57140cbeebcf43bdb084e8b728dd33eadf0d2c08 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 8 Mar 2021 15:55:26 +0800 Subject: [PATCH 10/11] Fix internals build. --- src/ast.rs | 4 ++-- src/lib.rs | 4 ---- tests/bit_shift.rs | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index e586caeb..81b77671 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1168,14 +1168,14 @@ pub struct FnCallExpr { pub hash: FnHash, /// Does this function call capture the parent scope? pub capture: bool, + /// List of function call arguments. + pub args: StaticVec, /// Namespace of the function, if any. Boxed because it occurs rarely. pub namespace: Option, /// Function name. /// Use [`Cow<'static, str>`][Cow] because a lot of operators (e.g. `==`, `>=`) are implemented as /// function calls and the function names are predictable, so no need to allocate a new [`String`]. pub name: Cow<'static, str>, - /// List of function call arguments. - pub args: StaticVec, } /// A type that wraps a [`FLOAT`] and implements [`Hash`]. diff --git a/src/lib.rs b/src/lib.rs index 1ed55ca3..aa95de62 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -141,10 +141,6 @@ pub use fn_native::Shared; #[cfg(not(feature = "no_closure"))] use fn_native::Locked; -#[cfg(feature = "internals")] -pub use utils::{calc_fn_hash, calc_fn_params_hash, combine_hashes, HashableHashMap}; - -#[cfg(not(feature = "internals"))] pub(crate) use utils::{calc_fn_hash, calc_fn_params_hash, combine_hashes}; pub use rhai_codegen::*; diff --git a/tests/bit_shift.rs b/tests/bit_shift.rs index fb318e4d..0b888e32 100644 --- a/tests/bit_shift.rs +++ b/tests/bit_shift.rs @@ -1,4 +1,4 @@ - use rhai::{Engine, EvalAltResult, INT}; +use rhai::{Engine, EvalAltResult, INT}; #[test] fn test_left_shift() -> Result<(), Box> { From fefa5a7dc725a27702fef4e77ae794c0ec2eaeba Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 8 Mar 2021 18:40:23 +0800 Subject: [PATCH 11/11] Split has_script_fn and has_native_fn. --- src/fn_call.rs | 64 +++++++++++++------------------------------------ src/optimize.rs | 28 ++++++++++++++++------ 2 files changed, 38 insertions(+), 54 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index b0d43420..63fe6079 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -577,67 +577,37 @@ impl Engine { result } - // Has a system function a Rust-native override? + // Does a scripted function exist? #[inline(always)] - pub(crate) fn has_native_override( + pub(crate) fn has_script_fn( &self, mods: Option<&Imports>, state: &mut State, lib: &[&Module], - fn_name: &str, - arg_types: &[TypeId], - ) -> bool { - let hash_script = calc_fn_hash(empty(), fn_name, arg_types.len()); - let hash_params = calc_fn_params_hash(arg_types.iter().cloned()); - let hash_fn = combine_hashes(hash_script, hash_params); - - self.has_override(mods, state, lib, Some(hash_fn), None) - } - - // Has a system function an override? - #[inline(always)] - pub(crate) fn has_override( - &self, - mods: Option<&Imports>, - state: &mut State, - lib: &[&Module], - hash_fn: Option, - hash_script: Option, + hash_script: u64, ) -> bool { let cache = state.fn_resolution_cache_mut(); - if hash_script.map_or(false, |hash| cache.contains_key(&hash)) - || hash_fn.map_or(false, |hash| cache.contains_key(&hash)) - { - return true; + if let Some(result) = cache.get(&hash_script).map(|v| v.is_some()) { + return result; } // First check script-defined functions - if hash_script.map_or(false, |hash| lib.iter().any(|&m| m.contains_fn(hash, false))) - //|| hash_fn.map_or(false, |hash| lib.iter().any(|&m| m.contains_fn(hash, false))) + let result = lib.iter().any(|&m| m.contains_fn(hash_script, false)) // Then check registered functions - || hash_script.map_or(false, |hash| self.global_namespace.contains_fn(hash, false)) - || hash_fn.map_or(false, |hash| self.global_namespace.contains_fn(hash, false)) + || self.global_namespace.contains_fn(hash_script, false) // Then check packages - || hash_script.map_or(false, |hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))) - || hash_fn.map_or(false, |hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))) + || self.global_modules.iter().any(|m| m.contains_fn(hash_script, false)) // Then check imported modules - || hash_script.map_or(false, |hash| mods.map_or(false, |m| m.contains_fn(hash))) - || hash_fn.map_or(false, |hash| mods.map_or(false, |m| m.contains_fn(hash))) + || mods.map_or(false, |m| m.contains_fn(hash_script)) // Then check sub-modules - || hash_script.map_or(false, |hash| self.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash))) - || hash_fn.map_or(false, |hash| self.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash))) - { - true - } else { - if let Some(hash_fn) = hash_fn { - cache.insert(hash_fn, None); - } - if let Some(hash_script) = hash_script { - cache.insert(hash_script, None); - } - false + || self.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash_script)); + + if !result { + cache.insert(hash_script, None); } + + result } /// Perform an actual function call, native Rust or scripted, taking care of special functions. @@ -689,7 +659,7 @@ impl Engine { Dynamic::FALSE } else { let hash_script = calc_fn_hash(empty(), &fn_name, num_params as usize); - self.has_override(Some(mods), state, lib, None, Some(hash_script)) + self.has_script_fn(Some(mods), state, lib, hash_script) .into() }, false, @@ -1165,7 +1135,7 @@ impl Engine { Dynamic::FALSE } else { let hash_script = calc_fn_hash(empty(), &fn_name, num_params as usize); - self.has_override(Some(mods), state, lib, None, Some(hash_script)) + self.has_script_fn(Some(mods), state, lib, hash_script) .into() }); } diff --git a/src/optimize.rs b/src/optimize.rs index 653c05f0..20456ad2 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -6,6 +6,7 @@ use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF} use crate::fn_builtin::get_builtin_binary_op_fn; use crate::parser::map_dynamic_to_expr; use crate::stdlib::{ + any::TypeId, boxed::Box, hash::{Hash, Hasher}, iter::empty, @@ -15,8 +16,11 @@ use crate::stdlib::{ vec::Vec, }; use crate::token::is_valid_identifier; -use crate::utils::{calc_fn_hash, get_hasher}; -use crate::{Dynamic, Engine, Module, Position, Scope, StaticVec, AST}; +use crate::utils::get_hasher; +use crate::{ + calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, Module, Position, Scope, + StaticVec, AST, +}; /// Level of optimization performed. #[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] @@ -126,15 +130,25 @@ impl<'a> State<'a> { } } +// Has a system function a Rust-native override? +fn has_native_fn(state: &State, hash_script: u64, arg_types: &[TypeId]) -> bool { + let hash_params = calc_fn_params_hash(arg_types.iter().cloned()); + let hash = combine_hashes(hash_script, hash_params); + + // First check registered functions + state.engine.global_namespace.contains_fn(hash, false) + // Then check packages + || state.engine.global_modules.iter().any(|m| m.contains_fn(hash, false)) + // Then check sub-modules + || state.engine.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash)) +} + /// Call a registered function fn call_fn_with_constant_arguments( state: &State, fn_name: &str, arg_values: &mut [Dynamic], ) -> Option { - // Search built-in's and external functions - let hash_native = calc_fn_hash(empty(), fn_name, arg_values.len()); - state .engine .call_native_fn( @@ -142,7 +156,7 @@ fn call_fn_with_constant_arguments( &mut Default::default(), state.lib, fn_name, - hash_native, + calc_fn_hash(empty(), fn_name, arg_values.len()), arg_values.iter_mut().collect::>().as_mut(), false, false, @@ -676,7 +690,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); // Search for overloaded operators (can override built-in). - if !state.engine.has_native_override(Some(&Default::default()), &mut Default::default(), state.lib, x.name.as_ref(), arg_types.as_ref()) { + if !has_native_fn(state, x.hash.native_hash(), arg_types.as_ref()) { if let Some(result) = get_builtin_binary_op_fn(x.name.as_ref(), &arg_values[0], &arg_values[1]) .and_then(|f| { let ctx = (state.engine, x.name.as_ref(), state.lib).into();