diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 98309937..12cfee33 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -3,6 +3,7 @@ on: push: branches: - master + - perf jobs: benchmark: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d3cf23b..136c1fd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ Rhai Release Notes ================== +Version 1.14.0 +============== + +The code hacks that attempt to optimize branch prediction performance are removed because benchmarks do not show any material speed improvements. + +New features +------------ + +* It is now possible to require a specific _type_ to the `this` pointer for a particular script-defined function so that it is called only when the `this` pointer contains the specified type. + + Version 1.13.0 ============== diff --git a/Cargo.toml b/Cargo.toml index 9730399c..503903db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = [".", "codegen"] [package] name = "rhai" -version = "1.13.0" +version = "1.14.0" rust-version = "1.61.0" edition = "2018" resolver = "2" diff --git a/src/README.md b/src/README.md index a22e5cfe..fc1d7bbf 100644 --- a/src/README.md +++ b/src/README.md @@ -28,5 +28,4 @@ Sub-Directories | `func` | Support for function calls | | `eval` | Evaluation engine | | `serde` | Support for [`serde`](https://crates.io/crates/serde) | -| `tools` | External tools needed for building | | `bin` | Pre-built CLI binaries (e.g. `rhai-run`, `rhai-repl`) | diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 6add6903..2f96af59 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -620,7 +620,7 @@ impl Expr { Self::Index(_, options, _) | Self::Dot(_, options, _) => *options, #[cfg(not(feature = "no_float"))] - Self::FloatConstant(..) => ASTFlags::NONE, + Self::FloatConstant(..) => ASTFlags::empty(), Self::DynamicConstant(..) | Self::BoolConstant(..) @@ -638,10 +638,10 @@ impl Expr { | Self::MethodCall(..) | Self::InterpolatedString(..) | Self::Property(..) - | Self::Stmt(..) => ASTFlags::NONE, + | Self::Stmt(..) => ASTFlags::empty(), #[cfg(not(feature = "no_custom_syntax"))] - Self::Custom(..) => ASTFlags::NONE, + Self::Custom(..) => ASTFlags::empty(), } } /// Get the [position][Position] of the expression. diff --git a/src/ast/flags.rs b/src/ast/flags.rs index 7085ee54..f58eae68 100644 --- a/src/ast/flags.rs +++ b/src/ast/flags.rs @@ -41,8 +41,6 @@ bitflags! { /// _(internals)_ Bit-flags containing [`AST`][crate::AST] node configuration options. /// Exported under the `internals` feature only. pub struct ASTFlags: u8 { - /// No options for the [`AST`][crate::AST] node. - const NONE = 0b_0000_0000; /// The [`AST`][crate::AST] node is read-only. const CONSTANT = 0b_0000_0001; /// The [`AST`][crate::AST] node is exposed to the outside (i.e. public). diff --git a/src/ast/script_fn.rs b/src/ast/script_fn.rs index 7126fdda..7120dbb3 100644 --- a/src/ast/script_fn.rs +++ b/src/ast/script_fn.rs @@ -17,6 +17,10 @@ pub struct ScriptFnDef { pub name: ImmutableString, /// Function access mode. pub access: FnAccess, + #[cfg(not(feature = "no_object"))] + /// Type of `this` pointer, if any. + /// Not available under `no_object`. + pub this_type: Option, /// Names of function parameters. pub params: FnArgsVec, /// _(metadata)_ Function doc-comments (if any). @@ -39,13 +43,23 @@ pub struct ScriptFnDef { impl fmt::Display for ScriptFnDef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + #[cfg(not(feature = "no_object"))] + let this_type = self + .this_type + .as_ref() + .map_or(String::new(), |s| format!("{:?}.", s)); + + #[cfg(feature = "no_object")] + let this_type = ""; + write!( f, - "{}{}({})", + "{}{}{}({})", match self.access { FnAccess::Public => "", FnAccess::Private => "private ", }, + this_type, self.name, self.params .iter() @@ -70,6 +84,10 @@ pub struct ScriptFnMetadata<'a> { pub params: Vec<&'a str>, /// Function access mode. pub access: FnAccess, + #[cfg(not(feature = "no_object"))] + /// Type of `this` pointer, if any. + /// Not available under `no_object`. + pub this_type: Option<&'a str>, /// _(metadata)_ Function doc-comments (if any). /// Exported under the `metadata` feature only. /// @@ -90,13 +108,23 @@ pub struct ScriptFnMetadata<'a> { impl fmt::Display for ScriptFnMetadata<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + #[cfg(not(feature = "no_object"))] + let this_type = self + .this_type + .as_ref() + .map_or(String::new(), |s| format!("{:?}.", s)); + + #[cfg(feature = "no_object")] + let this_type = ""; + write!( f, - "{}{}({})", + "{}{}{}({})", match self.access { FnAccess::Public => "", FnAccess::Private => "private ", }, + this_type, self.name, self.params .iter() @@ -114,6 +142,8 @@ impl<'a> From<&'a ScriptFnDef> for ScriptFnMetadata<'a> { name: &value.name, params: value.params.iter().map(|s| s.as_str()).collect(), access: value.access, + #[cfg(not(feature = "no_object"))] + this_type: value.this_type.as_ref().map(|s| s.as_str()), #[cfg(feature = "metadata")] comments: value.comments.iter().map(<_>::as_ref).collect(), } diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 9c67bcab..8487c4fb 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -779,13 +779,13 @@ impl Stmt { | Self::While(..) | Self::For(..) | Self::TryCatch(..) - | Self::Assignment(..) => ASTFlags::NONE, + | Self::Assignment(..) => ASTFlags::empty(), #[cfg(not(feature = "no_module"))] - Self::Import(..) | Self::Export(..) => ASTFlags::NONE, + Self::Import(..) | Self::Export(..) => ASTFlags::empty(), #[cfg(not(feature = "no_closure"))] - Self::Share(..) => ASTFlags::NONE, + Self::Share(..) => ASTFlags::empty(), } } /// Get the [position][Position] of this statement. diff --git a/src/engine.rs b/src/engine.rs index e4068fc7..1e35d21f 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -367,17 +367,4 @@ impl Engine { pub(crate) const fn is_debugger_registered(&self) -> bool { self.debugger_interface.is_some() } - - /// Imitation of std::hints::black_box which requires nightly. - #[cfg(not(target_family = "wasm"))] - #[inline(never)] - pub(crate) fn black_box() -> usize { - unsafe { core::ptr::read_volatile(&0_usize as *const usize) } - } - /// Imitation of std::hints::black_box which requires nightly. - #[cfg(target_family = "wasm")] - #[inline(always)] - pub(crate) fn black_box() -> usize { - 0 - } } diff --git a/src/eval/expr.rs b/src/eval/expr.rs index f73bd496..540eb7a7 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -247,37 +247,9 @@ impl Engine { self.track_operation(global, expr.position())?; - // Function calls should account for a relatively larger portion of expressions because - // binary operators are also function calls. - if let Expr::FnCall(x, pos) = expr { - return self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos); - } - - // Then variable access. - if let Expr::Variable(x, index, var_pos) = expr { - return if index.is_none() && x.0.is_none() && x.3 == KEYWORD_THIS { - this_ptr - .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into()) - .cloned() - } else { - self.search_namespace(global, caches, scope, this_ptr, expr) - .map(Target::take_or_clone) - }; - } - - // Then integer constants. - if let Expr::IntegerConstant(x, ..) = expr { - return Ok((*x).into()); - } - - // Stop merging branches here! - // We shouldn't lift out too many variants because, soon or later, the added comparisons - // will cost more than the mis-predicted `match` branch. - Self::black_box(); - match expr { // Constants - Expr::IntegerConstant(..) => unreachable!(), + Expr::IntegerConstant(x, ..) => Ok((*x).into()), Expr::StringConstant(x, ..) => Ok(x.clone().into()), Expr::BoolConstant(x, ..) => Ok((*x).into()), #[cfg(not(feature = "no_float"))] @@ -286,7 +258,21 @@ impl Engine { Expr::Unit(..) => Ok(Dynamic::UNIT), Expr::DynamicConstant(x, ..) => Ok(x.as_ref().clone()), - // `... ${...} ...` + Expr::FnCall(x, pos) => { + self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos) + } + + Expr::Variable(x, index, var_pos) => { + if index.is_none() && x.0.is_none() && x.3 == KEYWORD_THIS { + this_ptr + .ok_or_else(|| ERR::ErrorUnboundThis(*var_pos).into()) + .cloned() + } else { + self.search_namespace(global, caches, scope, this_ptr, expr) + .map(Target::take_or_clone) + } + } + Expr::InterpolatedString(x, _) => { let mut concat = SmartString::new_const(); @@ -445,6 +431,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] Expr::Dot(..) => self.eval_dot_index_chain(global, caches, scope, this_ptr, expr, None), + #[allow(unreachable_patterns)] _ => unreachable!("expression cannot be evaluated: {:?}", expr), } } diff --git a/src/eval/global_state.rs b/src/eval/global_state.rs index caaf210f..6f253f63 100644 --- a/src/eval/global_state.rs +++ b/src/eval/global_state.rs @@ -251,12 +251,22 @@ impl GlobalRuntimeState { pub fn get_qualified_fn( &self, hash: u64, + global_namespace_only: bool, ) -> Option<(&crate::func::CallableFunction, Option<&ImmutableString>)> { - self.modules.as_ref().and_then(|m| { - m.iter() - .rev() - .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw()))) - }) + if global_namespace_only { + self.modules.as_ref().and_then(|m| { + m.iter() + .rev() + .filter(|m| m.contains_indexed_global_functions()) + .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw()))) + }) + } else { + self.modules.as_ref().and_then(|m| { + m.iter() + .rev() + .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw()))) + }) + } } /// Does the specified [`TypeId`][std::any::TypeId] iterator exist in the stack of /// globally-imported [modules][crate::Module]? diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 799846b3..2410d036 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -275,173 +275,6 @@ impl Engine { self.track_operation(global, stmt.position())?; - // Coded this way for better branch prediction. - // Popular branches are lifted out of the `match` statement into their own branches. - - // Function calls should account for a relatively larger portion of statements. - if let Stmt::FnCall(x, pos) = stmt { - return self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos); - } - - // Then assignments. - if let Stmt::Assignment(x, ..) = stmt { - let (op_info, BinaryExpr { lhs, rhs }) = &**x; - - if let Expr::Variable(x, ..) = lhs { - let rhs_val = self - .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), rhs)? - .flatten(); - - let mut target = self.search_namespace(global, caches, scope, this_ptr, lhs)?; - - let is_temp_result = !target.is_ref(); - let var_name = x.3.as_str(); - - #[cfg(not(feature = "no_closure"))] - // Also handle case where target is a `Dynamic` shared value - // (returned by a variable resolver, for example) - let is_temp_result = is_temp_result && !target.is_shared(); - - // Cannot assign to temp result from expression - if is_temp_result { - return Err(ERR::ErrorAssignmentToConstant( - var_name.to_string(), - lhs.position(), - ) - .into()); - } - - self.track_operation(global, lhs.position())?; - - self.eval_op_assignment(global, caches, op_info, lhs, &mut target, rhs_val)?; - - return Ok(Dynamic::UNIT); - } - - #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] - { - let rhs_val = self - .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), rhs)? - .flatten() - .intern_string(self); - - let _new_val = Some((rhs_val, op_info)); - - // Must be either `var[index] op= val` or `var.prop op= val`. - // The return value of any op-assignment (should be `()`) is thrown away and not used. - let _ = - match lhs { - // name op= rhs (handled above) - Expr::Variable(..) => { - unreachable!("Expr::Variable case is already handled") - } - // idx_lhs[idx_expr] op= rhs - #[cfg(not(feature = "no_index"))] - Expr::Index(..) => self - .eval_dot_index_chain(global, caches, scope, this_ptr, lhs, _new_val), - // dot_lhs.dot_rhs op= rhs - #[cfg(not(feature = "no_object"))] - Expr::Dot(..) => self - .eval_dot_index_chain(global, caches, scope, this_ptr, lhs, _new_val), - _ => unreachable!("cannot assign to expression: {:?}", lhs), - }?; - - return Ok(Dynamic::UNIT); - } - } - - // Then variable definitions. - if let Stmt::Var(x, options, pos) = stmt { - if !self.allow_shadowing() && scope.contains(&x.0) { - return Err(ERR::ErrorVariableExists(x.0.to_string(), *pos).into()); - } - - // Let/const statement - let (var_name, expr, index) = &**x; - - let access = if options.contains(ASTFlags::CONSTANT) { - AccessMode::ReadOnly - } else { - AccessMode::ReadWrite - }; - let export = options.contains(ASTFlags::EXPORTED); - - // Check variable definition filter - if let Some(ref filter) = self.def_var_filter { - let will_shadow = scope.contains(var_name); - let is_const = access == AccessMode::ReadOnly; - let info = VarDefInfo { - name: var_name, - is_const, - nesting_level: global.scope_level, - will_shadow, - }; - let orig_scope_len = scope.len(); - let context = - EvalContext::new(self, global, caches, scope, this_ptr.as_deref_mut()); - let filter_result = filter(true, info, context); - - if orig_scope_len != scope.len() { - // The scope is changed, always search from now on - global.always_search_scope = true; - } - - if !filter_result? { - return Err(ERR::ErrorForbiddenVariable(var_name.to_string(), *pos).into()); - } - } - - // Evaluate initial value - let mut value = self - .eval_expr(global, caches, scope, this_ptr, expr)? - .flatten() - .intern_string(self); - - let _alias = if !rewind_scope { - // Put global constants into global module - #[cfg(not(feature = "no_function"))] - #[cfg(not(feature = "no_module"))] - if global.scope_level == 0 - && access == AccessMode::ReadOnly - && global.lib.iter().any(|m| !m.is_empty()) - { - crate::func::locked_write(global.constants.get_or_insert_with(|| { - crate::Shared::new(crate::Locked::new(std::collections::BTreeMap::new())) - })) - .insert(var_name.name.clone(), value.clone()); - } - - if export { - Some(var_name) - } else { - None - } - } else if export { - unreachable!("exported variable not on global level"); - } else { - None - }; - - if let Some(index) = index { - value.set_access_mode(access); - *scope.get_mut_by_index(scope.len() - index.get()) = value; - } else { - scope.push_entry(var_name.name.clone(), access, value); - } - - #[cfg(not(feature = "no_module"))] - if let Some(alias) = _alias { - scope.add_alias_by_index(scope.len() - 1, alias.as_str().into()); - } - - return Ok(Dynamic::UNIT); - } - - // Stop merging branches here! - // We shouldn't lift out too many variants because, soon or later, the added comparisons - // will cost more than the mis-predicted `match` branch. - Self::black_box(); - match stmt { // No-op Stmt::Noop(..) => Ok(Dynamic::UNIT), @@ -460,6 +293,166 @@ impl Engine { } } + // Function call + Stmt::FnCall(x, pos) => { + self.eval_fn_call_expr(global, caches, scope, this_ptr, x, *pos) + } + + // Assignment + Stmt::Assignment(x, ..) => { + let (op_info, BinaryExpr { lhs, rhs }) = &**x; + + if let Expr::Variable(x, ..) = lhs { + let rhs_val = self + .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), rhs)? + .flatten(); + + let mut target = self.search_namespace(global, caches, scope, this_ptr, lhs)?; + + let is_temp_result = !target.is_ref(); + let var_name = x.3.as_str(); + + #[cfg(not(feature = "no_closure"))] + // Also handle case where target is a `Dynamic` shared value + // (returned by a variable resolver, for example) + let is_temp_result = is_temp_result && !target.is_shared(); + + // Cannot assign to temp result from expression + if is_temp_result { + return Err(ERR::ErrorAssignmentToConstant( + var_name.to_string(), + lhs.position(), + ) + .into()); + } + + self.track_operation(global, lhs.position())?; + + self.eval_op_assignment(global, caches, op_info, lhs, &mut target, rhs_val)?; + } else { + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] + { + let rhs_val = self + .eval_expr(global, caches, scope, this_ptr.as_deref_mut(), rhs)? + .flatten() + .intern_string(self); + + let _new_val = Some((rhs_val, op_info)); + + // Must be either `var[index] op= val` or `var.prop op= val`. + // The return value of any op-assignment (should be `()`) is thrown away and not used. + let _ = match lhs { + // name op= rhs (handled above) + Expr::Variable(..) => { + unreachable!("Expr::Variable case is already handled") + } + // idx_lhs[idx_expr] op= rhs + #[cfg(not(feature = "no_index"))] + Expr::Index(..) => self.eval_dot_index_chain( + global, caches, scope, this_ptr, lhs, _new_val, + ), + // dot_lhs.dot_rhs op= rhs + #[cfg(not(feature = "no_object"))] + Expr::Dot(..) => self.eval_dot_index_chain( + global, caches, scope, this_ptr, lhs, _new_val, + ), + _ => unreachable!("cannot assign to expression: {:?}", lhs), + }?; + } + } + + Ok(Dynamic::UNIT) + } + + // Variable definition + Stmt::Var(x, options, pos) => { + if !self.allow_shadowing() && scope.contains(&x.0) { + return Err(ERR::ErrorVariableExists(x.0.to_string(), *pos).into()); + } + + // Let/const statement + let (var_name, expr, index) = &**x; + + let access = if options.contains(ASTFlags::CONSTANT) { + AccessMode::ReadOnly + } else { + AccessMode::ReadWrite + }; + let export = options.contains(ASTFlags::EXPORTED); + + // Check variable definition filter + if let Some(ref filter) = self.def_var_filter { + let will_shadow = scope.contains(var_name); + let is_const = access == AccessMode::ReadOnly; + let info = VarDefInfo { + name: var_name, + is_const, + nesting_level: global.scope_level, + will_shadow, + }; + let orig_scope_len = scope.len(); + let context = + EvalContext::new(self, global, caches, scope, this_ptr.as_deref_mut()); + let filter_result = filter(true, info, context); + + if orig_scope_len != scope.len() { + // The scope is changed, always search from now on + global.always_search_scope = true; + } + + if !filter_result? { + return Err(ERR::ErrorForbiddenVariable(var_name.to_string(), *pos).into()); + } + } + + // Evaluate initial value + let mut value = self + .eval_expr(global, caches, scope, this_ptr, expr)? + .flatten() + .intern_string(self); + + let _alias = if !rewind_scope { + // Put global constants into global module + #[cfg(not(feature = "no_function"))] + #[cfg(not(feature = "no_module"))] + if global.scope_level == 0 + && access == AccessMode::ReadOnly + && global.lib.iter().any(|m| !m.is_empty()) + { + crate::func::locked_write(global.constants.get_or_insert_with(|| { + crate::Shared::new( + crate::Locked::new(std::collections::BTreeMap::new()), + ) + })) + .insert(var_name.name.clone(), value.clone()); + } + + if export { + Some(var_name) + } else { + None + } + } else if export { + unreachable!("exported variable not on global level"); + } else { + None + }; + + if let Some(index) = index { + value.set_access_mode(access); + *scope.get_mut_by_index(scope.len() - index.get()) = value; + } else { + scope.push_entry(var_name.name.clone(), access, value); + } + + #[cfg(not(feature = "no_module"))] + if let Some(alias) = _alias { + scope.add_alias_by_index(scope.len() - 1, alias.as_str().into()); + } + + Ok(Dynamic::UNIT) + } + // If statement Stmt::If(x, ..) => { let FlowControl { @@ -953,6 +946,7 @@ impl Engine { Ok(Dynamic::UNIT) } + #[allow(unreachable_patterns)] _ => unreachable!("statement cannot be evaluated: {:?}", stmt), } } diff --git a/src/func/call.rs b/src/func/call.rs index 886cdd75..e5fa0507 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -202,20 +202,18 @@ impl Engine { }); #[cfg(not(feature = "no_module"))] - let func = if args.is_none() { - // Scripted functions are not exposed globally - func - } else { - func.or_else(|| _global.get_qualified_fn(hash)).or_else(|| { + let func = func + .or_else(|| _global.get_qualified_fn(hash, true)) + .or_else(|| { self.global_sub_modules .as_deref() .into_iter() .flatten() + .filter(|(_, m)| m.contains_indexed_global_functions()) .find_map(|(_, m)| { m.get_qualified_fn(hash).map(|f| (f, m.id_raw())) }) - }) - }; + }); if let Some((f, s)) = func { // Specific version found @@ -335,8 +333,7 @@ impl Engine { /// Function call arguments be _consumed_ when the function requires them to be passed by value. /// All function arguments not in the first position are always passed by value and thus consumed. /// - /// **DO NOT** reuse the argument values unless for the first `&mut` argument - - /// all others are silently replaced by `()`! + /// **DO NOT** reuse the argument values except for the first `&mut` argument - all others are silently replaced by `()`! pub(crate) fn exec_native_fn_call( &self, global: &mut GlobalRuntimeState, @@ -562,8 +559,7 @@ impl Engine { /// Function call arguments may be _consumed_ when the function requires them to be passed by /// value. All function arguments not in the first position are always passed by value and thus consumed. /// - /// **DO NOT** reuse the argument values unless for the first `&mut` argument - - /// all others are silently replaced by `()`! + /// **DO NOT** reuse the argument values except for the first `&mut` argument - all others are silently replaced by `()`! pub(crate) fn exec_fn_call( &self, global: &mut GlobalRuntimeState, @@ -572,14 +568,14 @@ impl Engine { fn_name: &str, op_token: Option<&Token>, hashes: FnCallHashes, - mut _args: &mut FnCallArgs, + args: &mut FnCallArgs, is_ref_mut: bool, _is_method_call: bool, pos: Position, ) -> RhaiResultOf<(Dynamic, bool)> { // Check for data race. #[cfg(not(feature = "no_closure"))] - ensure_no_data_race(fn_name, _args, is_ref_mut)?; + ensure_no_data_race(fn_name, args, is_ref_mut)?; auto_restore! { let orig_level = global.level; global.level += 1 } @@ -587,18 +583,18 @@ impl Engine { if hashes.is_native_only() { match fn_name { // Handle type_of() - KEYWORD_TYPE_OF if _args.len() == 1 => { - let typ = self.map_type_name(_args[0].type_name()).to_string().into(); - return Ok((typ, false)); + KEYWORD_TYPE_OF if args.len() == 1 => { + let typ = self.get_interned_string(self.map_type_name(args[0].type_name())); + return Ok((typ.into(), false)); } // Handle is_def_fn() #[cfg(not(feature = "no_function"))] crate::engine::KEYWORD_IS_DEF_FN - if _args.len() == 2 && _args[0].is_fnptr() && _args[1].is_int() => + if args.len() == 2 && args[0].is_fnptr() && args[1].is_int() => { - let fn_name = _args[0].read_lock::().expect("`FnPtr`"); - let num_params = _args[1].as_int().expect("`INT`"); + let fn_name = args[0].read_lock::().expect("`FnPtr`"); + let num_params = args[1].as_int().expect("`INT`"); return Ok(( if (0..=crate::MAX_USIZE_INT).contains(&num_params) { @@ -629,16 +625,30 @@ impl Engine { } } + // Script-defined function call? #[cfg(not(feature = "no_function"))] if !hashes.is_native_only() { - // Script-defined function call? let hash = hashes.script(); let local_entry = &mut None; - if let Some(FnResolutionCacheEntry { func, ref source }) = self - .resolve_fn(global, caches, local_entry, None, hash, None, false) - .cloned() - { + #[cfg(not(feature = "no_object"))] + let resolved = if _is_method_call && !args.is_empty() { + let typed_hash = + crate::calc_typed_method_hash(hash, self.map_type_name(args[0].type_name())); + self.resolve_fn(global, caches, local_entry, None, typed_hash, None, false) + } else { + None + }; + #[cfg(feature = "no_object")] + let resolved = None; + + let resolved = if resolved.is_none() { + self.resolve_fn(global, caches, local_entry, None, hash, None, false) + } else { + resolved + }; + + if let Some(FnResolutionCacheEntry { func, ref source }) = resolved.cloned() { // Script function call debug_assert!(func.is_script()); @@ -662,7 +672,7 @@ impl Engine { return if _is_method_call { // Method call of script function - map first argument to `this` - let (first_arg, rest_args) = _args.split_first_mut().unwrap(); + let (first_arg, rest_args) = args.split_first_mut().unwrap(); self.call_script_fn( global, @@ -680,13 +690,13 @@ impl Engine { let backup = &mut ArgBackup::new(); // The first argument is a reference? - let swap = is_ref_mut && !_args.is_empty(); + let swap = is_ref_mut && !args.is_empty(); if swap { - backup.change_first_arg_to_copy(_args); + backup.change_first_arg_to_copy(args); } - auto_restore! { args = (_args) if swap => move |a| backup.restore_first_arg(a) } + auto_restore! { args = (args) if swap => move |a| backup.restore_first_arg(a) } self.call_script_fn(global, caches, scope, None, environ, f, args, true, pos) } @@ -698,7 +708,7 @@ impl Engine { let hash = hashes.native(); self.exec_native_fn_call( - global, caches, fn_name, op_token, hash, _args, is_ref_mut, pos, + global, caches, fn_name, op_token, hash, args, is_ref_mut, pos, ) } @@ -1392,15 +1402,11 @@ impl Engine { .ok_or_else(|| ERR::ErrorModuleNotFound(namespace.to_string(), namespace.position()))?; // First search script-defined functions in namespace (can override built-in) - let mut func = match module.get_qualified_fn(hash) { + let mut func = module.get_qualified_fn(hash).or_else(|| { // Then search native Rust functions - None => { - self.track_operation(global, pos)?; - let hash_qualified_fn = calc_fn_hash_full(hash, args.iter().map(|a| a.type_id())); - module.get_qualified_fn(hash_qualified_fn) - } - r => r, - }; + let hash_qualified_fn = calc_fn_hash_full(hash, args.iter().map(|a| a.type_id())); + module.get_qualified_fn(hash_qualified_fn) + }); // Check for `Dynamic` parameters. // diff --git a/src/func/hashing.rs b/src/func/hashing.rs index acc94fae..227921b4 100644 --- a/src/func/hashing.rs +++ b/src/func/hashing.rs @@ -76,6 +76,9 @@ pub fn get_hasher() -> ahash::AHasher { #[must_use] pub fn calc_var_hash<'a>(namespace: impl IntoIterator, var_name: &str) -> u64 { let s = &mut get_hasher(); + + s.write_u8(b'V'); // hash a discriminant + let mut count = 0; // We always skip the first module @@ -111,6 +114,9 @@ pub fn calc_fn_hash<'a>( num: usize, ) -> u64 { let s = &mut get_hasher(); + + s.write_u8(b'F'); // hash a discriminant + let mut count = 0; namespace.into_iter().for_each(|m| { @@ -134,6 +140,9 @@ pub fn calc_fn_hash<'a>( #[must_use] pub fn calc_fn_hash_full(base: u64, params: impl IntoIterator) -> u64 { let s = &mut get_hasher(); + + s.write_u8(b'A'); // hash a discriminant + let mut count = 0; params.into_iter().for_each(|t| { t.hash(s); @@ -143,3 +152,17 @@ pub fn calc_fn_hash_full(base: u64, params: impl IntoIterator) -> s.finish() ^ base } + +/// Calculate a [`u64`] hash key from a base [`u64`] hash key and the type of the `this` pointer. +#[cfg(not(feature = "no_object"))] +#[cfg(not(feature = "no_function"))] +#[inline] +#[must_use] +pub fn calc_typed_method_hash(base: u64, this_type: &str) -> u64 { + let s = &mut get_hasher(); + + s.write_u8(b'T'); // hash a discriminant + this_type.hash(s); + + s.finish() ^ base +} diff --git a/src/func/mod.rs b/src/func/mod.rs index b726b5ac..bf9f60c5 100644 --- a/src/func/mod.rs +++ b/src/func/mod.rs @@ -21,6 +21,9 @@ pub use call::FnCallArgs; pub use callable_function::{CallableFunction, EncapsulatedEnviron}; #[cfg(not(feature = "no_function"))] pub use func::Func; +#[cfg(not(feature = "no_object"))] +#[cfg(not(feature = "no_function"))] +pub use hashing::calc_typed_method_hash; pub use hashing::{calc_fn_hash, calc_fn_hash_full, calc_var_hash, get_hasher, StraightHashMap}; #[cfg(feature = "internals")] #[allow(deprecated)] diff --git a/src/func/script.rs b/src/func/script.rs index 18c09099..4d119cfc 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -22,7 +22,7 @@ impl Engine { /// Function call arguments may be _consumed_ when the function requires them to be passed by value. /// All function arguments not in the first position are always passed by value and thus consumed. /// - /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! + /// **DO NOT** reuse the argument values except for the first `&mut` argument - all others are silently replaced by `()`! pub(crate) fn call_script_fn( &self, global: &mut GlobalRuntimeState, diff --git a/src/lib.rs b/src/lib.rs index d27a2a4d..8f4dda89 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -227,6 +227,9 @@ pub use api::{eval::eval, events::VarDefInfo, run::run}; pub use ast::{FnAccess, AST}; pub use engine::{Engine, OP_CONTAINS, OP_EQUALS}; pub use eval::EvalContext; +#[cfg(not(feature = "no_function"))] +#[cfg(not(feature = "no_object"))] +use func::calc_typed_method_hash; use func::{calc_fn_hash, calc_fn_hash_full, calc_var_hash}; pub use func::{plugin, FuncArgs, NativeCallContext, RegisterNativeFunction}; pub use module::{FnNamespace, Module}; diff --git a/src/module/mod.rs b/src/module/mod.rs index 950a6d30..84b27c4b 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -73,6 +73,9 @@ pub struct FuncInfoMetadata { pub access: FnAccess, /// Function name. pub name: Identifier, + #[cfg(not(feature = "no_object"))] + /// Type of `this` pointer, if any. + pub this_type: Option, /// Number of parameters. pub num_params: usize, /// Parameter types (if applicable). @@ -728,8 +731,16 @@ impl Module { let fn_def = fn_def.into(); // None + function name + number of arguments. + let namespace = FnNamespace::Internal; let num_params = fn_def.params.len(); let hash_script = crate::calc_fn_hash(None, &fn_def.name, num_params); + #[cfg(not(feature = "no_object"))] + let (hash_script, namespace) = if let Some(ref this_type) = fn_def.this_type { + let hash = crate::calc_typed_method_hash(hash_script, this_type); + (hash, FnNamespace::Global) + } else { + (hash_script, namespace) + }; // Catch hash collisions in testing environment only. #[cfg(feature = "testing-environ")] @@ -750,8 +761,10 @@ impl Module { FuncInfo { metadata: FuncInfoMetadata { name: fn_def.name.as_str().into(), - namespace: FnNamespace::Internal, + namespace, access: fn_def.access, + #[cfg(not(feature = "no_object"))] + this_type: fn_def.this_type.clone(), num_params, param_types: FnArgsVec::new_const(), #[cfg(feature = "metadata")] @@ -765,8 +778,10 @@ impl Module { func: fn_def.into(), }, ); + self.flags .remove(ModuleFlags::INDEXED | ModuleFlags::INDEXED_GLOBAL_FUNCTIONS); + hash_script } @@ -1009,7 +1024,7 @@ impl Module { type_id } - /// Set a Rust function into the [`Module`], returning a [`u64`] hash key. + /// Set a native Rust function into the [`Module`], returning a [`u64`] hash key. /// /// If there is an existing Rust function of the same hash, it is replaced. /// @@ -1067,22 +1082,22 @@ impl Module { }; let name = name.as_ref(); - let hash_script = calc_fn_hash(None, name, param_types.len()); - let hash_fn = calc_fn_hash_full(hash_script, param_types.iter().copied()); + let hash_base = calc_fn_hash(None, name, param_types.len()); + let hash_fn = calc_fn_hash_full(hash_base, param_types.iter().copied()); // Catch hash collisions in testing environment only. #[cfg(feature = "testing-environ")] - if let Some(f) = self.functions.as_ref().and_then(|f| f.get(&hash_script)) { + if let Some(f) = self.functions.as_ref().and_then(|f| f.get(&hash_base)) { panic!( "Hash {} already exists when registering function {}:\n{:#?}", - hash_script, name, f + hash_base, name, f ); } if is_dynamic { self.dynamic_functions_filter .get_or_insert_with(Default::default) - .mark(hash_script); + .mark(hash_base); } self.functions @@ -1095,6 +1110,8 @@ impl Module { name: name.into(), namespace, access, + #[cfg(not(feature = "no_object"))] + this_type: None, num_params: param_types.len(), param_types, #[cfg(feature = "metadata")] @@ -1114,7 +1131,7 @@ impl Module { hash_fn } - /// _(metadata)_ Set a Rust function into the [`Module`], returning a [`u64`] hash key. + /// _(metadata)_ Set a native Rust function into the [`Module`], returning a [`u64`] hash key. /// Exported under the `metadata` feature only. /// /// If there is an existing Rust function of the same hash, it is replaced. @@ -1167,13 +1184,7 @@ impl Module { hash } - /// Set a Rust function taking a reference to the scripting [`Engine`][crate::Engine], - /// the current set of functions, plus a list of mutable [`Dynamic`] references - /// into the [`Module`], returning a [`u64`] hash key. - /// - /// Use this to register a built-in function which must reference settings on the scripting - /// [`Engine`][crate::Engine] (e.g. to prevent growing an array beyond the allowed maximum size), - /// or to call a script-defined function in the current evaluation context. + /// Set a native Rust function into the [`Module`], returning a [`u64`] hash key. /// /// If there is a similar existing Rust function, it is replaced. /// @@ -1259,7 +1270,7 @@ impl Module { ) } - /// Set a Rust function into the [`Module`], returning a [`u64`] hash key. + /// Set a native Rust function into the [`Module`], returning a [`u64`] hash key. /// /// If there is a similar existing Rust function, it is replaced. /// @@ -1618,7 +1629,7 @@ impl Module { ) } - /// Look up a Rust function by hash. + /// Look up a native Rust function by hash. /// /// The [`u64`] hash is returned by the [`set_native_fn`][Module::set_native_fn] call. #[inline] @@ -2298,14 +2309,14 @@ impl Module { } } - // Index type iterators + // Index all type iterators if let Some(ref t) = module.type_iterators { for (&type_id, func) in t.iter() { type_iterators.insert(type_id, func.clone()); } } - // Index all Rust functions + // Index all functions for (&hash, f) in module.functions.iter().flatten() { match f.metadata.namespace { FnNamespace::Global => { @@ -2346,23 +2357,33 @@ impl Module { } functions.insert(hash_qualified_fn, f.func.clone()); - } else if cfg!(not(feature = "no_function")) { - let hash_qualified_script = crate::calc_fn_hash( - path.iter().copied(), - &f.metadata.name, - f.metadata.num_params, - ); - - // Catch hash collisions in testing environment only. - #[cfg(feature = "testing-environ")] - if let Some(fx) = functions.get(&hash_qualified_script) { - panic!( - "Hash {} already exists when indexing function {:#?}:\n{:#?}", - hash_qualified_script, f.func, fx + } else { + #[cfg(not(feature = "no_function"))] + { + let hash_qualified_script = crate::calc_fn_hash( + path.iter().copied(), + &f.metadata.name, + f.metadata.num_params, ); - } + #[cfg(not(feature = "no_object"))] + let hash_qualified_script = + if let Some(ref this_type) = f.metadata.this_type { + crate::calc_typed_method_hash(hash_qualified_script, this_type) + } else { + hash_qualified_script + }; - functions.insert(hash_qualified_script, f.func.clone()); + // Catch hash collisions in testing environment only. + #[cfg(feature = "testing-environ")] + if let Some(fx) = functions.get(&hash_qualified_script) { + panic!( + "Hash {} already exists when indexing function {:#?}:\n{:#?}", + hash_qualified_script, f.func, fx + ); + } + + functions.insert(hash_qualified_script, f.func.clone()); + } } } diff --git a/src/optimizer.rs b/src/optimizer.rs index 021a62d0..dd8166c0 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -1348,8 +1348,9 @@ impl Engine { name: fn_def.name.clone(), access: fn_def.access, body: crate::ast::StmtBlock::NONE, + #[cfg(not(feature = "no_object"))] + this_type: fn_def.this_type.clone(), params: fn_def.params.clone(), - #[cfg(not(feature = "no_function"))] #[cfg(feature = "metadata")] comments: Box::default(), }) diff --git a/src/packages/lang_core.rs b/src/packages/lang_core.rs index 1e5a736b..c7041387 100644 --- a/src/packages/lang_core.rs +++ b/src/packages/lang_core.rs @@ -208,6 +208,10 @@ fn collect_fn_metadata( "is_anonymous".into(), func.name.starts_with(FN_ANONYMOUS).into(), ); + #[cfg(not(feature = "no_object"))] + if let Some(ref this_type) = func.this_type { + map.insert("this_type".into(), this_type.into()); + } map.insert( "params".into(), func.params diff --git a/src/parser.rs b/src/parser.rs index cbe69827..207a53a6 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -864,7 +864,7 @@ impl Engine { let settings = settings.level_up()?; // Recursively parse the indexing chain, right-binding each let options = match token { - Token::LeftBracket => ASTFlags::NONE, + Token::LeftBracket => ASTFlags::empty(), Token::QuestionBracket => ASTFlags::NEGATED, _ => unreachable!("`[` or `?[`"), }; @@ -1810,7 +1810,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] (expr, token @ (Token::LeftBracket | Token::QuestionBracket)) => { let opt = match token { - Token::LeftBracket => ASTFlags::NONE, + Token::LeftBracket => ASTFlags::empty(), Token::QuestionBracket => ASTFlags::NEGATED, _ => unreachable!("`[` or `?[`"), }; @@ -1834,7 +1834,7 @@ impl Engine { } let op_flags = match op { - Token::Period => ASTFlags::NONE, + Token::Period => ASTFlags::empty(), Token::Elvis => ASTFlags::NEGATED, _ => unreachable!("`.` or `?.`"), }; @@ -1842,7 +1842,7 @@ impl Engine { let rhs = self.parse_primary(input, state, lib, settings.level_up()?, options)?; - Self::make_dot_expr(state, expr, rhs, ASTFlags::NONE, op_flags, tail_pos)? + Self::make_dot_expr(state, expr, rhs, ASTFlags::empty(), op_flags, tail_pos)? } // Unknown postfix operator (expr, token) => { @@ -2141,7 +2141,7 @@ impl Engine { { let options = options | parent_options; x.rhs = Self::make_dot_expr(state, x.rhs, rhs, options, op_flags, op_pos)?; - Ok(Expr::Index(x, ASTFlags::NONE, pos)) + Ok(Expr::Index(x, ASTFlags::empty(), pos)) } // lhs.module::id - syntax error #[cfg(not(feature = "no_module"))] @@ -2770,7 +2770,7 @@ impl Engine { let body = self.parse_block(input, state, lib, settings)?.into(); let negated = match input.next().expect(NEVER_ENDS) { - (Token::While, ..) => ASTFlags::NONE, + (Token::While, ..) => ASTFlags::empty(), (Token::Until, ..) => ASTFlags::NEGATED, (.., pos) => { return Err( @@ -2966,7 +2966,7 @@ impl Engine { let export = if is_export { ASTFlags::EXPORTED } else { - ASTFlags::NONE + ASTFlags::empty() }; let (existing, hit_barrier) = state.find_var(&name); @@ -3392,10 +3392,15 @@ impl Engine { comments, )?; - // Restore parse state - let hash = calc_fn_hash(None, &f.name, f.params.len()); + #[cfg(not(feature = "no_object"))] + let hash = if let Some(ref this_type) = f.this_type { + crate::calc_typed_method_hash(hash, this_type) + } else { + hash + }; + if !lib.is_empty() && lib.contains_key(&hash) { return Err(PERR::FnDuplicatedDefinition( f.name.to_string(), @@ -3433,7 +3438,7 @@ impl Engine { if self.allow_looping() && settings.has_flag(ParseSettingFlags::BREAKABLE) => { let pos = eat_token(input, Token::Continue); - Ok(Stmt::BreakLoop(None, ASTFlags::NONE, pos)) + Ok(Stmt::BreakLoop(None, ASTFlags::empty(), pos)) } Token::Break if self.allow_looping() && settings.has_flag(ParseSettingFlags::BREAKABLE) => @@ -3465,7 +3470,7 @@ impl Engine { .next() .map(|(token, pos)| { let flags = match token { - Token::Return => ASTFlags::NONE, + Token::Return => ASTFlags::empty(), Token::Throw => ASTFlags::BREAK, token => unreachable!( "Token::Return or Token::Throw expected but gets {:?}", @@ -3605,6 +3610,35 @@ impl Engine { let (token, pos) = input.next().expect(NEVER_ENDS); + // Parse type for `this` pointer + #[cfg(not(feature = "no_object"))] + let ((token, pos), this_type) = match token { + Token::StringConstant(s) if input.peek().expect(NEVER_ENDS).0 == Token::Period => { + eat_token(input, Token::Period); + let s = match s.as_str() { + "int" => state.get_interned_string(std::any::type_name::()), + #[cfg(not(feature = "no_float"))] + "float" => state.get_interned_string(std::any::type_name::()), + _ => state.get_interned_string(*s), + }; + (input.next().expect(NEVER_ENDS), Some(s)) + } + Token::StringConstant(..) => { + return Err(PERR::MissingSymbol(".".to_string()).into_err(pos)) + } + Token::Identifier(s) if input.peek().expect(NEVER_ENDS).0 == Token::Period => { + eat_token(input, Token::Period); + let s = match s.as_str() { + "int" => state.get_interned_string(std::any::type_name::()), + #[cfg(not(feature = "no_float"))] + "float" => state.get_interned_string(std::any::type_name::()), + _ => state.get_interned_string(*s), + }; + (input.next().expect(NEVER_ENDS), Some(s)) + } + _ => ((token, pos), None), + }; + let name = match token.into_function_name_for_override() { Ok(r) => r, Err(Token::Reserved(s)) => return Err(PERR::Reserved(s.to_string()).into_err(pos)), @@ -3679,6 +3713,8 @@ impl Engine { Ok(ScriptFnDef { name: state.get_interned_string(name), access, + #[cfg(not(feature = "no_object"))] + this_type, params, body, #[cfg(feature = "metadata")] @@ -3839,6 +3875,8 @@ impl Engine { let script = Shared::new(ScriptFnDef { name: fn_name.clone(), access: crate::FnAccess::Public, + #[cfg(not(feature = "no_object"))] + this_type: None, params, body: body.into(), #[cfg(not(feature = "no_function"))] diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index e207ee29..719298eb 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -38,6 +38,9 @@ struct FnMetadata<'a> { pub is_anonymous: bool, #[serde(rename = "type")] pub typ: FnType, + #[cfg(not(feature = "no_object"))] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub this_type: Option<&'a str>, pub num_params: usize, #[serde(default, skip_serializing_if = "StaticVec::is_empty")] pub params: StaticVec>, @@ -88,6 +91,8 @@ impl<'a> From<&'a FuncInfo> for FnMetadata<'a> { #[cfg(not(feature = "no_function"))] is_anonymous: crate::parser::is_anonymous_fn(&info.metadata.name), typ, + #[cfg(not(feature = "no_object"))] + this_type: info.metadata.this_type.as_ref().map(|s| s.as_str()), num_params: info.metadata.num_params, params: info .metadata diff --git a/tests/method_call.rs b/tests/method_call.rs index dee55417..824f1c72 100644 --- a/tests/method_call.rs +++ b/tests/method_call.rs @@ -75,3 +75,67 @@ fn test_method_call_with_full_optimization() -> Result<(), Box> { Ok(()) } + +#[cfg(not(feature = "no_function"))] +#[test] +fn test_method_call_typed() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine + .register_type_with_name::("Test-Struct#ABC") + .register_fn("update", TestStruct::update) + .register_fn("new_ts", TestStruct::new); + + assert_eq!( + engine.eval::( + r#" + fn "Test-Struct#ABC".foo(x) { + this.update(x); + } + fn foo(x) { + this += x; + } + + let z = 1000; + z.foo(1); + + let x = new_ts(); + x.foo(z); + + x + "# + )?, + TestStruct { x: 1002 } + ); + + assert!(matches!( + *engine + .run( + r#" + fn "Test-Struct#ABC".foo(x) { + this.update(x); + } + foo(1000); + "# + ) + .expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(f, ..) if f.starts_with("foo") + )); + + assert!(matches!( + *engine + .run( + r#" + fn "Test-Struct#ABC".foo(x) { + this.update(x); + } + let x = 42; + x.foo(1000); + "# + ) + .expect_err("should error"), + EvalAltResult::ErrorFunctionNotFound(f, ..) if f.starts_with("foo") + )); + + Ok(()) +} diff --git a/src/tools/README.md b/tools/README.md similarity index 100% rename from src/tools/README.md rename to tools/README.md diff --git a/src/tools/keywords.txt b/tools/keywords.txt similarity index 100% rename from src/tools/keywords.txt rename to tools/keywords.txt diff --git a/src/tools/reserved.txt b/tools/reserved.txt similarity index 100% rename from src/tools/reserved.txt rename to tools/reserved.txt