From fc10df7d638d4760029f0cc6bb0628e921efe452 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 1 Mar 2021 17:17:13 +0800 Subject: [PATCH] Keyword can no longer be overloaded. --- CHANGELOG.md | 1 + src/fn_call.rs | 218 ++++++++++++++------------------------- src/optimize.rs | 2 +- src/packages/fn_basic.rs | 2 +- src/token.rs | 11 -- tests/eval.rs | 28 +---- 6 files changed, 79 insertions(+), 183 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d9d2e94..4413b740 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Breaking changes * `private` functions in an `AST` can now be called with `call_fn` etc. * `NativeCallContext::call_fn_dynamic_raw` no longer has the `pub_only` parameter. * `Module::update_fn_metadata` input parameter is changed. +* Function keywords (e.g. `type_of`, `eval`, `Fn`) can no longer be overloaded. It is more trouble than worth. To disable these keywords, use `Engine::disable_symbol`. Enhancements ------------ diff --git a/src/fn_call.rs b/src/fn_call.rs index a60f1284..d703b30c 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -192,9 +192,9 @@ impl Engine { #[inline] fn resolve_function<'s>( &self, + mods: &Imports, state: &'s mut State, lib: &[&Module], - mods: &Imports, fn_name: &str, mut hash: NonZeroU64, args: &mut FnCallArgs, @@ -203,8 +203,8 @@ impl Engine { fn find_function( engine: &Engine, hash: NonZeroU64, - lib: &[&Module], mods: &Imports, + lib: &[&Module], ) -> Option<(CallableFunction, Option)> { lib.iter() .find_map(|m| { @@ -249,7 +249,7 @@ impl Engine { let mut bitmask = 1usize; // Bitmask of which parameter to replace with `Dynamic` loop { - match find_function(self, hash, lib, mods) { + match find_function(self, hash, mods, lib) { // Specific version found Some(f) => return Some(f), @@ -304,7 +304,7 @@ impl Engine { let source = state.source.clone(); // Check if function access already in the cache - let func = self.resolve_function(state, lib, mods, fn_name, hash_fn, args, true); + let func = self.resolve_function(mods, state, lib, fn_name, hash_fn, args, true); if let Some((func, src)) = func { assert!(func.is_native()); @@ -578,16 +578,18 @@ impl Engine { pub(crate) fn has_override_by_name_and_arguments( &self, mods: Option<&Imports>, - state: Option<&mut State>, lib: &[&Module], fn_name: &str, arg_types: &[TypeId], ) -> bool { let arg_types = arg_types.as_ref(); - let hash_fn = calc_native_fn_hash(empty(), fn_name, arg_types.iter().cloned()); - let hash_script = calc_script_fn_hash(empty(), fn_name, arg_types.len()); - self.has_override(mods, state, lib, hash_fn, hash_script) + self.has_override( + mods, + lib, + calc_native_fn_hash(empty(), fn_name, arg_types.iter().cloned()), + calc_script_fn_hash(empty(), fn_name, arg_types.len()), + ) } // Has a system function an override? @@ -595,29 +597,12 @@ impl Engine { pub(crate) fn has_override( &self, mods: Option<&Imports>, - mut state: Option<&mut State>, lib: &[&Module], hash_fn: Option, hash_script: Option, ) -> bool { - // Check if it is already in the cache - if let Some(state) = state.as_mut() { - if let Some(hash) = hash_script { - match state.fn_resolution_cache().map_or(None, |c| c.get(&hash)) { - Some(v) => return v.is_some(), - None => (), - } - } - if let Some(hash) = hash_fn { - match state.fn_resolution_cache().map_or(None, |c| c.get(&hash)) { - Some(v) => return v.is_some(), - None => (), - } - } - } - // First check script-defined functions - let r = hash_script.map_or(false, |hash| lib.iter().any(|&m| m.contains_fn(hash, false))) + 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))) // Then check registered functions || hash_script.map_or(false, |hash| self.global_namespace.contains_fn(hash, false)) @@ -630,21 +615,7 @@ impl Engine { || hash_fn.map_or(false, |hash| mods.map_or(false, |m| m.contains_fn(hash))) // 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))); - - // If there is no override, put that information into the cache - if !r { - if let Some(state) = state.as_mut() { - if let Some(hash) = hash_script { - state.fn_resolution_cache_mut().insert(hash, None); - } - if let Some(hash) = hash_fn { - state.fn_resolution_cache_mut().insert(hash, None); - } - } - } - - r + || hash_fn.map_or(false, |hash| self.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash))) } /// Perform an actual function call, native Rust or scripted, taking care of special functions. @@ -678,49 +649,27 @@ impl Engine { match fn_name { // type_of - KEYWORD_TYPE_OF - if args.len() == 1 - && !self.has_override( - Some(mods), - Some(state), - lib, - Some(hash_fn), - hash_script, - ) => - { - Ok(( - self.map_type_name(args[0].type_name()).to_string().into(), - false, - )) - } + KEYWORD_TYPE_OF if args.len() == 1 => Ok(( + self.map_type_name(args[0].type_name()).to_string().into(), + false, + )), // Fn/eval - reaching this point it must be a method-style call, mostly like redirected // by a function pointer so it isn't caught at parse time. - KEYWORD_FN_PTR | KEYWORD_EVAL - if args.len() == 1 - && !self.has_override( - Some(mods), - Some(state), - lib, - Some(hash_fn), - hash_script, - ) => - { - EvalAltResult::ErrorRuntime( - format!( - "'{}' should not be called in method style. Try {}(...);", - fn_name, fn_name - ) - .into(), - pos, + KEYWORD_FN_PTR | KEYWORD_EVAL if args.len() == 1 => EvalAltResult::ErrorRuntime( + format!( + "'{}' should not be called in method style. Try {}(...);", + fn_name, fn_name ) - .into() - } + .into(), + pos, + ) + .into(), #[cfg(not(feature = "no_function"))] _ if hash_script.is_some() => { let (func, source) = self - .resolve_function(state, lib, mods, fn_name, hash_script.unwrap(), args, false) + .resolve_function(mods, state, lib, fn_name, hash_script.unwrap(), args, false) .as_ref() .map(|(f, s)| (Some(f.clone()), s.clone())) .unwrap_or((None, None)); @@ -1037,10 +986,7 @@ impl Engine { let mut curry = StaticVec::new(); let mut name = fn_name; - if name == KEYWORD_FN_PTR_CALL - && args_expr.len() >= 1 - && !self.has_override(Some(mods), Some(state), lib, None, hash_script) - { + if name == KEYWORD_FN_PTR_CALL && args_expr.len() >= 1 { let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; if !fn_ptr.is::() { @@ -1067,20 +1013,16 @@ impl Engine { // Handle Fn() if name == KEYWORD_FN_PTR && args_expr.len() == 1 { - let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); - - if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script) { - // Fn - only in function call style - return self - .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)? - .take_immutable_string() - .map_err(|typ| { - self.make_type_mismatch_err::(typ, args_expr[0].position()) - }) - .and_then(|s| FnPtr::try_from(s)) - .map(Into::::into) - .map_err(|err| err.fill_position(args_expr[0].position())); - } + // Fn - only in function call style + return self + .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)? + .take_immutable_string() + .map_err(|typ| { + self.make_type_mismatch_err::(typ, args_expr[0].position()) + }) + .and_then(|s| FnPtr::try_from(s)) + .map(Into::::into) + .map_err(|err| err.fill_position(args_expr[0].position())); } // Handle curry() @@ -1119,63 +1061,53 @@ impl Engine { // Handle is_def_var() if name == KEYWORD_IS_DEF_VAR && args_expr.len() == 1 { - let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); - - if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script) { - let var_name = - self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; - let var_name = var_name.as_str().map_err(|err| { - self.make_type_mismatch_err::(err, args_expr[0].position()) - })?; - return Ok(scope.contains(var_name).into()); - } + let var_name = + self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; + let var_name = var_name.as_str().map_err(|err| { + self.make_type_mismatch_err::(err, args_expr[0].position()) + })?; + return Ok(scope.contains(var_name).into()); } // Handle eval() if name == KEYWORD_EVAL && args_expr.len() == 1 { - let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); - let script_expr = &args_expr[0]; + let script_pos = script_expr.position(); - if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script) { - let script_pos = script_expr.position(); + // 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 + .as_str() + .map_err(|typ| self.make_type_mismatch_err::(typ, script_pos))?; + let result = self.eval_script_expr_in_place( + scope, + mods, + state, + lib, + script, + script_pos, + level + 1, + ); - // 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.as_str().map_err(|typ| { - self.make_type_mismatch_err::(typ, script_pos) - })?; - let result = self.eval_script_expr_in_place( - scope, - mods, - state, - lib, - script, - script_pos, - level + 1, - ); - - // IMPORTANT! If the eval defines new variables in the current scope, - // all variable offsets from this point on will be mis-aligned. - if scope.len() != prev_len { - state.always_search = true; - } - - return result.map_err(|err| { - Box::new(EvalAltResult::ErrorInFunctionCall( - KEYWORD_EVAL.to_string(), - state - .source - .as_ref() - .map_or_else(|| "", |s| s.as_str()) - .to_string(), - err, - pos, - )) - }); + // IMPORTANT! If the eval defines new variables in the current scope, + // all variable offsets from this point on will be mis-aligned. + if scope.len() != prev_len { + state.always_search = true; } + + return result.map_err(|err| { + Box::new(EvalAltResult::ErrorInFunctionCall( + KEYWORD_EVAL.to_string(), + state + .source + .as_ref() + .map_or_else(|| "", |s| s.as_str()) + .to_string(), + err, + pos, + )) + }); } // Normal function call - except for Fn, curry, call and eval (handled above) diff --git a/src/optimize.rs b/src/optimize.rs index 4106f3cc..81543baa 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -673,7 +673,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(&state.mods), None, state.lib, x.name.as_ref(), arg_types.as_ref()) { + if !state.engine.has_override_by_name_and_arguments(Some(&state.mods), state.lib, x.name.as_ref(), arg_types.as_ref()) { if let Some(result) = run_builtin_binary_op(x.name.as_ref(), &arg_values[0], &arg_values[1]) .ok().flatten() .and_then(|result| map_dynamic_to_expr(result, *pos)) diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs index 91baee2a..01aa1e39 100644 --- a/src/packages/fn_basic.rs +++ b/src/packages/fn_basic.rs @@ -33,7 +33,7 @@ mod fn_ptr_functions { let hash_script = calc_script_fn_hash(empty(), fn_name, num_params as usize); ctx.engine() - .has_override(ctx.mods, None, ctx.lib, None, hash_script) + .has_override(ctx.mods, ctx.lib, None, hash_script) } } } diff --git a/src/token.rs b/src/token.rs index ea25f37a..7eb89947 100644 --- a/src/token.rs +++ b/src/token.rs @@ -759,7 +759,6 @@ impl Token { #[cfg(not(feature = "no_function"))] pub(crate) fn into_function_name_for_override(self) -> Result { match self { - Self::Reserved(s) if can_override_keyword(&s) => Ok(s), Self::Custom(s) | Self::Identifier(s) if is_valid_identifier(s.chars()) => Ok(s), _ => Err(self), } @@ -1630,16 +1629,6 @@ pub fn is_keyword_function(name: &str) -> bool { } } -/// Can this keyword be overridden as a function? -#[cfg(not(feature = "no_function"))] -#[inline(always)] -pub fn can_override_keyword(name: &str) -> bool { - match name { - KEYWORD_PRINT | KEYWORD_DEBUG | KEYWORD_TYPE_OF | KEYWORD_EVAL | KEYWORD_FN_PTR => true, - _ => false, - } -} - /// Is a text string a valid identifier? pub fn is_valid_identifier(name: impl Iterator) -> bool { let mut first_alphabetic = false; diff --git a/tests/eval.rs b/tests/eval.rs index c59ccfff..18b4145d 100644 --- a/tests/eval.rs +++ b/tests/eval.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, LexError, ParseErrorType, RegisterFn, Scope, INT}; +use rhai::{Engine, EvalAltResult, LexError, ParseErrorType, Scope, INT}; #[test] fn test_eval() -> Result<(), Box> { @@ -80,32 +80,6 @@ fn test_eval_function() -> Result<(), Box> { Ok(()) } -#[test] -#[cfg(not(feature = "no_function"))] -fn test_eval_override() -> Result<(), Box> { - let engine = Engine::new(); - - assert_eq!( - engine.eval::( - r#" - fn eval(x) { x } // reflect the script back - - eval("40 + 2") - "# - )?, - "40 + 2" - ); - - let mut engine = Engine::new(); - - // Reflect the script back - engine.register_fn("eval", |script: &str| script.to_string()); - - assert_eq!(engine.eval::(r#"eval("40 + 2")"#)?, "40 + 2"); - - Ok(()) -} - #[test] fn test_eval_disabled() -> Result<(), Box> { let mut engine = Engine::new();