From 423285cf1cbd86d85db066dbfe26de4fd68333d9 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Fri, 16 Aug 2024 17:20:47 -0700 Subject: [PATCH] bevy_reflect: Store functions as `DynamicClosure<'static>` in `FunctionRegistry` (#14704) # Objective #14098 added the `FunctionRegistry` for registering functions such that they can be retrieved by name and used dynamically. One thing we chose to leave out in that initial PR is support for closures. Why support closures? Mainly, we don't want to prohibit users from injecting environmental data into their registered functions. This allows these functions to not leak their internals to the public API. For example, let's say we're writing a library crate that allows users to register callbacks for certain actions. We want to perform some actions before invoking the user's callback so we can't just call it directly. We need a closure for this: ```rust registry.register("my_lib::onclick", move |event: ClickEvent| { // ...other work... user_onclick.call(event); // <-- Captured variable }); ``` We could have made our callback take a reference to the user's callback. This would remove the need for the closure, but it would change our desired API to place the burden of fetching the correct callback on the caller. ## Solution Modify the `FunctionRegistry` to store registered functions as `DynamicClosure<'static>` instead of `DynamicFunction` (now using `IntoClosure` instead of `IntoFunction`). Due to limitations in Rust and how function reflection works, `DynamicClosure<'static>` is functionally equivalent to `DynamicFunction`. And a normal function is considered a subset of closures (it's a closure that doesn't capture anything), so there shouldn't be any difference in usage: all functions that satisfy `IntoFunction` should satisfy `IntoClosure`. This means that the registration API introduced in #14098 should require little-to-no changes on anyone following `main`. ### Closures vs Functions One consideration here is whether we should keep closures and functions separate. This PR unifies them into `DynamicClosure<'static>`, but we can consider splitting them up. The reasons we might want to do so are: - Simplifies mental model and terminology (users don't have to understand that functions turn into closures) - If Rust ever improves its function model, we may be able to add additional guarantees to `DynamicFunction` that make it useful to separate the two - Adding support for generic functions may be less confusing for users since closures in Rust technically can't be generic The reasons behind this PR's unification approach are: - Reduces the number of methods needed on `FunctionRegistry` - Reduces the number of lookups a user may have to perform (i.e. "`get_function` or else `get_closure`") - Establishes `DynamicClosure<'static>` as the de facto dynamic callable (similar to how most APIs in Rust code tend to prefer `impl Fn() -> String` over `fn() -> String`) I'd love to hear feedback on this matter, and whether we should continue with this PR's approach or switch to a split model. ## Testing You can test locally by running: ``` cargo test --package bevy_reflect ``` --- ## Showcase Closures can now be registered into the `FunctionRegistry`: ```rust let punct = String::from("!!!"); registry.register_with_name("my_crate::punctuate", move |text: String| { format!("{}{}", text, punct) }); ``` --- crates/bevy_app/src/app.rs | 32 ++-- crates/bevy_app/src/sub_app.rs | 4 +- .../src/func/closures/dynamic_closure.rs | 69 ++++++-- .../src/func/closures/dynamic_closure_mut.rs | 25 ++- .../src/func/closures/into_closure.rs | 2 +- crates/bevy_reflect/src/func/function.rs | 24 ++- crates/bevy_reflect/src/func/registry.rs | 151 +++++++++++------- 7 files changed, 212 insertions(+), 95 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 7188102837dea..d1fa331a23101 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -605,16 +605,16 @@ impl App { /// Registers the given function into the [`AppFunctionRegistry`] resource. /// - /// The given function will internally be stored as a [`DynamicFunction`] + /// The given function will internally be stored as a [`DynamicClosure`] /// and mapped according to its [name]. /// /// Because the function must have a name, - /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) must instead - /// be registered using [`register_function_with_name`] or converted to a [`DynamicFunction`] - /// and named using [`DynamicFunction::with_name`]. - /// Failure to do so will result in an error being returned. + /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) and closures must instead + /// be registered using [`register_function_with_name`] or converted to a [`DynamicClosure`] + /// and named using [`DynamicClosure::with_name`]. + /// Failure to do so will result in a panic. /// - /// Only functions that implement [`IntoFunction`] may be registered via this method. + /// Only types that implement [`IntoClosure`] may be registered via this method. /// /// See [`FunctionRegistry::register`] for more information. /// @@ -650,7 +650,7 @@ impl App { /// .register_function(add); /// ``` /// - /// Anonymous functions should be registered using [`register_function_with_name`] or given a name using [`DynamicFunction::with_name`]. + /// Anonymous functions and closures should be registered using [`register_function_with_name`] or given a name using [`DynamicClosure::with_name`]. /// /// ```should_panic /// use bevy_app::App; @@ -660,21 +660,21 @@ impl App { /// ``` /// /// [`register_function_with_name`]: Self::register_function_with_name - /// [`DynamicFunction`]: bevy_reflect::func::DynamicFunction + /// [`DynamicClosure`]: bevy_reflect::func::DynamicClosure /// [name]: bevy_reflect::func::FunctionInfo::name - /// [`DynamicFunction::with_name`]: bevy_reflect::func::DynamicFunction::with_name - /// [`IntoFunction`]: bevy_reflect::func::IntoFunction + /// [`DynamicClosure::with_name`]: bevy_reflect::func::DynamicClosure::with_name + /// [`IntoClosure`]: bevy_reflect::func::IntoClosure /// [`FunctionRegistry::register`]: bevy_reflect::func::FunctionRegistry::register #[cfg(feature = "reflect_functions")] pub fn register_function(&mut self, function: F) -> &mut Self where - F: bevy_reflect::func::IntoFunction + 'static, + F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static, { self.main_mut().register_function(function); self } - /// Registers the given function into the [`AppFunctionRegistry`] resource using the given name. + /// Registers the given function or closure into the [`AppFunctionRegistry`] resource using the given name. /// /// To avoid conflicts, it's recommended to use a unique name for the function. /// This can be achieved by "namespacing" the function with a unique identifier, @@ -689,7 +689,7 @@ impl App { /// For named functions (e.g. `fn add(a: i32, b: i32) -> i32 { a + b }`) where a custom name is not needed, /// it's recommended to use [`register_function`] instead as the generated name is guaranteed to be unique. /// - /// Only functions that implement [`IntoFunction`] may be registered via this method. + /// Only types that implement [`IntoClosure`] may be registered via this method. /// /// See [`FunctionRegistry::register_with_name`] for more information. /// @@ -718,7 +718,7 @@ impl App { /// // Registering an existing function with a custom name /// .register_function_with_name("my_crate::mul", mul) /// // Be careful not to register anonymous functions with their type name. - /// // This code works but registers the function with the non-unique name of `fn(i32, i32) -> i32` + /// // This code works but registers the function with a non-unique name like `foo::bar::{{closure}}` /// .register_function_with_name(std::any::type_name_of_val(&div), div); /// ``` /// @@ -738,7 +738,7 @@ impl App { /// /// [type name]: std::any::type_name /// [`register_function`]: Self::register_function - /// [`IntoFunction`]: bevy_reflect::func::IntoFunction + /// [`IntoClosure`]: bevy_reflect::func::IntoClosure /// [`FunctionRegistry::register_with_name`]: bevy_reflect::func::FunctionRegistry::register_with_name #[cfg(feature = "reflect_functions")] pub fn register_function_with_name( @@ -747,7 +747,7 @@ impl App { function: F, ) -> &mut Self where - F: bevy_reflect::func::IntoFunction + 'static, + F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static, { self.main_mut().register_function_with_name(name, function); self diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index ff9eb5d2cb5e7..fc91ca2f34d89 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -413,7 +413,7 @@ impl SubApp { #[cfg(feature = "reflect_functions")] pub fn register_function(&mut self, function: F) -> &mut Self where - F: bevy_reflect::func::IntoFunction + 'static, + F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static, { let registry = self.world.resource_mut::(); registry.write().register(function).unwrap(); @@ -428,7 +428,7 @@ impl SubApp { function: F, ) -> &mut Self where - F: bevy_reflect::func::IntoFunction + 'static, + F: bevy_reflect::func::IntoClosure<'static, Marker> + 'static, { let registry = self.world.resource_mut::(); registry.write().register_with_name(name, function).unwrap(); diff --git a/crates/bevy_reflect/src/func/closures/dynamic_closure.rs b/crates/bevy_reflect/src/func/closures/dynamic_closure.rs index 159e4c30eb64d..f49645f70f4a7 100644 --- a/crates/bevy_reflect/src/func/closures/dynamic_closure.rs +++ b/crates/bevy_reflect/src/func/closures/dynamic_closure.rs @@ -1,9 +1,11 @@ -use alloc::borrow::Cow; -use core::fmt::{Debug, Formatter}; - use crate::func::args::{ArgInfo, ArgList}; use crate::func::info::FunctionInfo; -use crate::func::{FunctionResult, IntoClosure, ReturnInfo}; +use crate::func::{ + DynamicClosureMut, DynamicFunction, FunctionResult, IntoClosure, IntoClosureMut, ReturnInfo, +}; +use alloc::borrow::Cow; +use core::fmt::{Debug, Formatter}; +use std::sync::Arc; /// A dynamic representation of a Rust closure. /// @@ -42,12 +44,9 @@ use crate::func::{FunctionResult, IntoClosure, ReturnInfo}; /// // Check the result: /// assert_eq!(value.try_take::().unwrap(), "Hello, world!!!"); /// ``` -/// -/// [`DynamicClosureMut`]: crate::func::closures::DynamicClosureMut -/// [`DynamicFunction`]: crate::func::DynamicFunction pub struct DynamicClosure<'env> { - info: FunctionInfo, - func: Box Fn(ArgList<'a>) -> FunctionResult<'a> + 'env>, + pub(super) info: FunctionInfo, + pub(super) func: Arc Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'env>, } impl<'env> DynamicClosure<'env> { @@ -57,13 +56,13 @@ impl<'env> DynamicClosure<'env> { /// /// It's important that the closure signature matches the provided [`FunctionInfo`]. /// This info may be used by consumers of the function for validation and debugging. - pub fn new Fn(ArgList<'a>) -> FunctionResult<'a> + 'env>( + pub fn new Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'env>( func: F, info: FunctionInfo, ) -> Self { Self { info, - func: Box::new(func), + func: Arc::new(func), } } @@ -160,6 +159,25 @@ impl<'env> Debug for DynamicClosure<'env> { } } +impl<'env> Clone for DynamicClosure<'env> { + fn clone(&self) -> Self { + Self { + info: self.info.clone(), + func: Arc::clone(&self.func), + } + } +} + +impl From for DynamicClosure<'static> { + #[inline] + fn from(func: DynamicFunction) -> Self { + Self { + info: func.info, + func: func.func, + } + } +} + impl<'env> IntoClosure<'env, ()> for DynamicClosure<'env> { #[inline] fn into_closure(self) -> DynamicClosure<'env> { @@ -167,6 +185,13 @@ impl<'env> IntoClosure<'env, ()> for DynamicClosure<'env> { } } +impl<'env> IntoClosureMut<'env, ()> for DynamicClosure<'env> { + #[inline] + fn into_closure_mut(self) -> DynamicClosureMut<'env> { + DynamicClosureMut::from(self) + } +} + #[cfg(test)] mod tests { use super::*; @@ -190,4 +215,26 @@ mod tests { let closure: DynamicClosure = make_closure(|a: i32, b: i32| a + b + c); let _: DynamicClosure = make_closure(closure); } + + #[test] + fn should_clone_dynamic_closure() { + let hello = String::from("Hello"); + + let greet = |name: &String| -> String { format!("{}, {}!", hello, name) }; + + let greet = greet.into_closure().with_name("greet"); + let clone = greet.clone(); + + assert_eq!(greet.name().unwrap(), "greet"); + assert_eq!(clone.name().unwrap(), "greet"); + + let clone_value = clone + .call(ArgList::default().push_ref(&String::from("world"))) + .unwrap() + .unwrap_owned() + .try_take::() + .unwrap(); + + assert_eq!(clone_value, "Hello, world!"); + } } diff --git a/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs b/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs index e5f1a33c19709..b85521d939ba1 100644 --- a/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs +++ b/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs @@ -3,7 +3,7 @@ use core::fmt::{Debug, Formatter}; use crate::func::args::{ArgInfo, ArgList}; use crate::func::info::FunctionInfo; -use crate::func::{FunctionResult, IntoClosureMut, ReturnInfo}; +use crate::func::{DynamicClosure, DynamicFunction, FunctionResult, IntoClosureMut, ReturnInfo}; /// A dynamic representation of a Rust closure. /// @@ -51,9 +51,6 @@ use crate::func::{FunctionResult, IntoClosureMut, ReturnInfo}; /// drop(func); /// assert_eq!(list, vec![1, -2, 3]); /// ``` -/// -/// [`DynamicClosure`]: crate::func::closures::DynamicClosure -/// [`DynamicFunction`]: crate::func::DynamicFunction pub struct DynamicClosureMut<'env> { info: FunctionInfo, func: Box FnMut(ArgList<'a>) -> FunctionResult<'a> + 'env>, @@ -202,6 +199,26 @@ impl<'env> Debug for DynamicClosureMut<'env> { } } +impl From for DynamicClosureMut<'static> { + #[inline] + fn from(func: DynamicFunction) -> Self { + Self { + info: func.info, + func: Box::new(move |args| (func.func)(args)), + } + } +} + +impl<'env> From> for DynamicClosureMut<'env> { + #[inline] + fn from(closure: DynamicClosure<'env>) -> Self { + Self { + info: closure.info, + func: Box::new(move |args| (closure.func)(args)), + } + } +} + impl<'env> IntoClosureMut<'env, ()> for DynamicClosureMut<'env> { #[inline] fn into_closure_mut(self) -> DynamicClosureMut<'env> { diff --git a/crates/bevy_reflect/src/func/closures/into_closure.rs b/crates/bevy_reflect/src/func/closures/into_closure.rs index 6c871c281c2f8..f3cba4a5d1b8d 100644 --- a/crates/bevy_reflect/src/func/closures/into_closure.rs +++ b/crates/bevy_reflect/src/func/closures/into_closure.rs @@ -18,7 +18,7 @@ pub trait IntoClosure<'env, Marker> { impl<'env, F, Marker1, Marker2> IntoClosure<'env, (Marker1, Marker2)> for F where - F: ReflectFn<'env, Marker1> + TypedFunction + 'env, + F: ReflectFn<'env, Marker1> + TypedFunction + Send + Sync + 'env, { fn into_closure(self) -> DynamicClosure<'env> { DynamicClosure::new(move |args| self.reflect_call(args), Self::function_info()) diff --git a/crates/bevy_reflect/src/func/function.rs b/crates/bevy_reflect/src/func/function.rs index d2d38e422b8ce..28c1ae3ad848e 100644 --- a/crates/bevy_reflect/src/func/function.rs +++ b/crates/bevy_reflect/src/func/function.rs @@ -4,7 +4,10 @@ use std::sync::Arc; use crate::func::args::{ArgInfo, ArgList}; use crate::func::info::FunctionInfo; -use crate::func::{FunctionResult, IntoFunction, ReturnInfo}; +use crate::func::{ + DynamicClosure, DynamicClosureMut, FunctionResult, IntoClosure, IntoClosureMut, IntoFunction, + ReturnInfo, +}; /// A dynamic representation of a Rust function. /// @@ -88,11 +91,10 @@ use crate::func::{FunctionResult, IntoFunction, ReturnInfo}; /// assert_eq!(list, vec!["Hello, World!!!"]); /// ``` /// -/// [`DynamicClosure`]: crate::func::DynamicClosure /// [module-level documentation]: crate::func pub struct DynamicFunction { - info: FunctionInfo, - func: Arc Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'static>, + pub(super) info: FunctionInfo, + pub(super) func: Arc Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'static>, } impl DynamicFunction { @@ -221,6 +223,20 @@ impl IntoFunction<()> for DynamicFunction { } } +impl IntoClosure<'_, ()> for DynamicFunction { + #[inline] + fn into_closure(self) -> DynamicClosure<'static> { + DynamicClosure::from(self) + } +} + +impl IntoClosureMut<'_, ()> for DynamicFunction { + #[inline] + fn into_closure_mut(self) -> DynamicClosureMut<'static> { + DynamicClosureMut::from(self) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/bevy_reflect/src/func/registry.rs b/crates/bevy_reflect/src/func/registry.rs index 2fad8a60c35f4..f1297a925a392 100644 --- a/crates/bevy_reflect/src/func/registry.rs +++ b/crates/bevy_reflect/src/func/registry.rs @@ -4,35 +4,40 @@ use std::sync::{Arc, PoisonError, RwLock, RwLockReadGuard, RwLockWriteGuard}; use bevy_utils::HashMap; -use crate::func::{DynamicFunction, FunctionRegistrationError, IntoFunction}; +use crate::func::{DynamicClosure, FunctionRegistrationError, IntoClosure}; /// A registry of [reflected functions]. /// /// This is the function-equivalent to the [`TypeRegistry`]. /// +/// All functions and closures are stored as `'static` closures via [`DynamicClosure<'static>`]. +/// [`DynamicClosure`] is used instead of [`DynamicFunction`] as it can represent both functions and closures, +/// allowing registered functions to contain captured environment data. +/// /// [reflected functions]: crate::func /// [`TypeRegistry`]: crate::TypeRegistry +/// [`DynamicFunction`]: crate::func::DynamicFunction #[derive(Default)] pub struct FunctionRegistry { - /// Maps function [names] to their respective [`DynamicFunctions`]. + /// Maps function [names] to their respective [`DynamicClosures`]. /// - /// [names]: DynamicFunction::name - /// [`DynamicFunctions`]: DynamicFunction - functions: HashMap, DynamicFunction>, + /// [names]: DynamicClosure::name + /// [`DynamicClosures`]: DynamicClosure + functions: HashMap, DynamicClosure<'static>>, } impl FunctionRegistry { /// Attempts to register the given function. /// - /// This function accepts both functions that satisfy [`IntoFunction`] - /// and direct [`DynamicFunction`] instances. - /// The given function will internally be stored as a [`DynamicFunction`] + /// This function accepts both functions/closures that satisfy [`IntoClosure`] + /// and direct [`DynamicFunction`]/[`DynamicClosure`] instances. + /// The given function will internally be stored as a [`DynamicClosure<'static>`] /// and mapped according to its [name]. /// /// Because the function must have a name, - /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) must instead - /// be registered using [`register_with_name`] or converted to a [`DynamicFunction`] - /// and named using [`DynamicFunction::with_name`]. + /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) and closures must instead + /// be registered using [`register_with_name`] or converted to a [`DynamicClosure`] + /// and named using [`DynamicClosure::with_name`]. /// Failure to do so will result in an error being returned. /// /// If a registered function with the same name already exists, @@ -58,7 +63,7 @@ impl FunctionRegistry { /// Functions cannot be registered more than once. /// /// ``` - /// # use bevy_reflect::func::{DynamicFunction, FunctionRegistrationError, FunctionRegistry, IntoFunction}; + /// # use bevy_reflect::func::{DynamicFunction, FunctionRegistrationError, FunctionRegistry, IntoClosure}; /// fn add(a: i32, b: i32) -> i32 { /// a + b /// } @@ -71,14 +76,14 @@ impl FunctionRegistry { /// /// // Note that this simply relies on the name of the function to determine uniqueness. /// // You can rename the function to register a separate instance of it. - /// let result = registry.register(add.into_function().with_name("add2")); + /// let result = registry.register(add.into_closure().with_name("add2")); /// assert!(result.is_ok()); /// ``` /// - /// Anonymous functions should be registered using [`register_with_name`] or given a name using [`DynamicFunction::with_name`]. + /// Anonymous functions and closures should be registered using [`register_with_name`] or given a name using [`DynamicClosure::with_name`]. /// /// ``` - /// # use bevy_reflect::func::{DynamicFunction, FunctionRegistrationError, FunctionRegistry, IntoFunction}; + /// # use bevy_reflect::func::{DynamicFunction, FunctionRegistrationError, FunctionRegistry, IntoClosure}; /// /// let anonymous = || -> i32 { 123 }; /// @@ -90,11 +95,12 @@ impl FunctionRegistry { /// let result = registry.register_with_name("my_crate::add", |a: i32, b: i32| a + b); /// assert!(result.is_ok()); /// - /// let result = registry.register((|a: i32, b: i32| a * b).into_function().with_name("my_crate::mul")); + /// let result = registry.register((|a: i32, b: i32| a * b).into_closure().with_name("my_crate::mul")); /// assert!(result.is_ok()); /// ``` /// - /// [name]: DynamicFunction::name + /// [`DynamicFunction`]: crate::func::DynamicFunction + /// [name]: DynamicClosure::name /// [`register_with_name`]: Self::register_with_name /// [`overwrite_registration`]: Self::overwrite_registration pub fn register( @@ -102,15 +108,15 @@ impl FunctionRegistry { function: F, ) -> Result<&mut Self, FunctionRegistrationError> where - F: IntoFunction + 'static, + F: IntoClosure<'static, Marker> + 'static, { - let function = function.into_function(); + let function = function.into_closure(); let name = function .name() .ok_or(FunctionRegistrationError::MissingName)? .clone(); self.functions - .try_insert(name, function) + .try_insert(name, function.into_closure()) .map_err(|err| FunctionRegistrationError::DuplicateName(err.entry.key().clone()))?; Ok(self) @@ -118,9 +124,9 @@ impl FunctionRegistry { /// Attempts to register the given function with the given name. /// - /// This function accepts both functions that satisfy [`IntoFunction`] - /// and direct [`DynamicFunction`] instances. - /// The given function will internally be stored as a [`DynamicFunction`] + /// This function accepts both functions/closures that satisfy [`IntoClosure`] + /// and direct [`DynamicFunction`]/[`DynamicClosure`] instances. + /// The given function will internally be stored as a [`DynamicClosure<'static>`] /// with its [name] set to the given name. /// /// For named functions (e.g. `fn add(a: i32, b: i32) -> i32 { a + b }`) where a custom name is not needed, @@ -141,7 +147,7 @@ impl FunctionRegistry { /// Another approach could be to use the [type name] of the function, /// however, it should be noted that anonymous functions do _not_ have unique type names. /// - /// This method is a convenience around calling [`IntoFunction::into_function`] and [`DynamicFunction::with_name`] + /// This method is a convenience around calling [`IntoClosure::into_closure`] and [`DynamicClosure::with_name`] /// on the function and inserting it into the registry using the [`register`] method. /// /// # Examples @@ -167,7 +173,7 @@ impl FunctionRegistry { /// .register_with_name("my_crate::mul", mul)?; /// /// // Be careful not to register anonymous functions with their type name. - /// // This code works but registers the function with the non-unique name of `fn(i32, i32) -> i32` + /// // This code works but registers the function with a non-unique name like `foo::bar::{{closure}}` /// registry.register_with_name(std::any::type_name_of_val(&div), div)?; /// # Ok(()) /// # } @@ -187,7 +193,8 @@ impl FunctionRegistry { /// registry.register_with_name("my_function", two).unwrap(); /// ``` /// - /// [name]: DynamicFunction::name + /// [`DynamicFunction`]: crate::func::DynamicFunction + /// [name]: DynamicClosure::name /// [`register`]: Self::register /// [`overwrite_registration_with_name`]: Self::overwrite_registration_with_name /// [type name]: std::any::type_name @@ -197,23 +204,23 @@ impl FunctionRegistry { function: F, ) -> Result<&mut Self, FunctionRegistrationError> where - F: IntoFunction + 'static, + F: IntoClosure<'static, Marker> + 'static, { - let function = function.into_function().with_name(name); + let function = function.into_closure().with_name(name); self.register(function) } /// Registers the given function, overwriting any existing registration. /// - /// This function accepts both functions that satisfy [`IntoFunction`] - /// and direct [`DynamicFunction`] instances. - /// The given function will internally be stored as a [`DynamicFunction`] + /// This function accepts both functions/closures that satisfy [`IntoClosure`] + /// and direct [`DynamicFunction`]/[`DynamicClosure`] instances. + /// The given function will internally be stored as a [`DynamicClosure<'static>`] /// and mapped according to its [name]. /// /// Because the function must have a name, - /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) must instead - /// be registered using [`overwrite_registration_with_name`] or converted to a [`DynamicFunction`] - /// and named using [`DynamicFunction::with_name`]. + /// anonymous functions (e.g. `|a: i32, b: i32| { a + b }`) and closures must instead + /// be registered using [`overwrite_registration_with_name`] or converted to a [`DynamicClosure`] + /// and named using [`DynamicClosure::with_name`]. /// Failure to do so will result in an error being returned. /// /// To avoid overwriting existing registrations, @@ -221,17 +228,18 @@ impl FunctionRegistry { /// /// Returns the previous function with the same name, if any. /// - /// [name]: DynamicFunction::name + /// [`DynamicFunction`]: crate::func::DynamicFunction + /// [name]: DynamicClosure::name /// [`overwrite_registration_with_name`]: Self::overwrite_registration_with_name /// [`register`]: Self::register pub fn overwrite_registration( &mut self, function: F, - ) -> Result, FunctionRegistrationError> + ) -> Result>, FunctionRegistrationError> where - F: IntoFunction + 'static, + F: IntoClosure<'static, Marker> + 'static, { - let function = function.into_function(); + let function = function.into_closure(); let name = function .name() .ok_or(FunctionRegistrationError::MissingName)? @@ -242,32 +250,33 @@ impl FunctionRegistry { /// Registers the given function, overwriting any existing registration. /// - /// This function accepts both functions that satisfy [`IntoFunction`] - /// and direct [`DynamicFunction`] instances. - /// The given function will internally be stored as a [`DynamicFunction`] + /// This function accepts both functions/closures that satisfy [`IntoClosure`] + /// and direct [`DynamicFunction`]/[`DynamicClosure`] instances. + /// The given function will internally be stored as a [`DynamicClosure<'static>`] /// with its [name] set to the given name. /// /// Functions are mapped according to their name. /// To avoid overwriting existing registrations, /// it's recommended to use the [`register_with_name`] method instead. /// - /// This method is a convenience around calling [`IntoFunction::into_function`] and [`DynamicFunction::with_name`] + /// This method is a convenience around calling [`IntoClosure::into_closure`] and [`DynamicClosure::with_name`] /// on the function and inserting it into the registry using the [`overwrite_registration`] method. /// /// Returns the previous function with the same name, if any. /// - /// [name]: DynamicFunction::name + /// [`DynamicFunction`]: crate::func::DynamicFunction + /// [name]: DynamicClosure::name /// [`register_with_name`]: Self::register_with_name /// [`overwrite_registration`]: Self::overwrite_registration pub fn overwrite_registration_with_name( &mut self, name: impl Into>, function: F, - ) -> Option + ) -> Option> where - F: IntoFunction + 'static, + F: IntoClosure<'static, Marker> + 'static, { - let function = function.into_function().with_name(name); + let function = function.into_closure().with_name(name); match self.overwrite_registration(function) { Ok(existing) => existing, Err(FunctionRegistrationError::MissingName) => { @@ -279,31 +288,31 @@ impl FunctionRegistry { } } - /// Get a reference to a registered function by [name]. + /// Get a reference to a registered function or closure by [name]. /// - /// [name]: DynamicFunction::name - pub fn get(&self, name: &str) -> Option<&DynamicFunction> { + /// [name]: DynamicClosure::name + pub fn get(&self, name: &str) -> Option<&DynamicClosure<'static>> { self.functions.get(name) } - /// Returns `true` if a function with the given [name] is registered. + /// Returns `true` if a function or closure with the given [name] is registered. /// - /// [name]: DynamicFunction::name + /// [name]: DynamicClosure::name pub fn contains(&self, name: &str) -> bool { self.functions.contains_key(name) } - /// Returns an iterator over all registered functions. - pub fn iter(&self) -> impl ExactSizeIterator { + /// Returns an iterator over all registered functions/closures. + pub fn iter(&self) -> impl ExactSizeIterator> { self.functions.values() } - /// Returns the number of registered functions. + /// Returns the number of registered functions/closures. pub fn len(&self) -> usize { self.functions.len() } - /// Returns `true` if no functions are registered. + /// Returns `true` if no functions or closures are registered. pub fn is_empty(&self) -> bool { self.functions.is_empty() } @@ -338,7 +347,7 @@ impl FunctionRegistryArc { #[cfg(test)] mod tests { use super::*; - use crate::func::ArgList; + use crate::func::{ArgList, IntoFunction}; #[test] fn should_register_function() { @@ -364,6 +373,19 @@ mod tests { assert_eq!(value.try_downcast_ref::(), Some(&123)); } + #[test] + fn should_register_closure() { + let value = 123; + let foo = move || -> i32 { value }; + + let mut registry = FunctionRegistry::default(); + registry.register_with_name("foo", foo).unwrap(); + + let function = registry.get("foo").unwrap(); + let value = function.call(ArgList::new()).unwrap().unwrap_owned(); + assert_eq!(value.try_downcast_ref::(), Some(&123)); + } + #[test] fn should_register_dynamic_function() { fn foo() -> i32 { @@ -380,6 +402,21 @@ mod tests { assert_eq!(value.try_downcast_ref::(), Some(&123)); } + #[test] + fn should_register_dynamic_closure() { + let value = 123; + let foo = move || -> i32 { value }; + + let function = foo.into_closure().with_name("custom_name"); + + let mut registry = FunctionRegistry::default(); + registry.register(function).unwrap(); + + let function = registry.get("custom_name").unwrap(); + let value = function.call(ArgList::new()).unwrap().unwrap_owned(); + assert_eq!(value.try_downcast_ref::(), Some(&123)); + } + #[test] fn should_only_register_function_once() { fn foo() -> i32 { @@ -457,6 +494,6 @@ mod tests { registry.register_with_name("foo", foo).unwrap(); let debug = format!("{:?}", registry); - assert_eq!(debug, "{DynamicFunction(fn foo() -> i32)}"); + assert_eq!(debug, "{DynamicClosure(fn foo() -> i32)}"); } }