From d350a948e7ea235080e091811b641b0822595d87 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 12 Dec 2022 18:31:02 +0800 Subject: [PATCH] Allow exporting function pointers from modules. --- CHANGELOG.md | 2 +- src/func/call.rs | 30 ++++++++++---------- src/module/mod.rs | 69 ++++++++++++++++----------------------------- src/tests.rs | 4 +-- src/types/fn_ptr.rs | 33 +++++++++++++++++++--- 5 files changed, 72 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fba207a2..e8552c91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ Net features * 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. +* An additional benefit is that function pointers can now be `export`ed from modules! ### `!in` diff --git a/src/func/call.rs b/src/func/call.rs index 4d4bd0b8..de8198dc 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -781,7 +781,7 @@ impl Engine { caches, &mut Scope::new(), &mut this_ptr, - None, + fn_ptr.encapsulated_environ(), fn_def, args, true, @@ -834,15 +834,15 @@ impl Engine { let fn_ptr = mem::take(&mut call_args[0]).cast::(); #[cfg(not(feature = "no_function"))] - let (fn_name, is_anon, fn_curry, fn_def) = { + let (fn_name, is_anon, fn_curry, environ, 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) + let (fn_name, fn_curry, environ, fn_def) = fn_ptr.take_data(); + (fn_name, is_anon, fn_curry, environ, fn_def) }; #[cfg(feature = "no_function")] - let (fn_name, is_anon, fn_curry) = { - let (fn_name, fn_curry) = fn_ptr.take_data(); - (fn_name, false, fn_curry) + let (fn_name, is_anon, environ, fn_curry) = { + let (fn_name, fn_curry, environ) = fn_ptr.take_data(); + (fn_name, false, fn_curry, environ) }; // Replace the first argument with the object pointer, adding the curried arguments @@ -868,7 +868,7 @@ impl Engine { caches, &mut Scope::new(), target, - None, + environ.as_deref(), &fn_def, args, true, @@ -1043,15 +1043,15 @@ impl Engine { let fn_ptr = arg_value.cast::(); #[cfg(not(feature = "no_function"))] - let (fn_name, is_anon, fn_curry, fn_def) = { + let (fn_name, is_anon, fn_curry, environ, 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) + let (fn_name, fn_curry, environ, fn_def) = fn_ptr.take_data(); + (fn_name, is_anon, fn_curry, environ, fn_def) }; #[cfg(feature = "no_function")] - let (fn_name, is_anon, fn_curry) = { - let (fn_name, fn_curry) = fn_ptr.take_data(); - (fn_name, false, fn_curry) + let (fn_name, is_anon, fn_curry, environ) = { + let (fn_name, fn_curry, environ) = fn_ptr.take_data(); + (fn_name, false, fn_curry, environ) }; curry.extend(fn_curry.into_iter()); @@ -1077,7 +1077,7 @@ impl Engine { caches, &mut Scope::new(), &mut this_ptr, - None, + environ.as_deref(), &fn_def, args, true, diff --git a/src/module/mod.rs b/src/module/mod.rs index c642189e..534f0ff4 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -2138,30 +2138,19 @@ impl Module { // The return value is thrown away and not used let _ = result?; + // Encapsulated environment + #[cfg(not(feature = "no_function"))] + let environ = Shared::new(crate::func::EncapsulatedEnviron { + lib: ast.shared_lib().clone(), + imports: imports.into_boxed_slice(), + constants, + }); + // 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. - // - // 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. + for (_name, mut value, mut aliases) in scope { #[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()) { - return Err(crate::ERR::ErrorMismatchDataType( - String::new(), - if fn_ptr.is_anonymous() { - format!("cannot export closure in variable {_name}") - } else { - format!( - "cannot export function pointer to local function '{}' in variable {_name}", - fn_ptr.fn_name() - ) - }, - crate::Position::NONE, - ) - .into()); - } + if let Some(mut fn_ptr) = value.write_lock::() { + fn_ptr.set_encapsulated_environ(Some(environ.clone())); } match aliases.len() { @@ -2183,30 +2172,22 @@ impl Module { // Non-private functions defined become module functions #[cfg(not(feature = "no_function"))] - { - let environ = Shared::new(crate::func::EncapsulatedEnviron { - lib: ast.shared_lib().clone(), - imports: imports.into_boxed_slice(), - constants, + ast.iter_fn_def() + .filter(|&f| match f.access { + FnAccess::Public => true, + FnAccess::Private => false, + }) + .for_each(|f| { + let hash = module.set_script_fn(f.clone()); + let f = module.functions.as_mut().unwrap().get_mut(&hash).unwrap(); + + // Encapsulate AST environment + match f.func { + CallableFunction::Script(.., ref mut e) => *e = Some(environ.clone()), + _ => (), + } }); - ast.iter_fn_def() - .filter(|&f| match f.access { - FnAccess::Public => true, - FnAccess::Private => false, - }) - .for_each(|f| { - let hash = module.set_script_fn(f.clone()); - let f = module.functions.as_mut().unwrap().get_mut(&hash).unwrap(); - - // Encapsulate AST environment - match f.func { - CallableFunction::Script(.., ref mut e) => *e = Some(environ.clone()), - _ => (), - } - }); - } - module.id = ast.source_raw().cloned(); #[cfg(feature = "metadata")] diff --git a/src/tests.rs b/src/tests.rs index d4560a3f..e6f07dd4 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -50,9 +50,9 @@ fn check_struct_sizes() { assert_eq!( size_of::(), if cfg!(feature = "no_function") { - 64 - } else { 72 + } else { + 80 } ); assert_eq!(size_of::(), 56); diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index 135bfbe5..7be7fcee 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -23,6 +23,7 @@ use std::{ pub struct FnPtr { name: ImmutableString, curry: StaticVec, + environ: Option>, #[cfg(not(feature = "no_function"))] fn_def: Option>, } @@ -75,6 +76,7 @@ impl FnPtr { Self { name: name.into(), curry, + environ: None, #[cfg(not(feature = "no_function"))] fn_def: None, } @@ -100,16 +102,23 @@ impl FnPtr { ) -> ( ImmutableString, StaticVec, + Option>, Option>, ) { - (self.name, self.curry, self.fn_def) + (self.name, self.curry, self.environ, 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) { - (self.name, self.curry) + pub(crate) fn take_data( + self, + ) -> ( + ImmutableString, + StaticVec, + Option>, + ) { + (self.name, self.curry, self.environ) } /// Get the curried arguments. #[inline(always)] @@ -290,7 +299,7 @@ impl FnPtr { caches, &mut crate::Scope::new(), this_ptr.unwrap_or(&mut null_ptr), - None, + self.encapsulated_environ(), &fn_def, args, true, @@ -307,6 +316,20 @@ impl FnPtr { context.call_fn_raw(self.fn_name(), is_method, is_method, args) } + /// Get a reference to the [encapsulated environment][crate::func::EncapsulatedEnviron]. + #[inline(always)] + #[must_use] + pub(crate) fn encapsulated_environ(&self) -> Option<&crate::func::EncapsulatedEnviron> { + self.environ.as_deref() + } + /// Set a reference to the [encapsulated environment][crate::func::EncapsulatedEnviron]. + #[inline(always)] + pub(crate) fn set_encapsulated_environ( + &mut self, + value: Option>>, + ) { + self.environ = value.map(Into::into); + } /// Get a reference to the linked [`ScriptFnDef`][crate::ast::ScriptFnDef]. #[cfg(not(feature = "no_function"))] #[inline(always)] @@ -340,6 +363,7 @@ impl TryFrom for FnPtr { Ok(Self { name: value, curry: StaticVec::new_const(), + environ: None, #[cfg(not(feature = "no_function"))] fn_def: None, }) @@ -358,6 +382,7 @@ impl>> From for FnPtr { Self { name: fn_def.name.clone(), curry: StaticVec::new_const(), + environ: None, fn_def: Some(fn_def), } }