From 39474d642001f9438c3239fd542c6f28329f94f8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 17 Oct 2020 13:49:16 +0800 Subject: [PATCH] Streamline function pointers and currying. --- RELEASES.md | 11 ++ doc/src/language/fn-ptr.md | 2 + src/engine.rs | 2 +- src/fn_call.rs | 217 ++++++++++++++++++------------------- src/fn_native.rs | 27 ++--- src/module/mod.rs | 24 ++++ src/packages/mod.rs | 7 ++ src/parser.rs | 8 ++ src/syntax.rs | 5 +- 9 files changed, 171 insertions(+), 132 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 19a0e65d..c06a0242 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,6 +1,17 @@ Rhai Release Notes ================== + +Version 0.19.3 +============== + + +Version 0.19.2 +============== + +Bug fix on call module functions. + + Version 0.19.1 ============== diff --git a/doc/src/language/fn-ptr.md b/doc/src/language/fn-ptr.md index b8412138..9e22eb70 100644 --- a/doc/src/language/fn-ptr.md +++ b/doc/src/language/fn-ptr.md @@ -6,6 +6,8 @@ Function Pointers It is possible to store a _function pointer_ in a variable just like a normal value. In fact, internally a function pointer simply stores the _name_ of the function as a string. +A function pointer is created via the `Fn` function, which takes a [string] parameter. + Call a function pointer using the `call` method. diff --git a/src/engine.rs b/src/engine.rs index b8bbbc1e..aab58fbc 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1479,7 +1479,7 @@ impl Engine { Expr::Stmt(x) => self.eval_stmt(scope, mods, state, lib, this_ptr, &x.0, level), // var op= rhs - Expr::Assignment(x) if matches!(x.0, Expr::Variable(_)) => { + Expr::Assignment(x) if x.0.get_variable_access(false).is_some() => { let (lhs_expr, op, rhs_expr, op_pos) = x.as_ref(); let mut rhs_val = self .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)? diff --git a/src/fn_call.rs b/src/fn_call.rs index e0cf13e0..5785f1d2 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -711,16 +711,17 @@ impl Engine { let (result, updated) = if _fn_name == KEYWORD_FN_PTR_CALL && obj.is::() { // FnPtr call let fn_ptr = obj.read_lock::().unwrap(); - let mut curry = fn_ptr.curry().iter().cloned().collect::>(); // Redirect function name let fn_name = fn_ptr.fn_name(); + let args_len = call_args.len() + fn_ptr.curry().len(); // Recalculate hash let hash = if native { 0 } else { - calc_fn_hash(empty(), fn_name, curry.len() + call_args.len(), empty()) + calc_fn_hash(empty(), fn_name, args_len, empty()) }; // Arguments are passed as-is, adding the curried arguments + let mut curry = fn_ptr.curry().iter().cloned().collect::>(); let mut arg_values = curry .iter_mut() .chain(call_args.iter_mut()) @@ -737,16 +738,17 @@ impl Engine { { // FnPtr call on object let fn_ptr = call_args.remove(0).cast::(); - let mut curry = fn_ptr.curry().iter().cloned().collect::>(); // Redirect function name - let fn_name = fn_ptr.get_fn_name().clone(); + let fn_name = fn_ptr.fn_name(); + let args_len = call_args.len() + fn_ptr.curry().len(); // Recalculate hash let hash = if native { 0 } else { - calc_fn_hash(empty(), &fn_name, curry.len() + call_args.len(), empty()) + calc_fn_hash(empty(), fn_name, args_len, empty()) }; // 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) .chain(curry.iter_mut()) .chain(call_args.iter_mut()) @@ -755,7 +757,7 @@ impl Engine { // Map it to name(args) in function-call style self.exec_fn_call( - state, lib, &fn_name, hash, args, is_ref, true, pub_only, None, def_val, level, + state, lib, fn_name, hash, args, is_ref, true, pub_only, None, def_val, level, ) } else if _fn_name == KEYWORD_FN_PTR_CURRY && obj.is::() { // Curry call @@ -796,14 +798,12 @@ impl Engine { _redirected = fn_ptr.get_fn_name().clone(); _fn_name = &_redirected; // Add curried arguments - if !fn_ptr.curry().is_empty() { - fn_ptr - .curry() - .iter() - .cloned() - .enumerate() - .for_each(|(i, v)| call_args.insert(i, v)); - } + fn_ptr + .curry() + .iter() + .cloned() + .enumerate() + .for_each(|(i, v)| call_args.insert(i, v)); // Recalculate the hash based on the new function name and new arguments hash = if native { 0 @@ -861,35 +861,33 @@ impl Engine { if !self.has_override(lib, hash_fn, hash_script, pub_only) { // Fn - only in function call style - let expr = args_expr.get(0).unwrap(); - let arg_value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; - - return arg_value + 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, expr.position()) + 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(expr.position())); + .map_err(|err| err.fill_position(args_expr[0].position())); } } // Handle curry() if name == KEYWORD_FN_PTR_CURRY && args_expr.len() > 1 { - let expr = args_expr.get(0).unwrap(); - let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; if !fn_ptr.is::() { return Err(self.make_type_mismatch_err::( self.map_type_name(fn_ptr.type_name()), - expr.position(), + args_expr[0].position(), )); } let (fn_name, mut fn_curry) = fn_ptr.cast::().take_data(); // Append the new curried arguments to the existing list. + args_expr .iter() .skip(1) @@ -904,8 +902,7 @@ impl Engine { // Handle is_shared() #[cfg(not(feature = "no_closure"))] if name == KEYWORD_IS_SHARED && args_expr.len() == 1 { - let expr = args_expr.get(0).unwrap(); - let value = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let value = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; return Ok(value.is_shared().into()); } @@ -913,25 +910,24 @@ impl Engine { // Handle call() - Redirect function call let redirected; let mut args_expr = args_expr.as_ref(); - let mut curry: StaticVec<_> = Default::default(); + let mut curry = StaticVec::new(); let mut name = name; if name == KEYWORD_FN_PTR_CALL && args_expr.len() >= 1 && !self.has_override(lib, 0, hash_script, pub_only) { - let expr = args_expr.get(0).unwrap(); - let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; if !fn_ptr.is::() { return Err(self.make_type_mismatch_err::( self.map_type_name(fn_ptr.type_name()), - expr.position(), + args_expr[0].position(), )); } let fn_ptr = fn_ptr.cast::(); - curry = fn_ptr.curry().iter().cloned().collect(); + curry.extend(fn_ptr.curry().iter().cloned()); // Redirect function name redirected = fn_ptr.take_data().0; @@ -941,7 +937,8 @@ impl Engine { args_expr = &args_expr.as_ref()[1..]; // Recalculate hash - hash_script = calc_fn_hash(empty(), name, curry.len() + args_expr.len(), empty()); + let args_len = args_expr.len() + curry.len(); + hash_script = calc_fn_hash(empty(), name, args_len, empty()); } // Handle is_def_var() @@ -949,10 +946,10 @@ impl Engine { let hash_fn = calc_fn_hash(empty(), name, 1, once(TypeId::of::())); if !self.has_override(lib, hash_fn, hash_script, pub_only) { - let expr = args_expr.get(0).unwrap(); - let var_name = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + 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, expr.position()) + self.make_type_mismatch_err::(err, args_expr[0].position()) })?; if var_name.is_empty() { return Ok(false.into()); @@ -974,18 +971,17 @@ impl Engine { ); if !self.has_override(lib, hash_fn, hash_script, pub_only) { - let expr0 = args_expr.get(0).unwrap(); - let expr1 = args_expr.get(1).unwrap(); - - let fn_name = self.eval_expr(scope, mods, state, lib, this_ptr, expr0, level)?; - let num_params = self.eval_expr(scope, mods, state, lib, this_ptr, expr1, level)?; + let fn_name = + self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; + let num_params = + self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[1], level)?; let fn_name = fn_name.as_str().map_err(|err| { - self.make_type_mismatch_err::(err, expr0.position()) + self.make_type_mismatch_err::(err, args_expr[0].position()) + })?; + let num_params = num_params.as_int().map_err(|err| { + self.make_type_mismatch_err::(err, args_expr[1].position()) })?; - let num_params = num_params - .as_int() - .map_err(|err| self.make_type_mismatch_err::(err, expr1.position()))?; if fn_name.is_empty() || num_params < 0 { return Ok(false.into()); @@ -1003,14 +999,14 @@ impl Engine { if !self.has_override(lib, hash_fn, hash_script, pub_only) { // eval - only in function call style let prev_len = scope.len(); - let expr = args_expr.get(0).unwrap(); - let script = self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?; + let script = + self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; let script = script.as_str().map_err(|typ| { - self.make_type_mismatch_err::(typ, expr.position()) + self.make_type_mismatch_err::(typ, args_expr[0].position()) })?; let result = if !script.is_empty() { self.eval_script_expr(scope, mods, state, lib, script, level + 1) - .map_err(|err| err.fill_position(expr.position())) + .map_err(|err| err.fill_position(args_expr[0].position())) } else { Ok(().into()) }; @@ -1041,41 +1037,38 @@ impl Engine { } else { // If the first argument is a variable, and there is no curried arguments, convert to method-call style // in order to leverage potential &mut first argument and avoid cloning the value - match args_expr.get(0).unwrap() { + if args_expr[0].get_variable_access(false).is_some() && curry.is_empty() { // func(x, ...) -> x.func(...) - lhs @ Expr::Variable(_) if curry.is_empty() => { - arg_values = args_expr - .iter() - .skip(1) - .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)) - .collect::>()?; + arg_values = args_expr + .iter() + .skip(1) + .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)) + .collect::>()?; - let (target, _, _, pos) = - self.search_namespace(scope, mods, state, lib, this_ptr, lhs)?; + let (target, _, _, pos) = + self.search_namespace(scope, mods, state, lib, this_ptr, &args_expr[0])?; - self.inc_operations(state) - .map_err(|err| err.fill_position(pos))?; + self.inc_operations(state) + .map_err(|err| err.fill_position(pos))?; - args = if target.is_shared() || target.is_value() { - arg_values.insert(0, target.take_or_clone().flatten()); - arg_values.iter_mut().collect() - } else { - // Turn it into a method call only if the object is not shared and not a simple value - is_ref = true; - once(target.take_ref().unwrap()) - .chain(arg_values.iter_mut()) - .collect() - }; - } + args = if target.is_shared() || target.is_value() { + arg_values.insert(0, target.take_or_clone().flatten()); + arg_values.iter_mut().collect() + } else { + // Turn it into a method call only if the object is not shared and not a simple value + is_ref = true; + once(target.take_ref().unwrap()) + .chain(arg_values.iter_mut()) + .collect() + }; + } else { // func(..., ...) - _ => { - arg_values = args_expr - .iter() - .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)) - .collect::>()?; + arg_values = args_expr + .iter() + .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)) + .collect::>()?; - args = curry.iter_mut().chain(arg_values.iter_mut()).collect(); - } + args = curry.iter_mut().chain(arg_values.iter_mut()).collect(); } } @@ -1116,50 +1109,46 @@ impl Engine { // See if the first argument is a variable (not module-qualified). // If so, convert to method-call style in order to leverage potential // &mut first argument and avoid cloning the value - match args_expr.get(0).unwrap() { + if args_expr[0].get_variable_access(true).is_some() { // func(x, ...) -> x.func(...) - Expr::Variable(x) if x.1.is_none() => { - arg_values = args_expr - .iter() - .enumerate() - .map(|(i, expr)| { - // Skip the first argument - if i == 0 { - Ok(Default::default()) - } else { - self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) - } - }) - .collect::>()?; + arg_values = args_expr + .iter() + .enumerate() + .map(|(i, expr)| { + // Skip the first argument + if i == 0 { + Ok(Default::default()) + } else { + self.eval_expr(scope, mods, state, lib, this_ptr, expr, level) + } + }) + .collect::>()?; - // Get target reference to first argument - let var_expr = args_expr.get(0).unwrap(); - let (target, _, _, pos) = - self.search_scope_only(scope, mods, state, lib, this_ptr, var_expr)?; + // Get target reference to first argument + let (target, _, _, pos) = + self.search_scope_only(scope, mods, state, lib, this_ptr, &args_expr[0])?; - self.inc_operations(state) - .map_err(|err| err.fill_position(pos))?; - - if target.is_shared() || target.is_value() { - arg_values[0] = target.take_or_clone().flatten(); - args = arg_values.iter_mut().collect(); - } else { - let (first, rest) = arg_values.split_first_mut().unwrap(); - first_arg_value = Some(first); - args = once(target.take_ref().unwrap()) - .chain(rest.iter_mut()) - .collect(); - } - } - // func(..., ...) or func(mod::x, ...) - _ => { - arg_values = args_expr - .iter() - .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)) - .collect::>()?; + self.inc_operations(state) + .map_err(|err| err.fill_position(pos))?; + if target.is_shared() || target.is_value() { + arg_values[0] = target.take_or_clone().flatten(); args = arg_values.iter_mut().collect(); + } else { + let (first, rest) = arg_values.split_first_mut().unwrap(); + first_arg_value = Some(first); + args = once(target.take_ref().unwrap()) + .chain(rest.iter_mut()) + .collect(); } + } else { + // func(..., ...) or func(mod::x, ...) + arg_values = args_expr + .iter() + .map(|expr| self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)) + .collect::>()?; + + args = arg_values.iter_mut().collect(); } } diff --git a/src/fn_native.rs b/src/fn_native.rs index eabc19e3..7ced25a9 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -11,9 +11,7 @@ use crate::token::{is_valid_identifier, Position}; use crate::utils::ImmutableString; use crate::{calc_fn_hash, StaticVec}; -use crate::stdlib::{ - boxed::Box, convert::TryFrom, fmt, iter::empty, mem, string::String, vec::Vec, -}; +use crate::stdlib::{boxed::Box, convert::TryFrom, fmt, iter::empty, mem, string::String}; #[cfg(feature = "sync")] use crate::stdlib::sync::{Arc, RwLock}; @@ -79,12 +77,15 @@ pub type FnCallArgs<'a> = [&'a mut Dynamic]; /// A general function pointer, which may carry additional (i.e. curried) argument values /// to be passed onto a function during a call. #[derive(Debug, Clone, Default)] -pub struct FnPtr(ImmutableString, Vec); +pub struct FnPtr(ImmutableString, StaticVec); impl FnPtr { /// Create a new function pointer. #[inline(always)] - pub(crate) fn new_unchecked>(name: S, curry: Vec) -> Self { + pub(crate) fn new_unchecked>( + name: S, + curry: StaticVec, + ) -> Self { Self(name.into(), curry) } /// Get the name of the function. @@ -99,13 +100,13 @@ impl FnPtr { } /// Get the underlying data of the function pointer. #[inline(always)] - pub(crate) fn take_data(self) -> (ImmutableString, Vec) { + pub(crate) fn take_data(self) -> (ImmutableString, StaticVec) { (self.0, self.1) } /// Get the curried arguments. #[inline(always)] pub fn curry(&self) -> &[Dynamic] { - &self.1 + self.1.as_ref() } /// Call the function pointer with curried arguments (if any). @@ -125,24 +126,24 @@ impl FnPtr { this_ptr: Option<&mut Dynamic>, mut arg_values: impl AsMut<[Dynamic]>, ) -> Result> { + let arg_values = arg_values.as_mut(); + let fn_name = self.fn_name(); + let mut args_data = self - .1 + .curry() .iter() .cloned() - .chain(arg_values.as_mut().iter_mut().map(|v| mem::take(v))) + .chain(arg_values.iter_mut().map(mem::take)) .collect::>(); let has_this = this_ptr.is_some(); - let args_len = args_data.len(); let mut args = args_data.iter_mut().collect::>(); + let hash_script = calc_fn_hash(empty(), fn_name, args.len(), empty()); if let Some(obj) = this_ptr { args.insert(0, obj); } - let fn_name = self.0.as_str(); - let hash_script = calc_fn_hash(empty(), fn_name, args_len, empty()); - engine .exec_fn_call( &mut Default::default(), diff --git a/src/module/mod.rs b/src/module/mod.rs index d7cc40d9..5ad7c3a5 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -425,6 +425,30 @@ impl Module { } } + /// Does the particular Rust function exist in the module? + /// + /// The `u64` hash is calculated by the function `crate::calc_fn_hash`. + /// It is also returned by the `set_fn_XXX` calls. + /// + /// # Examples + /// + /// ``` + /// use rhai::Module; + /// + /// let mut module = Module::new(); + /// module.set_fn_0("calc", || Ok(42_i64)); + /// assert!(module.contains_fn_with_name("calc", true)); + /// ``` + #[inline] + #[allow(dead_code)] + pub(crate) fn contains_fn_with_name(&self, fn_name: &str, public_only: bool) -> bool { + self.functions + .values() + .filter(|(_, access, _, _, _)| !public_only || access.is_public()) + .find(|(name, _, _, _, _)| name == fn_name) + .is_some() + } + /// Set a Rust function into the module, returning a hash key. /// /// If there is an existing Rust function of the same hash, it is replaced. diff --git a/src/packages/mod.rs b/src/packages/mod.rs index d7c7d023..1bc33737 100644 --- a/src/packages/mod.rs +++ b/src/packages/mod.rs @@ -65,6 +65,13 @@ impl PackagesCollection { pub fn contains_fn(&self, hash: u64, public_only: bool) -> bool { self.0.iter().any(|p| p.contains_fn(hash, public_only)) } + /// Does the specified function name exist in the `PackagesCollection`? + #[allow(dead_code)] + pub fn contains_fn_with_name(&self, fn_name: &str, public_only: bool) -> bool { + self.0 + .iter() + .any(|p| p.contains_fn_with_name(fn_name, public_only)) + } /// Get specified function via its hash key. pub fn get_fn(&self, hash: u64, public_only: bool) -> Option<&CallableFunction> { self.0 diff --git a/src/parser.rs b/src/parser.rs index 77b460f7..55d97082 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1079,6 +1079,14 @@ impl Expr { }) } + /// Is the expression a simple variable access? + pub(crate) fn get_variable_access(&self, non_qualified: bool) -> Option<&str> { + match self { + Self::Variable(x) if !non_qualified || x.1.is_none() => Some((x.0).0.as_str()), + _ => None, + } + } + /// Get the `Position` of the expression. pub fn position(&self) -> Position { match self { diff --git a/src/syntax.rs b/src/syntax.rs index f0372649..5e827105 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -41,10 +41,7 @@ impl Expression<'_> { /// If this expression is a variable name, return it. Otherwise `None`. #[inline(always)] pub fn get_variable_name(&self) -> Option<&str> { - match self.0 { - Expr::Variable(x) => Some((x.0).0.as_str()), - _ => None, - } + self.0.get_variable_access(true) } /// Get the expression. #[inline(always)]