Allow moving `NativeObject` variables into closures as external captures (#1523)

* Allow passing additional `NativeObject` captures to closures

* Add test for external closure captures

boa/examples/closures.rs

@@ -1,21 +1,110 @@
-use boa::{Context, JsValue};
+// This example goes into the details on how to pass closures as functions
+// inside Rust and call them from Javascript.
+
+use boa::{
+    gc::{Finalize, Trace},
+    object::{FunctionBuilder, JsObject},
+    property::{Attribute, PropertyDescriptor},
+    Context, JsString, JsValue,
+};
 
 fn main() -> Result<(), JsValue> {
+    // We create a new `Context` to create a new Javascript executor.
     let mut context = Context::new();
 
-    let variable = "I am a captured variable";
+    // We make some operations in Rust that return a `Copy` value that we want
+    // to pass to a Javascript function.
+    let variable = 128 + 64 + 32 + 16 + 8 + 4 + 2 + 1;
 
     // We register a global closure function that has the name 'closure' with length 0.
     context.register_global_closure("closure", 0, move |_, _, _| {
-        // This value is captured from main function.
+        println!("Called `closure`");
+        // `variable` is captured from the main function.
         println!("variable = {}", variable);
+
+        // We return the moved variable as a `JsValue`.
         Ok(JsValue::new(variable))
     })?;
 
+    assert_eq!(context.eval("closure()")?, 255.into());
+
+    // We have created a closure with moved variables and executed that closure
+    // inside Javascript!
+
+    // This struct is passed to a closure as a capture.
+    #[derive(Debug, Clone, Trace, Finalize)]
+    struct BigStruct {
+        greeting: JsString,
+        object: JsObject,
+    }
+
+    // We create a new `JsObject` with some data
+    let object = context.construct_object();
+    object.define_property_or_throw(
+        "name",
+        PropertyDescriptor::builder()
+            .value("Boa dev")
+            .writable(false)
+            .enumerable(false)
+            .configurable(false),
+        &mut context,
+    )?;
+
+    // Now, we execute some operations that return a `Clone` type
+    let clone_variable = BigStruct {
+        greeting: JsString::from("Hello from Javascript!"),
+        object,
+    };
+
+    // We can use `FunctionBuilder` to define a closure with additional
+    // captures.
+    let js_function = FunctionBuilder::closure_with_captures(
+        &mut context,
+        |_, _, context, captures| {
+            println!("Called `createMessage`");
+            // We obtain the `name` property of `captures.object`
+            let name = captures.object.get("name", context)?;
+
+            // We create a new message from our captured variable.
+            let message = JsString::concat_array(&[
+                "message from `",
+                name.to_string(context)?.as_str(),
+                "`: ",
+                captures.greeting.as_str(),
+            ]);
+
+            println!("{}", message);
+
+            // We convert `message` into `Jsvalue` to be able to return it.
+            Ok(message.into())
+        },
+        // Here is where we move `clone_variable` into the closure.
+        clone_variable,
+    )
+    // And here we assign `createMessage` to the `name` property of the closure.
+    .name("createMessage")
+    // By default all `FunctionBuilder`s set the `length` property to `0` and
+    // the `constructable` property to `false`.
+    .build();
+
+    // We bind the newly constructed closure as a global property in Javascript.
+    context.register_global_property(
+        // We set the key to access the function the same as its name for
+        // consistency, but it may be different if needed.
+        "createMessage",
+        // We pass `js_function` as a property value.
+        js_function,
+        // We assign to the "createMessage" property the desired attributes.
+        Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE,
+    );
+
     assert_eq!(
-        context.eval("closure()")?,
-        "I am a captured variable".into()
+        context.eval("createMessage()")?,
+        "message from `Boa dev`: Hello from Javascript!".into()
     );
 
+    // We have moved `Clone` variables into a closure and executed that closure
+    // inside Javascript!
+
     Ok(())
 }

boa/src/builtins/function/mod.rs

@@ -11,22 +11,25 @@
 //! [spec]: https://tc39.es/ecma262/#sec-function-objects
 //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function
 
-use crate::context::StandardObjects;
-use crate::object::internal_methods::get_prototype_from_constructor;
-
 use crate::{
     builtins::{Array, BuiltIn},
+    context::StandardObjects,
     environment::lexical_environment::Environment,
     gc::{empty_trace, Finalize, Trace},
-    object::{ConstructorBuilder, FunctionBuilder, JsObject, Object, ObjectData},
+    object::{
+        internal_methods::get_prototype_from_constructor, ConstructorBuilder, FunctionBuilder,
+        JsObject, NativeObject, Object, ObjectData,
+    },
     property::{Attribute, PropertyDescriptor},
     syntax::ast::node::{FormalParameter, RcStatementList},
     BoaProfiler, Context, JsResult, JsValue,
 };
 use bitflags::bitflags;
 use dyn_clone::DynClone;
+
 use sealed::Sealed;
 use std::fmt::{self, Debug};
+use std::ops::{Deref, DerefMut};
 
 use super::JsArgs;
 
@@ -55,13 +58,13 @@ pub type NativeFunction = fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsV
 /// be callable from Javascript, but most of the time the compiler
 /// is smart enough to correctly infer the types.
 pub trait ClosureFunction:
-    Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue> + DynCopy + DynClone + 'static
+    Fn(&JsValue, &[JsValue], &mut Context, Captures) -> JsResult<JsValue> + DynCopy + DynClone + 'static
 {
 }
 
 // The `Copy` bound automatically infers `DynCopy` and `DynClone`
 impl<T> ClosureFunction for T where
-    T: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue> + Copy + 'static
+    T: Fn(&JsValue, &[JsValue], &mut Context, Captures) -> JsResult<JsValue> + Copy + 'static
 {
 }
 
@@ -111,6 +114,83 @@ unsafe impl Trace for FunctionFlags {
     empty_trace!();
 }
 
+// We don't use a standalone `NativeObject` for `Captures` because it doesn't
+// guarantee that the internal type implements `Clone`.
+// This private trait guarantees that the internal type passed to `Captures`
+// implements `Clone`, and `DynClone` allows us to implement `Clone` for
+// `Box<dyn CapturesObject>`.
+trait CapturesObject: NativeObject + DynClone {}
+impl<T: NativeObject + Clone> CapturesObject for T {}
+dyn_clone::clone_trait_object!(CapturesObject);
+
+/// Wrapper for `Box<dyn NativeObject + Clone>` that allows passing additional
+/// captures through a `Copy` closure.
+///
+/// Any type implementing `Trace + Any + Debug + Clone`
+/// can be used as a capture context, so you can pass e.g. a String,
+/// a tuple or even a full struct.
+///
+/// You can downcast to any type and handle the fail case as you like
+/// with `downcast_ref` and `downcast_mut`, or you can use `try_downcast_ref`
+/// and `try_downcast_mut` to automatically throw a `TypeError` if the downcast
+/// fails.
+#[derive(Debug, Clone, Trace, Finalize)]
+pub struct Captures(Box<dyn CapturesObject>);
+
+impl Captures {
+    /// Creates a new capture context.
+    pub(crate) fn new<T>(captures: T) -> Self
+    where
+        T: NativeObject + Clone,
+    {
+        Self(Box::new(captures))
+    }
+
+    /// Downcasts `Captures` to the specified type, returning a reference to the
+    /// downcasted type if successful or `None` otherwise.
+    pub fn downcast_ref<T>(&self) -> Option<&T>
+    where
+        T: NativeObject + Clone,
+    {
+        self.0.deref().as_any().downcast_ref::<T>()
+    }
+
+    /// Mutably downcasts `Captures` to the specified type, returning a
+    /// mutable reference to the downcasted type if successful or `None` otherwise.
+    pub fn downcast_mut<T>(&mut self) -> Option<&mut T>
+    where
+        T: NativeObject + Clone,
+    {
+        self.0.deref_mut().as_mut_any().downcast_mut::<T>()
+    }
+
+    /// Downcasts `Captures` to the specified type, returning a reference to the
+    /// downcasted type if successful or a `TypeError` otherwise.
+    pub fn try_downcast_ref<T>(&self, context: &mut Context) -> JsResult<&T>
+    where
+        T: NativeObject + Clone,
+    {
+        self.0
+            .deref()
+            .as_any()
+            .downcast_ref::<T>()
+            .ok_or_else(|| context.construct_type_error("cannot downcast `Captures` to given type"))
+    }
+
+    /// Downcasts `Captures` to the specified type, returning a reference to the
+    /// downcasted type if successful or a `TypeError` otherwise.
+    pub fn try_downcast_mut<T>(&mut self, context: &mut Context) -> JsResult<&mut T>
+    where
+        T: NativeObject + Clone,
+    {
+        self.0
+            .deref_mut()
+            .as_mut_any()
+            .downcast_mut::<T>()
+            .ok_or_else(|| context.construct_type_error("cannot downcast `Captures` to given type"))
+    }
+}
+
 /// Boa representation of a Function Object.
 ///
 /// FunctionBody is specific to this interpreter, it will either be Rust code or JavaScript code (AST Node)
@@ -126,6 +206,7 @@ pub enum Function {
         #[unsafe_ignore_trace]
         function: Box<dyn ClosureFunction>,
         constructable: bool,
+        captures: Captures,
     },
     Ordinary {
         flags: FunctionFlags,

boa/src/builtins/function/tests.rs

@@ -1,4 +1,9 @@
-use crate::{forward, forward_val, Context};
+use crate::{
+    forward, forward_val,
+    object::FunctionBuilder,
+    property::{Attribute, PropertyDescriptor},
+    Context, JsString,
+};
 
 #[allow(clippy::float_cmp)]
 #[test]
@@ -212,3 +217,46 @@ fn function_prototype_apply_on_object() {
         .unwrap();
     assert!(boolean);
 }
+
+#[test]
+fn closure_capture_clone() {
+    let mut context = Context::new();
+
+    let string = JsString::from("Hello");
+    let object = context.construct_object();
+    object
+        .define_property_or_throw(
+            "key",
+            PropertyDescriptor::builder()
+                .value(" world!")
+                .writable(false)
+                .enumerable(false)
+                .configurable(false),
+            &mut context,
+        )
+        .unwrap();
+
+    let func = FunctionBuilder::closure_with_captures(
+        &mut context,
+        |_, _, context, captures| {
+            let (string, object) = &captures;
+
+            let hw = JsString::concat(
+                string,
+                object
+                    .__get_own_property__(&"key".into(), context)?
+                    .and_then(|prop| prop.value().cloned())
+                    .and_then(|val| val.as_string().cloned())
+                    .ok_or_else(|| context.construct_type_error("invalid `key` property"))?,
+            );
+            Ok(hw.into())
+        },
+        (string.clone(), object.clone()),
+    )
+    .name("closure")
+    .build();
+
+    context.register_global_property("closure", func, Attribute::default());
+
+    assert_eq!(forward(&mut context, "closure()"), "\"Hello world!\"");
+}

boa/src/context.rs

@@ -673,11 +673,20 @@ impl Context {
     /// The function will be bound to the global object with `writable`, `non-enumerable`
     /// and `configurable` attributes. The same as when you create a function in JavaScript.
     ///
-    /// # Note
+    /// # Note #1
     ///
     /// If you want to make a function only `constructable`, or wish to bind it differently
     /// to the global object, you can create the function object with [`FunctionBuilder`](crate::object::FunctionBuilder::closure).
     /// And bind it to the global object with [`Context::register_global_property`](Context::register_global_property) method.
+    ///
+    /// # Note #2
+    ///
+    /// This function will only accept `Copy` closures, meaning you cannot
+    /// move `Clone` types, just `Copy` types. If you need to move `Clone` types
+    /// as captures, see [`FunctionBuilder::closure_with_captures`].
+    ///
+    /// See <https://github.com/boa-dev/boa/issues/1515> for an explanation on
+    /// why we need to restrict the set of accepted closures.
     #[inline]
     pub fn register_global_closure<F>(&mut self, name: &str, length: usize, body: F) -> JsResult<()>
     where

boa/src/object/gcobject.rs

@@ -5,7 +5,7 @@
 use super::{NativeObject, Object, PROTOTYPE};
 use crate::{
     builtins::function::{
-        create_unmapped_arguments_object, ClosureFunction, Function, NativeFunction,
+        create_unmapped_arguments_object, Captures, ClosureFunction, Function, NativeFunction,
     },
     environment::{
         environment_record_trait::EnvironmentRecordTrait,
@@ -45,7 +45,10 @@ pub struct JsObject(Gc<GcCell<Object>>);
 enum FunctionBody {
     BuiltInFunction(NativeFunction),
     BuiltInConstructor(NativeFunction),
-    Closure(Box<dyn ClosureFunction>),
+    Closure {
+        function: Box<dyn ClosureFunction>,
+        captures: Captures,
+    },
     Ordinary(RcStatementList),
 }
 
@@ -151,7 +154,12 @@ impl JsObject {
                             FunctionBody::BuiltInFunction(function.0)
                         }
                     }
-                    Function::Closure { function, .. } => FunctionBody::Closure(function.clone()),
+                    Function::Closure {
+                        function, captures, ..
+                    } => FunctionBody::Closure {
+                        function: function.clone(),
+                        captures: captures.clone(),
+                    },
                     Function::Ordinary {
                         body,
                         params,
@@ -297,7 +305,9 @@ impl JsObject {
                 function(&JsValue::undefined(), args, context)
             }
             FunctionBody::BuiltInFunction(function) => function(this_target, args, context),
-            FunctionBody::Closure(function) => (function)(this_target, args, context),
+            FunctionBody::Closure { function, captures } => {
+                (function)(this_target, args, context, captures)
+            }
             FunctionBody::Ordinary(body) => {
                 let result = body.run(context);
                 let this = context.get_this_binding();

boa/src/object/mod.rs

@@ -3,7 +3,7 @@
 use crate::{
     builtins::{
         array::array_iterator::ArrayIterator,
-        function::{Function, NativeFunction},
+        function::{Captures, Function, NativeFunction},
         map::map_iterator::MapIterator,
         map::ordered_map::OrderedMap,
         regexp::regexp_string_iterator::RegExpStringIterator,
@@ -1135,8 +1135,40 @@ impl<'context> FunctionBuilder<'context> {
         Self {
             context,
             function: Some(Function::Closure {
-                function: Box::new(function),
+                function: Box::new(move |this, args, context, _| function(this, args, context)),
                 constructable: false,
+                captures: Captures::new(()),
+            }),
+            name: JsString::default(),
+            length: 0,
+        }
+    }
+
+    /// Create a new closure function with additional captures.
+    ///
+    /// # Note
+    ///
+    /// You can only move variables that implement `Debug + Any + Trace + Clone`.
+    /// In other words, only `NativeObject + Clone` objects are movable.
+    #[inline]
+    pub fn closure_with_captures<F, C>(
+        context: &'context mut Context,
+        function: F,
+        captures: C,
+    ) -> Self
+    where
+        F: Fn(&JsValue, &[JsValue], &mut Context, &mut C) -> JsResult<JsValue> + Copy + 'static,
+        C: NativeObject + Clone,
+    {
+        Self {
+            context,
+            function: Some(Function::Closure {
+                function: Box::new(move |this, args, context, mut captures: Captures| {
+                    let data = captures.try_downcast_mut::<C>(context)?;
+                    function(this, args, context, data)
+                }),
+                constructable: false,
+                captures: Captures::new(captures),
             }),
             name: JsString::default(),
             length: 0,