diff --git a/CHANGELOG.md b/CHANGELOG.md index 691836c9..c18bde75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,12 @@ Speed improvements Net features ------------ +### First class functions (sort of) + +* A function pointer created via a closure definition now links to the particular anonymous function itself. +* This avoids a potentially expensive function lookup when the function pointer is called, speeding up closures. +* It does _not_, however, allow the function pointer to be `export`ed as a constant from a script module because the closure may cross-call other functions defined in the module and the function pointer won't keep the fully encapsulated environment. + ### `!in` * A new operator `!in` is added which maps to `!(... in ...)`. diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 75e0520c..b0e6d518 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -152,20 +152,18 @@ impl Engine { _ if global.always_search_scope => 0, Expr::Variable(_, Some(i), ..) => i.get() as usize, - // Scripted function with the same name - #[cfg(not(feature = "no_function"))] - Expr::Variable(v, None, ..) - if global - .lib - .iter() - .flat_map(|m| m.iter_script_fn()) - .any(|(_, _, f, ..)| f == v.3.as_str()) => - { - let val: Dynamic = - crate::FnPtr::new_unchecked(v.3.as_str(), crate::StaticVec::default()).into(); - return Ok(val.into()); + Expr::Variable(v, None, ..) => { + // Scripted function with the same name + #[cfg(not(feature = "no_function"))] + if let Some(fn_def) = global.lib.iter().flat_map(|m| m.iter_script_fn()).find_map( + |(_, _, f, _, func)| if f == v.3.as_str() { Some(func) } else { None }, + ) { + let val: Dynamic = crate::FnPtr::from(fn_def.clone()).into(); + return Ok(val.into()); + } + + v.0.map_or(0, NonZeroUsize::get) } - Expr::Variable(v, None, ..) => v.0.map_or(0, NonZeroUsize::get), _ => unreachable!("Expr::Variable expected but gets {:?}", expr), }; diff --git a/src/func/call.rs b/src/func/call.rs index a01d283d..884a3d33 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -747,10 +747,36 @@ impl Engine { let is_ref_mut = target.is_ref(); let (result, updated) = match fn_name { + // Handle fn_ptr.call(...) KEYWORD_FN_PTR_CALL if target.is_fnptr() => { - // FnPtr call let fn_ptr = target.read_lock::().expect("`FnPtr`"); + // Arguments are passed as-is, adding the curried arguments + let mut curry = FnArgsVec::with_capacity(fn_ptr.curry().len()); + curry.extend(fn_ptr.curry().iter().cloned()); + let args = &mut FnArgsVec::with_capacity(curry.len() + call_args.len()); + args.extend(curry.iter_mut()); + args.extend(call_args.iter_mut()); + + // Linked to scripted function? + #[cfg(not(feature = "no_function"))] + if let Some(fn_def) = fn_ptr.fn_def() { + let mut this_ptr = Dynamic::NULL; + + return self + .call_script_fn( + global, + caches, + &mut Scope::new(), + &mut this_ptr, + fn_def, + args, + true, + fn_call_pos, + ) + .map(|v| (v, false)); + } + #[cfg(not(feature = "no_function"))] let is_anon = fn_ptr.is_anonymous(); #[cfg(feature = "no_function")] @@ -759,18 +785,11 @@ impl Engine { // Redirect function name let fn_name = fn_ptr.fn_name(); // Recalculate hashes - let args_len = call_args.len() + fn_ptr.curry().len(); let new_hash = if !is_anon && !is_valid_function_name(fn_name) { - FnCallHashes::from_native(calc_fn_hash(None, fn_name, args_len)) + FnCallHashes::from_native(calc_fn_hash(None, fn_name, args.len())) } else { - calc_fn_hash(None, fn_name, args_len).into() + calc_fn_hash(None, fn_name, args.len()).into() }; - // Arguments are passed as-is, adding the curried arguments - let mut curry = FnArgsVec::with_capacity(fn_ptr.curry().len()); - curry.extend(fn_ptr.curry().iter().cloned()); - let mut args = FnArgsVec::with_capacity(curry.len() + call_args.len()); - args.extend(curry.iter_mut()); - args.extend(call_args.iter_mut()); // Map it to name(args) in function-call style self.exec_fn_call( @@ -780,12 +799,14 @@ impl Engine { fn_name, NO_TOKEN, new_hash, - &mut args, + args, false, false, fn_call_pos, ) } + + // Handle obj.call(fn_ptr, ...) KEYWORD_FN_PTR_CALL => { if call_args.is_empty() { let typ = self.map_type_name(target.type_name()); @@ -799,32 +820,60 @@ impl Engine { let fn_ptr = mem::take(&mut call_args[0]).cast::(); #[cfg(not(feature = "no_function"))] - let is_anon = fn_ptr.is_anonymous(); + let (fn_name, is_anon, fn_curry, fn_def) = { + let is_anon = fn_ptr.is_anonymous(); + let (fn_name, fn_curry, fn_def) = fn_ptr.take_data(); + (fn_name, is_anon, fn_curry, fn_def) + }; #[cfg(feature = "no_function")] - let is_anon = false; + let (fn_name, is_anon, fn_curry) = { + let (fn_name, fn_curry, fn_def) = fn_ptr.take_data(); + (fn_name, false, fn_curry) + }; + // Replace the first argument with the object pointer, adding the curried arguments call_args = &mut call_args[1..]; - // Redirect function name - let (fn_name, fn_curry) = fn_ptr.take_data(); + let mut curry = FnArgsVec::with_capacity(fn_curry.len()); + curry.extend(fn_curry.into_iter()); + let args = &mut FnArgsVec::with_capacity(curry.len() + call_args.len() + 1); + args.extend(curry.iter_mut()); + args.extend(call_args.iter_mut()); + + // Linked to scripted function? + #[cfg(not(feature = "no_function"))] + if let Some(fn_def) = fn_def { + // Check for data race. + #[cfg(not(feature = "no_closure"))] + ensure_no_data_race(&fn_def.name, args, false)?; + + return self + .call_script_fn( + global, + caches, + &mut Scope::new(), + target, + &fn_def, + args, + true, + fn_call_pos, + ) + .map(|v| (v, false)); + } + + // Add the first argument with the object pointer + args.insert(0, target.as_mut()); + // Recalculate hash - let args_len = call_args.len() + fn_curry.len(); let new_hash = if !is_anon && !is_valid_function_name(&fn_name) { - FnCallHashes::from_native(calc_fn_hash(None, &fn_name, args_len + 1)) + FnCallHashes::from_native(calc_fn_hash(None, &fn_name, args.len())) } else { FnCallHashes::from_all( #[cfg(not(feature = "no_function"))] - calc_fn_hash(None, &fn_name, args_len), - calc_fn_hash(None, &fn_name, args_len + 1), + calc_fn_hash(None, &fn_name, args.len() - 1), + calc_fn_hash(None, &fn_name, args.len()), ) }; - // Replace the first argument with the object pointer, adding the curried arguments - let mut curry = FnArgsVec::with_capacity(fn_curry.len()); - curry.extend(fn_curry.into_iter()); - let mut args = FnArgsVec::with_capacity(curry.len() + call_args.len() + 1); - args.push(target.as_mut()); - args.extend(curry.iter_mut()); - args.extend(call_args.iter_mut()); // Map it to name(args) in function-call style self.exec_fn_call( @@ -834,7 +883,7 @@ impl Engine { &fn_name, NO_TOKEN, new_hash, - &mut args, + args, is_ref_mut, true, fn_call_pos, @@ -975,7 +1024,7 @@ impl Engine { match name { _ if op_token != NO_TOKEN => (), - // Handle call() + // Handle call(fn_ptr, ...) KEYWORD_FN_PTR_CALL if total_args >= 1 => { let arg = first_arg.unwrap(); let (arg_value, arg_pos) = @@ -989,13 +1038,47 @@ impl Engine { let fn_ptr = arg_value.cast::(); #[cfg(not(feature = "no_function"))] - let is_anon = fn_ptr.is_anonymous(); + let (fn_name, is_anon, fn_curry, fn_def) = { + let is_anon = fn_ptr.is_anonymous(); + let (fn_name, fn_curry, fn_def) = fn_ptr.take_data(); + (fn_name, is_anon, fn_curry, fn_def) + }; #[cfg(feature = "no_function")] - let is_anon = false; + let (fn_name, is_anon, fn_curry) = { + let (fn_name, fn_curry, fn_def) = fn_ptr.take_data(); + (fn_name, false, fn_curry) + }; - let (fn_name, fn_curry) = fn_ptr.take_data(); curry.extend(fn_curry.into_iter()); + // Linked to scripted function? + #[cfg(not(feature = "no_function"))] + if let Some(fn_def) = fn_def { + // Evaluate arguments + let mut arg_values = curry + .into_iter() + .map(Ok) + .chain(a_expr.iter().map(|expr| -> Result<_, RhaiError> { + self.get_arg_value(global, caches, scope, this_ptr, expr) + .map(|(v, ..)| v) + })) + .collect::>>()?; + let args = &mut arg_values.iter_mut().collect::>(); + + let mut this_ptr = Dynamic::NULL; + + return self.call_script_fn( + global, + caches, + &mut Scope::new(), + &mut this_ptr, + &fn_def, + args, + true, + pos, + ); + } + // Redirect function name redirected = fn_name; name = &redirected; @@ -1042,16 +1125,16 @@ impl Engine { return Err(self.make_type_mismatch_err::(typ, arg_pos)); } - let (name, fn_curry) = arg_value.cast::().take_data(); + let mut fn_ptr = arg_value.cast::(); // Append the new curried arguments to the existing list. - let fn_curry = a_expr.iter().try_fold(fn_curry, |mut curried, expr| { + a_expr.iter().try_for_each(|expr| -> Result<_, RhaiError> { let (value, ..) = self.get_arg_value(global, caches, scope, this_ptr, expr)?; - curried.push(value); - Ok::<_, RhaiError>(curried) + fn_ptr.add_curry(value); + Ok(()) })?; - return Ok(FnPtr::new_unchecked(name, fn_curry).into()); + return Ok(fn_ptr.into()); } // Handle is_shared() diff --git a/src/func/script.rs b/src/func/script.rs index 14f83f04..ea7cb0f7 100644 --- a/src/func/script.rs +++ b/src/func/script.rs @@ -33,7 +33,7 @@ impl Engine { rewind_scope: bool, pos: Position, ) -> RhaiResult { - assert!(fn_def.params.len() == args.len()); + assert_eq!(fn_def.params.len(), args.len()); self.track_operation(global, pos)?; diff --git a/src/module/mod.rs b/src/module/mod.rs index 2cc8d04b..24ac305b 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -2140,7 +2140,11 @@ impl Module { // Variables with an alias left in the scope become module variables for (_name, value, mut aliases) in scope { - // It is an error to export function pointers that refer to encapsulated local functions + // It is an error to export function pointers that refer to encapsulated local functions. + // + // Even if the function pointer already links to a scripted function definition, it may + // cross-call other functions inside the module and won't have the full encapsulated + // environment available. #[cfg(not(feature = "no_function"))] if let Some(fn_ptr) = value.downcast_ref::() { if ast.iter_fn_def().any(|f| f.name == fn_ptr.fn_name()) { diff --git a/src/parser.rs b/src/parser.rs index 3a28f38a..a046283f 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1464,7 +1464,7 @@ impl Engine { // Restore the strings interner by swapping it back std::mem::swap(state.interned_strings, new_state.interned_strings); - let (expr, f) = result?; + let (expr, fn_def) = result?; #[cfg(not(feature = "no_closure"))] new_state @@ -1490,8 +1490,8 @@ impl Engine { } })?; - let hash_script = calc_fn_hash(None, &f.name, f.params.len()); - lib.insert(hash_script, f.into()); + let hash_script = calc_fn_hash(None, &fn_def.name, fn_def.params.len()); + lib.insert(hash_script, fn_def); expr } @@ -3707,7 +3707,7 @@ impl Engine { _parent: &mut ParseState, lib: &mut FnLib, settings: ParseSettings, - ) -> ParseResult<(Expr, ScriptFnDef)> { + ) -> ParseResult<(Expr, Shared)> { let settings = settings.level_up()?; let mut params_list = StaticVec::::new_const(); @@ -3786,7 +3786,7 @@ impl Engine { let fn_name = state.get_interned_string(make_anonymous_fn(hash)); // Define the function - let script = ScriptFnDef { + let script = Shared::new(ScriptFnDef { name: fn_name.clone(), access: crate::FnAccess::Public, params, @@ -3796,9 +3796,9 @@ impl Engine { #[cfg(not(feature = "no_function"))] #[cfg(feature = "metadata")] comments: Box::default(), - }; + }); - let fn_ptr = crate::FnPtr::new_unchecked(fn_name, StaticVec::new_const()); + let fn_ptr = crate::FnPtr::from(script.clone()); let expr = Expr::DynamicConstant(Box::new(fn_ptr.into()), settings.pos); #[cfg(not(feature = "no_closure"))] diff --git a/src/tests.rs b/src/tests.rs index 01a0e8e7..f80e3e49 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -35,7 +35,14 @@ fn check_struct_sizes() { #[cfg(target_pointer_width = "64")] { assert_eq!(size_of::(), 536); - assert_eq!(size_of::(), 64); + assert_eq!( + size_of::(), + if cfg!(feature = "no_function") { + 64 + } else { + 72 + } + ); assert_eq!(size_of::(), 56); assert_eq!( size_of::(), diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index 7047da66..76b8f52c 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -12,15 +12,34 @@ use std::prelude::v1::*; use std::{ any::{type_name, TypeId}, convert::{TryFrom, TryInto}, - fmt, mem, + fmt, + hash::Hash, + mem, }; /// A general function pointer, which may carry additional (i.e. curried) argument values /// to be passed onto a function during a call. -#[derive(Clone, Hash)] +#[derive(Clone)] pub struct FnPtr { name: ImmutableString, curry: StaticVec, + #[cfg(not(feature = "no_function"))] + fn_def: Option>, +} + +impl Hash for FnPtr { + #[inline(always)] + fn hash(&self, state: &mut H) { + self.name.hash(state); + self.curry.hash(state); + + // Hash the linked [`ScriptFnDef`][crate::ast::ScriptFnDef] by hashing its shared pointer. + #[cfg(not(feature = "no_function"))] + self.fn_def + .as_ref() + .map(|f| crate::Shared::as_ptr(f)) + .hash(state); + } } impl fmt::Debug for FnPtr { @@ -56,6 +75,8 @@ impl FnPtr { Self { name: name.into(), curry, + #[cfg(not(feature = "no_function"))] + fn_def: None, } } /// Get the name of the function. @@ -71,6 +92,20 @@ impl FnPtr { &self.name } /// Get the underlying data of the function pointer. + #[cfg(not(feature = "no_function"))] + #[inline(always)] + #[must_use] + pub(crate) fn take_data( + self, + ) -> ( + ImmutableString, + StaticVec, + Option>, + ) { + (self.name, self.curry, self.fn_def) + } + /// Get the underlying data of the function pointer. + #[cfg(feature = "no_function")] #[inline(always)] #[must_use] pub(crate) fn take_data(self) -> (ImmutableString, StaticVec) { @@ -247,6 +282,13 @@ impl FnPtr { context.call_fn_raw(self.fn_name(), is_method, is_method, &mut args) } + /// Get a reference to the linked [`ScriptFnDef`][crate::ast::ScriptFnDef]. + #[cfg(not(feature = "no_function"))] + #[inline(always)] + #[must_use] + pub(crate) fn fn_def(&self) -> Option<&crate::Shared> { + self.fn_def.as_ref() + } } impl fmt::Display for FnPtr { @@ -264,9 +306,25 @@ impl TryFrom for FnPtr { Ok(Self { name: value, curry: StaticVec::new_const(), + #[cfg(not(feature = "no_function"))] + fn_def: None, }) } else { Err(ERR::ErrorFunctionNotFound(value.to_string(), Position::NONE).into()) } } } + +#[cfg(not(feature = "no_function"))] +impl>> From for FnPtr { + #[inline(always)] + fn from(value: T) -> Self { + let fn_def = value.into(); + + Self { + name: fn_def.name.clone(), + curry: StaticVec::new_const(), + fn_def: Some(fn_def), + } + } +}