diff --git a/Cargo.lock b/Cargo.lock index 511a5b6b79..5b82167f81 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10,6 +10,7 @@ dependencies = [ "boa_unicode", "chrono", "criterion", + "dyn-clone", "fast-float", "float-cmp", "gc", @@ -358,6 +359,12 @@ version = "0.4.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56899898ce76aaf4a0f24d914c97ea6ed976d42fec6ad33fcbb0a1103e07b2b0" +[[package]] +name = "dyn-clone" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee2626afccd7561a06cf1367e2950c4718ea04565e20fb5029b6c7d8ad09abcf" + [[package]] name = "either" version = "1.6.1" diff --git a/boa/Cargo.toml b/boa/Cargo.toml index 38f8ff8c4e..e7bff4a399 100644 --- a/boa/Cargo.toml +++ b/boa/Cargo.toml @@ -37,6 +37,7 @@ ryu-js = "0.2.1" chrono = "0.4.19" fast-float = "0.2.0" unicode-normalization = "0.1.19" +dyn-clone = "1.0.4" # Optional Dependencies measureme = { version = "9.1.2", optional = true } diff --git a/boa/examples/closures.rs b/boa/examples/closures.rs index 4760e41d76..2d5842d3bc 100644 --- a/boa/examples/closures.rs +++ b/boa/examples/closures.rs @@ -1,14 +1,15 @@ -use boa::{Context, JsString, JsValue}; +use boa::{Context, JsValue}; fn main() -> Result<(), JsValue> { let mut context = Context::new(); - let variable = JsString::new("I am a captured variable"); + let variable = "I am a captured variable"; // 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. - Ok(variable.clone().into()) + println!("variable = {}", variable); + Ok(JsValue::new(variable)) })?; assert_eq!( diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index fe9869dc2f..2df399cc2f 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -15,25 +15,54 @@ use crate::object::PROTOTYPE; use crate::{ builtins::{Array, BuiltIn}, environment::lexical_environment::Environment, - gc::{custom_trace, empty_trace, Finalize, Trace}, + gc::{empty_trace, Finalize, Trace}, object::{ConstructorBuilder, FunctionBuilder, JsObject, 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::rc::Rc; #[cfg(test)] mod tests; -/// _fn(this, arguments, context) -> ResultValue_ - The signature of a native built-in function +// Allows restricting closures to only `Copy` ones. +// Used the sealed pattern to disallow external implementations +// of `DynCopy`. +mod sealed { + pub trait Sealed {} + impl Sealed for T {} +} +pub trait DynCopy: Sealed {} +impl DynCopy for T {} + +/// Type representing a native built-in function a.k.a. function pointer. +/// +/// Native functions need to have this signature in order to +/// be callable from Javascript. pub type NativeFunction = fn(&JsValue, &[JsValue], &mut Context) -> JsResult; -/// _fn(this, arguments, context) -> ResultValue_ - The signature of a closure built-in function -pub type ClosureFunction = dyn Fn(&JsValue, &[JsValue], &mut Context) -> JsResult; +/// Trait representing a native built-in closure. +/// +/// Closures need to have this signature in order to +/// 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 + DynCopy + DynClone + 'static +{ +} + +// The `Copy` bound automatically infers `DynCopy` and `DynClone` +impl ClosureFunction for T where + T: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult + Copy + 'static +{ +} + +// Allows cloning Box +dyn_clone::clone_trait_object!(ClosureFunction); #[derive(Clone, Copy, Finalize)] pub struct BuiltInFunction(pub(crate) NativeFunction); @@ -83,14 +112,15 @@ unsafe impl Trace for FunctionFlags { /// FunctionBody is specific to this interpreter, it will either be Rust code or JavaScript code (AST Node) /// /// -#[derive(Finalize)] +#[derive(Trace, Finalize)] pub enum Function { Native { function: BuiltInFunction, constructable: bool, }, Closure { - function: Rc, + #[unsafe_ignore_trace] + function: Box, constructable: bool, }, Ordinary { @@ -107,18 +137,6 @@ impl Debug for Function { } } -unsafe impl Trace for Function { - custom_trace!(this, { - match this { - Function::Native { .. } => {} - Function::Closure { .. } => {} - Function::Ordinary { environment, .. } => { - mark(environment); - } - } - }); -} - impl Function { // Adds the final rest parameters to the Environment as an array pub(crate) fn add_rest_param( diff --git a/boa/src/context.rs b/boa/src/context.rs index f254959475..c7da7d06a5 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -635,7 +635,7 @@ impl Context { #[inline] pub fn register_global_closure(&mut self, name: &str, length: usize, body: F) -> JsResult<()> where - F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult + 'static, + F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult + Copy + 'static, { let function = FunctionBuilder::closure(self, body) .name(name) diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index 6bbdadb642..0664a79c0e 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -25,7 +25,6 @@ use std::{ collections::HashMap, error::Error, fmt::{self, Debug, Display}, - rc::Rc, result::Result as StdResult, }; @@ -46,7 +45,7 @@ pub struct JsObject(Gc>); enum FunctionBody { BuiltInFunction(NativeFunction), BuiltInConstructor(NativeFunction), - Closure(Rc), + Closure(Box), Ordinary(RcStatementList), } diff --git a/boa/src/object/mod.rs b/boa/src/object/mod.rs index e0f9286821..8e2e82288a 100644 --- a/boa/src/object/mod.rs +++ b/boa/src/object/mod.rs @@ -15,13 +15,12 @@ use crate::{ context::StandardConstructor, gc::{Finalize, Trace}, property::{Attribute, PropertyDescriptor, PropertyKey}, - BoaProfiler, Context, JsBigInt, JsString, JsSymbol, JsValue, + BoaProfiler, Context, JsBigInt, JsResult, JsString, JsSymbol, JsValue, }; use std::{ any::Any, fmt::{self, Debug, Display}, ops::{Deref, DerefMut}, - rc::Rc, }; #[cfg(test)] @@ -1109,12 +1108,12 @@ impl<'context> FunctionBuilder<'context> { #[inline] pub fn closure(context: &'context mut Context, function: F) -> Self where - F: Fn(&JsValue, &[JsValue], &mut Context) -> Result + 'static, + F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult + Copy + 'static, { Self { context, function: Some(Function::Closure { - function: Rc::new(function), + function: Box::new(function), constructable: false, }), name: JsString::default(),