From c721858338b27c6cbf74edbf655c3bbf52feac06 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 28 Mar 2024 22:41:16 -0700 Subject: [PATCH 01/13] Add more utility traits and funtions to boa_interop --- core/interop/src/into_js_function_impls.rs | 91 +++++++++ core/interop/src/lib.rs | 212 +++++++++++++++++++-- 2 files changed, 283 insertions(+), 20 deletions(-) create mode 100644 core/interop/src/into_js_function_impls.rs diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs new file mode 100644 index 00000000000..90b1e2e72df --- /dev/null +++ b/core/interop/src/into_js_function_impls.rs @@ -0,0 +1,91 @@ +//! Implementations of the `IntoJsFunction` trait for various function signatures. + +use crate::{IntoJsFunction, TryFromJsArgument}; +use boa_engine::{Context, JsValue, NativeFunction}; +use std::cell::RefCell; + +/// A token to represent the context argument in the function signature. +/// This should not be used directly and has no external meaning. +#[derive(Debug, Copy, Clone)] +pub struct ContextArgToken(); + +macro_rules! impl_into_js_function { + ($($id: ident: $t: ident),*) => { + impl<$($t,)* R, T> IntoJsFunction<($($t,)*), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: Into, + T: FnMut($($t,)*) -> R + 'static, + { + #[allow(unused_variables)] + fn into_js_function(self, _context: &mut Context) -> NativeFunction { + let s = RefCell::new(self); + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s.borrow_mut()( $($id),* ); + Ok(r.into()) + }) + } + } + } + + impl<$($t,)* R, T> IntoJsFunction<($($t,)* ContextArgToken), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: Into, + T: FnMut($($t,)* &mut Context) -> R + 'static, + { + #[allow(unused_variables)] + fn into_js_function(self, _context: &mut Context) -> NativeFunction { + let s = RefCell::new(self); + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s.borrow_mut()( $($id,)* ctx); + Ok(r.into()) + }) + } + } + } + }; +} + +impl IntoJsFunction<(), R> for T +where + R: Into, + T: FnMut() -> R + 'static, +{ + fn into_js_function(self, _context: &mut Context) -> NativeFunction { + let s = RefCell::new(self); + unsafe { + NativeFunction::from_closure(move |_this, _args, _ctx| { + let r = s.borrow_mut()(); + Ok(r.into()) + }) + } + } +} + +// Currently implemented up to 12 arguments. The empty argument list +// is implemented separately above. +// Consider that JsRest and JsThis are part of this list, but Context +// is not, as it is a special specialization of the template. +impl_into_js_function!(a: A); +impl_into_js_function!(a: A, b: B); +impl_into_js_function!(a: A, b: B, c: C); +impl_into_js_function!(a: A, b: B, c: C, d: D); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E, f: F); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E, f: F, g: G); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H, i: I); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H, i: I, j: J); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H, i: I, j: J, k: K); +impl_into_js_function!(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H, i: I, j: J, k: K, l: L); diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index f048bf2f655..bcff0c6e9b9 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -3,7 +3,8 @@ use std::cell::RefCell; use boa_engine::module::SyntheticModuleInitializer; -use boa_engine::{Context, JsString, JsValue, Module, NativeFunction}; +use boa_engine::value::TryFromJs; +use boa_engine::{Context, JsResult, JsString, JsValue, Module, NativeFunction}; pub mod loaders; @@ -39,6 +40,41 @@ impl + Clone> IntoJsModule fo /// This trait does not require the implementing type to be `Copy`, which /// can lead to undefined behaviour if it contains Garbage Collected objects. /// +/// This trait is implemented for functions with various signatures. +/// +/// For example: +/// ``` +/// # use boa_engine::{Context, JsValue, NativeFunction}; +/// # use boa_interop::IntoJsFunctionUnsafe; +/// # let mut context = Context::default(); +/// let f = |a: i32, b: i32| a + b; +/// let f = unsafe { f.into_js_function(&mut context) }; +/// let result = f.call(&JsValue::undefined(), &[JsValue::from(1), JsValue::from(2)], &mut context).unwrap(); +/// assert_eq!(result, JsValue::new(3)); +/// ``` +/// +/// Since the `IntoJsFunction` trait is implemented for `FnMut`, you can +/// also use closures directly: +/// ``` +/// # use boa_engine::{Context, JsValue, NativeFunction}; +/// # use boa_interop::IntoJsFunctionUnsafe; +/// # use std::cell::RefCell; +/// # use std::rc::Rc; +/// # let mut context = Context::default(); +/// let mut x = Rc::new(RefCell::new(0)); +/// // Because NativeFunction takes ownership of the closure, +/// // the compiler cannot be certain it won't outlive `x`, so +/// // we need to create a `Rc` and share it. +/// let f = unsafe { +/// let x = x.clone(); +/// move |a: i32| *x.borrow_mut() += a +/// }; +/// let f = f.into_js_function(&mut context); +/// f.call(&JsValue::undefined(), &[JsValue::from(1)], &mut context).unwrap(); +/// f.call(&JsValue::undefined(), &[JsValue::from(4)], &mut context).unwrap(); +/// assert_eq!(*x.borrow(), 5); +/// ``` +/// /// # Safety /// For this trait to be implemented safely, the implementing type must not contain any /// garbage collected objects (from [`boa_gc`]). @@ -63,13 +99,114 @@ unsafe impl IntoJsFunctionUnsafe for T { } } +/// Create a Rust value from a JS argument. This trait is used to +/// convert arguments from JS to Rust types. It allows support +/// for optional arguments or rest arguments. +pub trait TryFromJsArgument: Sized { + /// Try to convert a JS argument into a Rust value, returning the + /// value and the rest of the arguments to be parsed. + /// + /// # Errors + /// Any parsing errors that may occur during the conversion. + fn try_from_js_argument<'a>( + this: &'a JsValue, + rest: &'a [JsValue], + context: &mut Context, + ) -> JsResult<(Self, &'a [JsValue])>; +} + +impl TryFromJsArgument for T { + fn try_from_js_argument<'a>( + _: &'a JsValue, + rest: &'a [JsValue], + context: &mut Context, + ) -> JsResult<(Self, &'a [JsValue])> { + match rest.split_first() { + Some((first, rest)) => Ok((first.try_js_into(context)?, rest)), + None => T::try_from_js(&JsValue::undefined(), context).map(|v| (v, rest)), + } + } +} + +/// An argument that when used in a JS function will empty the list +/// of JS arguments as JsValues. This can be used for having the +/// rest of the arguments in a function. +#[derive(Debug, Clone)] +pub struct JsRest(pub Vec); + +#[allow(unused)] +impl JsRest { + /// Consumes the `JsRest` and returns the inner list of `JsValue`. + fn into_inner(self) -> Vec { + self.0 + } + + /// Returns an iterator over the inner list of `JsValue`. + fn iter(&self) -> impl Iterator { + self.0.iter() + } + + /// Returns a mutable iterator over the inner list of `JsValue`. + fn iter_mut(&mut self) -> impl Iterator { + self.0.iter_mut() + } + + /// Returns the length of the inner list of `JsValue`. + fn len(&self) -> usize { + self.0.len() + } + + /// Returns `true` if the inner list of `JsValue` is empty. + fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + +impl IntoIterator for JsRest { + type Item = JsValue; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.into_inner().into_iter() + } +} + +impl TryFromJsArgument for JsRest { + fn try_from_js_argument<'a>( + _: &'a JsValue, + rest: &'a [JsValue], + _context: &mut Context, + ) -> JsResult<(Self, &'a [JsValue])> { + Ok((JsRest(rest.to_vec()), &[])) + } +} + +/// Captures the `this` value in a JS function. Although this can be +/// specified multiple times as argument, it will always be filled +/// with clone of the same value. +#[derive(Debug, Clone)] +pub struct JsThis(pub T); + +impl TryFromJsArgument for JsThis { + fn try_from_js_argument<'a>( + this: &'a JsValue, + rest: &'a [JsValue], + context: &mut Context, + ) -> JsResult<(Self, &'a [JsValue])> { + Ok((JsThis(this.try_js_into(context)?), rest)) + } +} + +// Implement `IntoJsFunction` for functions with a various list of +// arguments. +mod into_js_function_impls; + #[test] #[allow(clippy::missing_panics_doc)] pub fn into_js_module() { - use boa_engine::builtins::promise::PromiseState; use boa_engine::{js_string, JsValue, Source}; + use std::cell::RefCell; use std::rc::Rc; - use std::sync::atomic::{AtomicU32, Ordering}; let loader = Rc::new(loaders::HashMapModuleLoader::new()); let mut context = Context::builder() @@ -77,29 +214,56 @@ pub fn into_js_module() { .build() .unwrap(); - let foo_count = Rc::new(AtomicU32::new(0)); - let bar_count = Rc::new(AtomicU32::new(0)); + let foo_count = Rc::new(RefCell::new(0)); + let bar_count = Rc::new(RefCell::new(0)); + let dad_count = Rc::new(RefCell::new(0)); + let result = Rc::new(RefCell::new(JsValue::undefined())); let module = unsafe { vec![ ( js_string!("foo"), + { + let counter = foo_count.clone(); + move || { + *counter.borrow_mut() += 1; + let result = *counter.borrow(); + result + } + } + .into_js_function(&mut context), + ), + ( + js_string!("bar"), IntoJsFunctionUnsafe::into_js_function( { - let counter = foo_count.clone(); - move || { - counter.fetch_add(1, Ordering::Relaxed); + let counter = bar_count.clone(); + move |i: i32| { + *counter.borrow_mut() += i; } }, &mut context, ), ), ( - js_string!("bar"), + js_string!("dad"), + { + let counter = dad_count.clone(); + move |args: JsRest, context: &mut Context| { + *counter.borrow_mut() += args + .into_iter() + .map(|i| i.try_js_into::(context).unwrap()) + .sum::(); + } + } + .into_js_function(&mut context), + ), + ( + js_string!("send"), IntoJsFunctionUnsafe::into_js_function( { - let counter = bar_count.clone(); - move || { - counter.fetch_add(1, Ordering::Relaxed); + let result = result.clone(); + move |value: JsValue| { + *result.borrow_mut() = value; } }, &mut context, @@ -115,11 +279,15 @@ pub fn into_js_module() { r" import * as test from 'test'; let result = test.foo(); - for (let i = 0; i < 10; i++) { - test.bar(); + test.foo(); + for (let i = 1; i <= 5; i++) { + test.bar(i); + } + for (let i = 1; i < 5; i++) { + test.dad(1, 2, 3); } - result + test.send(result); ", ); let root_module = Module::parse(source, None, &mut context).unwrap(); @@ -128,11 +296,15 @@ pub fn into_js_module() { context.run_jobs(); // Checking if the final promise didn't return an error. - let PromiseState::Fulfilled(v) = promise_result.state() else { - panic!("module didn't execute successfully!") + if promise_result.state().as_fulfilled().is_none() { + panic!( + "module didn't execute successfully! Promise: {:?}", + promise_result.state() + ); }; - assert_eq!(foo_count.load(Ordering::Relaxed), 1); - assert_eq!(bar_count.load(Ordering::Relaxed), 10); - assert_eq!(v, JsValue::undefined()); + assert_eq!(*foo_count.borrow(), 2); + assert_eq!(*bar_count.borrow(), 15); + assert_eq!(*dad_count.borrow(), 24); + assert_eq!(result.borrow().clone().try_js_into(&mut context), Ok(1u32)); } From a499b886432d7e68fde17a9f48ad5565c7af5da6 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 28 Mar 2024 22:44:57 -0700 Subject: [PATCH 02/13] cargo clippy --- core/interop/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index bcff0c6e9b9..39e64fac356 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -129,7 +129,7 @@ impl TryFromJsArgument for T { } /// An argument that when used in a JS function will empty the list -/// of JS arguments as JsValues. This can be used for having the +/// of JS arguments as `JsValue`s. This can be used for having the /// rest of the arguments in a function. #[derive(Debug, Clone)] pub struct JsRest(pub Vec); @@ -296,12 +296,11 @@ pub fn into_js_module() { context.run_jobs(); // Checking if the final promise didn't return an error. - if promise_result.state().as_fulfilled().is_none() { - panic!( - "module didn't execute successfully! Promise: {:?}", - promise_result.state() - ); - }; + assert!( + promise_result.state().as_fulfilled().is_some(), + "module didn't execute successfully! Promise: {:?}", + promise_result.state() + ); assert_eq!(*foo_count.borrow(), 2); assert_eq!(*bar_count.borrow(), 15); From bbcf043d778a5595eb1993c816d67803151c3d5a Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 1 Apr 2024 10:50:42 -0700 Subject: [PATCH 03/13] Readd the safe version for Fn which also implements Copy --- core/interop/src/into_js_function_impls.rs | 79 +++++++++++++++++++--- core/interop/src/lib.rs | 67 ++++++++++-------- 2 files changed, 108 insertions(+), 38 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 90b1e2e72df..7fe4bf12b7c 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -1,9 +1,11 @@ //! Implementations of the `IntoJsFunction` trait for various function signatures. -use crate::{IntoJsFunction, TryFromJsArgument}; -use boa_engine::{Context, JsValue, NativeFunction}; use std::cell::RefCell; +use boa_engine::{Context, JsValue, NativeFunction}; + +use crate::{IntoJsFunction, IntoJsFunctionUnsafe, TryFromJsArgument}; + /// A token to represent the context argument in the function signature. /// This should not be used directly and has no external meaning. #[derive(Debug, Copy, Clone)] @@ -11,14 +13,14 @@ pub struct ContextArgToken(); macro_rules! impl_into_js_function { ($($id: ident: $t: ident),*) => { - impl<$($t,)* R, T> IntoJsFunction<($($t,)*), R> for T + unsafe impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)*), R> for T where $($t: TryFromJsArgument + 'static,)* R: Into, T: FnMut($($t,)*) -> R + 'static, { #[allow(unused_variables)] - fn into_js_function(self, _context: &mut Context) -> NativeFunction { + unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { NativeFunction::from_closure(move |this, args, ctx| { @@ -33,14 +35,14 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> IntoJsFunction<($($t,)* ContextArgToken), R> for T + unsafe impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)* ContextArgToken), R> for T where $($t: TryFromJsArgument + 'static,)* R: Into, T: FnMut($($t,)* &mut Context) -> R + 'static, { #[allow(unused_variables)] - fn into_js_function(self, _context: &mut Context) -> NativeFunction { + unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { NativeFunction::from_closure(move |this, args, ctx| { @@ -54,15 +56,60 @@ macro_rules! impl_into_js_function { } } } + + // Safe versions for `Fn(..) -> ...`. + impl<$($t,)* R, T> IntoJsFunction<($($t,)*), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: Into, + T: Fn($($t,)*) -> R + 'static + Copy, + { + #[allow(unused_variables)] + fn into_js_function(self, _context: &mut Context) -> NativeFunction { + let s = self; + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id),* ); + Ok(r.into()) + }) + } + } + } + + impl<$($t,)* R, T> IntoJsFunction<($($t,)* ContextArgToken), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: Into, + T: Fn($($t,)* &mut Context) -> R + 'static + Copy, + { + #[allow(unused_variables)] + fn into_js_function(self, _context: &mut Context) -> NativeFunction { + let s = self; + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id,)* ctx); + Ok(r.into()) + }) + } + } + } }; } -impl IntoJsFunction<(), R> for T +unsafe impl IntoJsFunctionUnsafe<(), R> for T where R: Into, T: FnMut() -> R + 'static, { - fn into_js_function(self, _context: &mut Context) -> NativeFunction { + unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { NativeFunction::from_closure(move |_this, _args, _ctx| { @@ -73,6 +120,22 @@ where } } +impl IntoJsFunction<(), R> for T +where + R: Into, + T: Fn() -> R + 'static + Copy, +{ + fn into_js_function(self, _context: &mut Context) -> NativeFunction { + let s = self; + unsafe { + NativeFunction::from_closure(move |_this, _args, _ctx| { + let r = s(); + Ok(r.into()) + }) + } + } +} + // Currently implemented up to 12 arguments. The empty argument list // is implemented separately above. // Consider that JsRest and JsThis are part of this list, but Context diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 39e64fac356..72a807f4316 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -1,7 +1,5 @@ //! Interop utilities between Boa and its host. -use std::cell::RefCell; - use boa_engine::module::SyntheticModuleInitializer; use boa_engine::value::TryFromJs; use boa_engine::{Context, JsResult, JsString, JsValue, Module, NativeFunction}; @@ -48,12 +46,12 @@ impl + Clone> IntoJsModule fo /// # use boa_interop::IntoJsFunctionUnsafe; /// # let mut context = Context::default(); /// let f = |a: i32, b: i32| a + b; -/// let f = unsafe { f.into_js_function(&mut context) }; +/// let f = unsafe { f.into_js_function_unsafe(&mut context) }; /// let result = f.call(&JsValue::undefined(), &[JsValue::from(1), JsValue::from(2)], &mut context).unwrap(); /// assert_eq!(result, JsValue::new(3)); /// ``` /// -/// Since the `IntoJsFunction` trait is implemented for `FnMut`, you can +/// Since the `IntoJsFunctionUnsafe` trait is implemented for `FnMut`, you can /// also use closures directly: /// ``` /// # use boa_engine::{Context, JsValue, NativeFunction}; @@ -69,7 +67,7 @@ impl + Clone> IntoJsModule fo /// let x = x.clone(); /// move |a: i32| *x.borrow_mut() += a /// }; -/// let f = f.into_js_function(&mut context); +/// let f = unsafe { f.into_js_function_unsafe(&mut context) }; /// f.call(&JsValue::undefined(), &[JsValue::from(1)], &mut context).unwrap(); /// f.call(&JsValue::undefined(), &[JsValue::from(4)], &mut context).unwrap(); /// assert_eq!(*x.borrow(), 5); @@ -78,25 +76,32 @@ impl + Clone> IntoJsModule fo /// # Safety /// For this trait to be implemented safely, the implementing type must not contain any /// garbage collected objects (from [`boa_gc`]). -pub unsafe trait IntoJsFunctionUnsafe { +pub unsafe trait IntoJsFunctionUnsafe { /// Converts the type into a JS function. /// /// # Safety /// This function is unsafe to ensure the callee knows the risks of using this trait. /// The implementing type must not contain any garbage collected objects. - unsafe fn into_js_function(self, context: &mut Context) -> NativeFunction; + unsafe fn into_js_function_unsafe(self, context: &mut Context) -> NativeFunction; } -unsafe impl IntoJsFunctionUnsafe for T { - unsafe fn into_js_function(self, _context: &mut Context) -> NativeFunction { - let cell = RefCell::new(self); - unsafe { - NativeFunction::from_closure(move |_, _, _| { - cell.borrow_mut()(); - Ok(JsValue::undefined()) - }) - } - } +/// The safe equivalent of the [`IntoJsFunctionUnsafe`] trait. +/// This can only be used on closures that have the `Copy` trait. +/// +/// Since this function is implemented for `Fn(...)` closures, we can use +/// it directly when defining a function: +/// ``` +/// # use boa_engine::{Context, JsValue, NativeFunction}; +/// # use boa_interop::IntoJsFunction; +/// # let mut context = Context::default(); +/// let f = |a: i32, b: i32| a + b; +/// let f = f.into_js_function(&mut context); +/// let result = f.call(&JsValue::undefined(), &[JsValue::from(1), JsValue::from(2)], &mut context).unwrap(); +/// assert_eq!(result, JsValue::new(3)); +/// ``` +pub trait IntoJsFunction: Copy { + /// Converts the type into a JS function. + fn into_js_function(self, context: &mut Context) -> NativeFunction; } /// Create a Rust value from a JS argument. This trait is used to @@ -230,11 +235,11 @@ pub fn into_js_module() { result } } - .into_js_function(&mut context), + .into_js_function_unsafe(&mut context), ), ( js_string!("bar"), - IntoJsFunctionUnsafe::into_js_function( + IntoJsFunctionUnsafe::into_js_function_unsafe( { let counter = bar_count.clone(); move |i: i32| { @@ -246,20 +251,22 @@ pub fn into_js_module() { ), ( js_string!("dad"), - { - let counter = dad_count.clone(); - move |args: JsRest, context: &mut Context| { - *counter.borrow_mut() += args - .into_iter() - .map(|i| i.try_js_into::(context).unwrap()) - .sum::(); - } - } - .into_js_function(&mut context), + IntoJsFunctionUnsafe::into_js_function_unsafe( + { + let counter = dad_count.clone(); + move |args: JsRest, context: &mut Context| { + *counter.borrow_mut() += args + .into_iter() + .map(|i| i.try_js_into::(context).unwrap()) + .sum::(); + } + }, + &mut context, + ), ), ( js_string!("send"), - IntoJsFunctionUnsafe::into_js_function( + IntoJsFunctionUnsafe::into_js_function_unsafe( { let result = result.clone(); move |value: JsValue| { From 4b0f47d9ddca1adfd351478f27c7444f734739af Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 3 Apr 2024 20:26:04 -0700 Subject: [PATCH 04/13] Use a new trait for converting a Rust type to a JsResult --- core/interop/src/into_js_function_impls.rs | 34 ++++++------ core/interop/src/lib.rs | 12 +++++ core/interop/src/try_into_js_result_impls.rs | 55 ++++++++++++++++++++ 3 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 core/interop/src/try_into_js_result_impls.rs diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 7fe4bf12b7c..67ab1cc6028 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -2,21 +2,21 @@ use std::cell::RefCell; -use boa_engine::{Context, JsValue, NativeFunction}; +use boa_engine::{Context, NativeFunction}; -use crate::{IntoJsFunction, IntoJsFunctionUnsafe, TryFromJsArgument}; +use crate::{IntoJsFunction, IntoJsFunctionUnsafe, TryFromJsArgument, TryIntoJsResult}; /// A token to represent the context argument in the function signature. /// This should not be used directly and has no external meaning. #[derive(Debug, Copy, Clone)] -pub struct ContextArgToken(); +pub struct ContextArgToken; macro_rules! impl_into_js_function { ($($id: ident: $t: ident),*) => { unsafe impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)*), R> for T where $($t: TryFromJsArgument + 'static,)* - R: Into, + R: TryIntoJsResult, T: FnMut($($t,)*) -> R + 'static, { #[allow(unused_variables)] @@ -29,7 +29,7 @@ macro_rules! impl_into_js_function { let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* let r = s.borrow_mut()( $($id),* ); - Ok(r.into()) + r.try_into_js_result(ctx) }) } } @@ -38,7 +38,7 @@ macro_rules! impl_into_js_function { unsafe impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)* ContextArgToken), R> for T where $($t: TryFromJsArgument + 'static,)* - R: Into, + R: TryIntoJsResult, T: FnMut($($t,)* &mut Context) -> R + 'static, { #[allow(unused_variables)] @@ -51,7 +51,7 @@ macro_rules! impl_into_js_function { let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* let r = s.borrow_mut()( $($id,)* ctx); - Ok(r.into()) + r.try_into_js_result(ctx) }) } } @@ -61,7 +61,7 @@ macro_rules! impl_into_js_function { impl<$($t,)* R, T> IntoJsFunction<($($t,)*), R> for T where $($t: TryFromJsArgument + 'static,)* - R: Into, + R: TryIntoJsResult, T: Fn($($t,)*) -> R + 'static + Copy, { #[allow(unused_variables)] @@ -74,7 +74,7 @@ macro_rules! impl_into_js_function { let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* let r = s( $($id),* ); - Ok(r.into()) + r.try_into_js_result(ctx) }) } } @@ -83,7 +83,7 @@ macro_rules! impl_into_js_function { impl<$($t,)* R, T> IntoJsFunction<($($t,)* ContextArgToken), R> for T where $($t: TryFromJsArgument + 'static,)* - R: Into, + R: TryIntoJsResult, T: Fn($($t,)* &mut Context) -> R + 'static + Copy, { #[allow(unused_variables)] @@ -96,7 +96,7 @@ macro_rules! impl_into_js_function { let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* let r = s( $($id,)* ctx); - Ok(r.into()) + r.try_into_js_result(ctx) }) } } @@ -106,15 +106,15 @@ macro_rules! impl_into_js_function { unsafe impl IntoJsFunctionUnsafe<(), R> for T where - R: Into, + R: TryIntoJsResult, T: FnMut() -> R + 'static, { unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { - NativeFunction::from_closure(move |_this, _args, _ctx| { + NativeFunction::from_closure(move |_this, _args, ctx| { let r = s.borrow_mut()(); - Ok(r.into()) + r.try_into_js_result(ctx) }) } } @@ -122,15 +122,15 @@ where impl IntoJsFunction<(), R> for T where - R: Into, + R: TryIntoJsResult, T: Fn() -> R + 'static + Copy, { fn into_js_function(self, _context: &mut Context) -> NativeFunction { let s = self; unsafe { - NativeFunction::from_closure(move |_this, _args, _ctx| { + NativeFunction::from_closure(move |_this, _args, ctx| { let r = s(); - Ok(r.into()) + r.try_into_js_result(ctx) }) } } diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 72a807f4316..34d5d42aa90 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -104,6 +104,18 @@ pub trait IntoJsFunction: Copy { fn into_js_function(self, context: &mut Context) -> NativeFunction; } +/// Create a [`JsResult`] from a Rust value. This trait is used to +/// convert Rust types to JS types, including [`JsResult`] of +/// Rust values and [`JsValue`]s. It is used to convert the +/// return value of a function in [`IntoJsFunctionUnsafe`] and +/// [`IntoJsFunction`]. +pub trait TryIntoJsResult { + /// Try to convert a Rust value into a `JsResult`. + fn try_into_js_result(self, context: &mut Context) -> JsResult; +} + +mod try_into_js_result_impls; + /// Create a Rust value from a JS argument. This trait is used to /// convert arguments from JS to Rust types. It allows support /// for optional arguments or rest arguments. diff --git a/core/interop/src/try_into_js_result_impls.rs b/core/interop/src/try_into_js_result_impls.rs new file mode 100644 index 00000000000..1be5e1e92e8 --- /dev/null +++ b/core/interop/src/try_into_js_result_impls.rs @@ -0,0 +1,55 @@ +//! Declare implementations of [`TryIntoJsResult`] trait for various types. +//! We cannot rely on a generic implementation based on `TryIntoJs` due +//! to a limitation of the Rust type system which prevents generalization +//! of traits based on an upstream crate. + +use crate::TryIntoJsResult; +use boa_engine::{Context, JsBigInt, JsResult, JsString, JsSymbol, JsValue}; + +impl TryIntoJsResult for Option { + fn try_into_js_result(self, context: &mut Context) -> JsResult { + match self { + Some(value) => value.try_into_js_result(context), + None => Ok(JsValue::undefined()), + } + } +} + +impl TryIntoJsResult for JsResult { + fn try_into_js_result(self, context: &mut Context) -> JsResult { + self.and_then(|value| value.try_into_js_result(context)) + } +} + +macro_rules! impl_try_into_js_result { + ($($t: ty),*) => { + $( + impl TryIntoJsResult for $t { + fn try_into_js_result(self, _context: &mut Context) -> JsResult { + Ok(JsValue::from(self)) + } + } + )* + }; +} + +impl_try_into_js_result!( + bool, + char, + i8, + i16, + i32, + i64, + u8, + u16, + u32, + u64, + usize, + f32, + f64, + JsBigInt, + JsString, + JsSymbol, + JsValue, + () +); From 64fff68ac21c90a6b85fbce202ed97344052bbcb Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 3 Apr 2024 20:59:13 -0700 Subject: [PATCH 05/13] cargo clippy --- core/interop/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 34d5d42aa90..87bb950d5e1 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -111,6 +111,10 @@ pub trait IntoJsFunction: Copy { /// [`IntoJsFunction`]. pub trait TryIntoJsResult { /// Try to convert a Rust value into a `JsResult`. + /// + /// # Errors + /// Any parsing errors that may occur during the conversion, or any + /// error that happened during the call to a function. fn try_into_js_result(self, context: &mut Context) -> JsResult; } From 529dd788168f8e5f20569c56989bd259c37e2b17 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 3 Apr 2024 21:33:08 -0700 Subject: [PATCH 06/13] Add a test for returning result and Fn() --- core/interop/src/lib.rs | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 87bb950d5e1..f5b4f648320 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -330,3 +330,47 @@ pub fn into_js_module() { assert_eq!(*dad_count.borrow(), 24); assert_eq!(result.borrow().clone().try_js_into(&mut context), Ok(1u32)); } + +#[test] +fn can_throw_exception() { + use boa_engine::{js_string, JsError, JsValue, Source}; + use std::rc::Rc; + + let loader = Rc::new(loaders::HashMapModuleLoader::new()); + let mut context = Context::builder() + .module_loader(loader.clone()) + .build() + .unwrap(); + + let module = vec![( + js_string!("doTheThrow"), + IntoJsFunction::into_js_function( + |message: JsValue| -> JsResult<()> { JsResult::Err(JsError::from_opaque(message)) }, + &mut context, + ), + )] + .into_js_module(&mut context); + + loader.register(js_string!("test"), module); + + let source = Source::from_bytes( + r" + import * as test from 'test'; + try { + test.doTheThrow('javascript'); + } catch(e) { + throw 'from ' + e; + } + ", + ); + let root_module = Module::parse(source, None, &mut context).unwrap(); + + let promise_result = root_module.load_link_evaluate(&mut context); + context.run_jobs(); + + // Checking if the final promise didn't return an error. + assert_eq!( + promise_result.state().as_rejected(), + Some(&JsString::from("from javascript").into()) + ); +} From 921b35a988de544b51d8a7e36ffed4e33c054841 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 13:08:48 -0700 Subject: [PATCH 07/13] Seal both IntoJsFunction and IntoJsFunctionUnsafe --- core/interop/src/into_js_function_impls.rs | 28 +++++++++++++++++++--- core/interop/src/lib.rs | 15 +++++++----- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 67ab1cc6028..0cffe0f6478 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -4,6 +4,7 @@ use std::cell::RefCell; use boa_engine::{Context, NativeFunction}; +use crate::private::IntoJsFunctionSealed; use crate::{IntoJsFunction, IntoJsFunctionUnsafe, TryFromJsArgument, TryIntoJsResult}; /// A token to represent the context argument in the function signature. @@ -13,7 +14,21 @@ pub struct ContextArgToken; macro_rules! impl_into_js_function { ($($id: ident: $t: ident),*) => { - unsafe impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)*), R> for T + impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)*), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: FnMut($($t,)*) -> R + 'static + {} + + impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* ContextArgToken), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: FnMut($($t,)* &mut Context) -> R + 'static + {} + + impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)*), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, @@ -35,7 +50,7 @@ macro_rules! impl_into_js_function { } } - unsafe impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)* ContextArgToken), R> for T + impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)* ContextArgToken), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, @@ -104,7 +119,14 @@ macro_rules! impl_into_js_function { }; } -unsafe impl IntoJsFunctionUnsafe<(), R> for T +impl IntoJsFunctionSealed<(), R> for T +where + R: TryIntoJsResult, + T: FnMut() -> R + 'static, +{ +} + +impl IntoJsFunctionUnsafe<(), R> for T where R: TryIntoJsResult, T: FnMut() -> R + 'static, diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index f3e9d39312b..45c2f168d69 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -6,6 +6,13 @@ use boa_engine::{Context, JsResult, JsString, JsValue, Module, NativeFunction}; pub mod loaders; +/// Internal module only. +pub(crate) mod private { + /// A sealed trait to prevent users from implementing the `IntoJsModuleFunction` + /// and `IntoJsFunctionUnsafe` traits to their own types. + pub trait IntoJsFunctionSealed {} +} + /// A trait to convert a type into a JS module. pub trait IntoJsModule { /// Converts the type into a JS module. @@ -73,11 +80,7 @@ impl + Clone> IntoJsModule fo /// f.call(&JsValue::undefined(), &[JsValue::from(4)], &mut context).unwrap(); /// assert_eq!(*x.borrow(), 5); /// ``` -/// -/// # Safety -/// For this trait to be implemented safely, the implementing type must not contain any -/// garbage collected objects (from [`boa_gc`]). -pub unsafe trait IntoJsFunctionUnsafe { +pub trait IntoJsFunctionUnsafe: private::IntoJsFunctionSealed { /// Converts the type into a JS function. /// /// # Safety @@ -100,7 +103,7 @@ pub unsafe trait IntoJsFunctionUnsafe { /// let result = f.call(&JsValue::undefined(), &[JsValue::from(1), JsValue::from(2)], &mut context).unwrap(); /// assert_eq!(result, JsValue::new(3)); /// ``` -pub trait IntoJsFunction: Copy { +pub trait IntoJsFunction: private::IntoJsFunctionSealed + Copy { /// Converts the type into a JS function. fn into_js_function(self, context: &mut Context) -> NativeFunction; } From d8815ac53781a14f2ec282d7250b0c38870f3ac7 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 13:13:10 -0700 Subject: [PATCH 08/13] Try Borrowing and return a nice error instead of panicking --- core/interop/src/into_js_function_impls.rs | 18 +++++++++++------- core/interop/src/lib.rs | 8 ++++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 0cffe0f6478..73c3f200f94 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -2,10 +2,10 @@ use std::cell::RefCell; -use boa_engine::{Context, NativeFunction}; +use boa_engine::{js_string, Context, JsError, NativeFunction}; use crate::private::IntoJsFunctionSealed; -use crate::{IntoJsFunction, IntoJsFunctionUnsafe, TryFromJsArgument, TryIntoJsResult}; +use crate::{IntoJsFunctionCopied, IntoJsFunctionUnsafe, TryFromJsArgument, TryIntoJsResult}; /// A token to represent the context argument in the function signature. /// This should not be used directly and has no external meaning. @@ -43,8 +43,12 @@ macro_rules! impl_into_js_function { $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* - let r = s.borrow_mut()( $($id),* ); - r.try_into_js_result(ctx) + match s.try_borrow_mut() { + Ok(mut r) => r( $($id),* ).try_into_js_result(ctx), + Err(_) => { + Err(JsError::from_opaque(js_string!("recursive calls to this function not supported").into())) + } + } }) } } @@ -73,7 +77,7 @@ macro_rules! impl_into_js_function { } // Safe versions for `Fn(..) -> ...`. - impl<$($t,)* R, T> IntoJsFunction<($($t,)*), R> for T + impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)*), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, @@ -95,7 +99,7 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> IntoJsFunction<($($t,)* ContextArgToken), R> for T + impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* ContextArgToken), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, @@ -142,7 +146,7 @@ where } } -impl IntoJsFunction<(), R> for T +impl IntoJsFunctionCopied<(), R> for T where R: TryIntoJsResult, T: Fn() -> R + 'static + Copy, diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 45c2f168d69..e88200a3f79 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -96,14 +96,14 @@ pub trait IntoJsFunctionUnsafe: private::IntoJsFunctionSealed: private::IntoJsFunctionSealed + Copy { +pub trait IntoJsFunctionCopied: private::IntoJsFunctionSealed + Copy { /// Converts the type into a JS function. fn into_js_function(self, context: &mut Context) -> NativeFunction; } @@ -112,7 +112,7 @@ pub trait IntoJsFunction: private::IntoJsFunctionSealed + /// convert Rust types to JS types, including [`JsResult`] of /// Rust values and [`JsValue`]s. It is used to convert the /// return value of a function in [`IntoJsFunctionUnsafe`] and -/// [`IntoJsFunction`]. +/// [`IntoJsFunctionCopied`]. pub trait TryIntoJsResult { /// Try to convert a Rust value into a `JsResult`. /// @@ -348,7 +348,7 @@ fn can_throw_exception() { let module = vec![( js_string!("doTheThrow"), - IntoJsFunction::into_js_function( + IntoJsFunctionCopied::into_js_function( |message: JsValue| -> JsResult<()> { JsResult::Err(JsError::from_opaque(message)) }, &mut context, ), From 78d541f407118207eed874e543d496e6da4e4648 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 14:00:16 -0700 Subject: [PATCH 09/13] Address comments --- core/interop/src/into_js_function_impls.rs | 8 ++++---- core/interop/src/lib.rs | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 73c3f200f94..7ec023c43e8 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -5,7 +5,7 @@ use std::cell::RefCell; use boa_engine::{js_string, Context, JsError, NativeFunction}; use crate::private::IntoJsFunctionSealed; -use crate::{IntoJsFunctionCopied, IntoJsFunctionUnsafe, TryFromJsArgument, TryIntoJsResult}; +use crate::{IntoJsFunctionCopied, TryFromJsArgument, TryIntoJsResult, UnsafeIntoJsFunction}; /// A token to represent the context argument in the function signature. /// This should not be used directly and has no external meaning. @@ -28,7 +28,7 @@ macro_rules! impl_into_js_function { T: FnMut($($t,)* &mut Context) -> R + 'static {} - impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)*), R> for T + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)*), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, @@ -54,7 +54,7 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> IntoJsFunctionUnsafe<($($t,)* ContextArgToken), R> for T + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* ContextArgToken), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, @@ -130,7 +130,7 @@ where { } -impl IntoJsFunctionUnsafe<(), R> for T +impl UnsafeIntoJsFunction<(), R> for T where R: TryIntoJsResult, T: FnMut() -> R + 'static, diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index e88200a3f79..30c2d18327b 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -51,7 +51,7 @@ impl + Clone> IntoJsModule fo /// For example: /// ``` /// # use boa_engine::{Context, JsValue, NativeFunction}; -/// # use boa_interop::IntoJsFunctionUnsafe; +/// # use boa_interop::UnsafeIntoJsFunction; /// # let mut context = Context::default(); /// let f = |a: i32, b: i32| a + b; /// let f = unsafe { f.into_js_function_unsafe(&mut context) }; @@ -63,7 +63,7 @@ impl + Clone> IntoJsModule fo /// also use closures directly: /// ``` /// # use boa_engine::{Context, JsValue, NativeFunction}; -/// # use boa_interop::IntoJsFunctionUnsafe; +/// # use boa_interop::UnsafeIntoJsFunction; /// # use std::cell::RefCell; /// # use std::rc::Rc; /// # let mut context = Context::default(); @@ -80,7 +80,7 @@ impl + Clone> IntoJsModule fo /// f.call(&JsValue::undefined(), &[JsValue::from(4)], &mut context).unwrap(); /// assert_eq!(*x.borrow(), 5); /// ``` -pub trait IntoJsFunctionUnsafe: private::IntoJsFunctionSealed { +pub trait UnsafeIntoJsFunction: private::IntoJsFunctionSealed { /// Converts the type into a JS function. /// /// # Safety @@ -89,7 +89,7 @@ pub trait IntoJsFunctionUnsafe: private::IntoJsFunctionSealed NativeFunction; } -/// The safe equivalent of the [`IntoJsFunctionUnsafe`] trait. +/// The safe equivalent of the [`UnsafeIntoJsFunction`] trait. /// This can only be used on closures that have the `Copy` trait. /// /// Since this function is implemented for `Fn(...)` closures, we can use @@ -111,7 +111,7 @@ pub trait IntoJsFunctionCopied: private::IntoJsFunctionSealed`. @@ -259,7 +259,7 @@ pub fn into_js_module() { ), ( js_string!("bar"), - IntoJsFunctionUnsafe::into_js_function_unsafe( + UnsafeIntoJsFunction::into_js_function_unsafe( { let counter = bar_count.clone(); move |i: i32| { @@ -271,7 +271,7 @@ pub fn into_js_module() { ), ( js_string!("dad"), - IntoJsFunctionUnsafe::into_js_function_unsafe( + UnsafeIntoJsFunction::into_js_function_unsafe( { let counter = dad_count.clone(); move |args: JsRest, context: &mut Context| { @@ -286,7 +286,7 @@ pub fn into_js_module() { ), ( js_string!("send"), - IntoJsFunctionUnsafe::into_js_function_unsafe( + UnsafeIntoJsFunction::into_js_function_unsafe( { let result = result.clone(); move |value: JsValue| { From 3ff54d506fb2e45ffae2edda442a6e042bb87305 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 16:16:00 -0700 Subject: [PATCH 10/13] Rename into_js_function to into_js_function_copied --- core/interop/src/into_js_function_impls.rs | 6 +++--- core/interop/src/lib.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 7ec023c43e8..83fade587d4 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -84,7 +84,7 @@ macro_rules! impl_into_js_function { T: Fn($($t,)*) -> R + 'static + Copy, { #[allow(unused_variables)] - fn into_js_function(self, _context: &mut Context) -> NativeFunction { + fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { let s = self; unsafe { NativeFunction::from_closure(move |this, args, ctx| { @@ -106,7 +106,7 @@ macro_rules! impl_into_js_function { T: Fn($($t,)* &mut Context) -> R + 'static + Copy, { #[allow(unused_variables)] - fn into_js_function(self, _context: &mut Context) -> NativeFunction { + fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { let s = self; unsafe { NativeFunction::from_closure(move |this, args, ctx| { @@ -151,7 +151,7 @@ where R: TryIntoJsResult, T: Fn() -> R + 'static + Copy, { - fn into_js_function(self, _context: &mut Context) -> NativeFunction { + fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { let s = self; unsafe { NativeFunction::from_closure(move |_this, _args, ctx| { diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 30c2d18327b..5bc8b72b4bc 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -99,13 +99,13 @@ pub trait UnsafeIntoJsFunction: private::IntoJsFunctionSealed: private::IntoJsFunctionSealed + Copy { /// Converts the type into a JS function. - fn into_js_function(self, context: &mut Context) -> NativeFunction; + fn into_js_function_copied(self, context: &mut Context) -> NativeFunction; } /// Create a [`JsResult`] from a Rust value. This trait is used to @@ -348,7 +348,7 @@ fn can_throw_exception() { let module = vec![( js_string!("doTheThrow"), - IntoJsFunctionCopied::into_js_function( + IntoJsFunctionCopied::into_js_function_copied( |message: JsValue| -> JsResult<()> { JsResult::Err(JsError::from_opaque(message)) }, &mut context, ), From 3ea6baf471b227f0133e976b2a67e84b4487aacf Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sun, 7 Apr 2024 19:43:35 -0700 Subject: [PATCH 11/13] Move TryIntoJsResult to boa_engine --- core/engine/src/lib.rs | 16 ++++++ core/engine/src/try_into_js_result_impls.rs | 33 ++++++++++++ core/interop/src/into_js_function_impls.rs | 4 +- core/interop/src/lib.rs | 18 +------ core/interop/src/try_into_js_result_impls.rs | 55 -------------------- 5 files changed, 52 insertions(+), 74 deletions(-) create mode 100644 core/engine/src/try_into_js_result_impls.rs delete mode 100644 core/interop/src/try_into_js_result_impls.rs diff --git a/core/engine/src/lib.rs b/core/engine/src/lib.rs index c017bfb4c60..713906e9bd3 100644 --- a/core/engine/src/lib.rs +++ b/core/engine/src/lib.rs @@ -141,6 +141,22 @@ pub use prelude::*; /// The result of a Javascript expression is represented like this so it can succeed (`Ok`) or fail (`Err`) pub type JsResult = StdResult; +/// Create a [`JsResult`] from a Rust value. This trait is used to +/// convert Rust types to JS types, including [`JsResult`] of +/// Rust values and [`JsValue`]s. +/// +/// This trait is implemented for any that can be converted into a [`JsValue`]. +pub trait TryIntoJsResult { + /// Try to convert a Rust value into a `JsResult`. + /// + /// # Errors + /// Any parsing errors that may occur during the conversion, or any + /// error that happened during the call to a function. + fn try_into_js_result(self, context: &mut Context) -> JsResult; +} + +mod try_into_js_result_impls; + /// A utility trait to make working with function arguments easier. pub trait JsArgs { /// Utility function to `get` a parameter from a `[JsValue]` or default to `JsValue::Undefined` diff --git a/core/engine/src/try_into_js_result_impls.rs b/core/engine/src/try_into_js_result_impls.rs new file mode 100644 index 00000000000..e4fe117ec7d --- /dev/null +++ b/core/engine/src/try_into_js_result_impls.rs @@ -0,0 +1,33 @@ +//! Declare implementations of [`TryIntoJsResult`] trait for various types. + +use crate::{Context, JsResult, JsValue, TryIntoJsResult}; + +impl TryIntoJsResult for T +where + T: Into, +{ + fn try_into_js_result(self, _cx: &mut Context) -> JsResult { + Ok(self.into()) + } +} + +impl TryIntoJsResult for Option +where + T: TryIntoJsResult, +{ + fn try_into_js_result(self, cx: &mut Context) -> JsResult { + match self { + Some(value) => value.try_into_js_result(cx), + None => Ok(JsValue::undefined()), + } + } +} + +impl TryIntoJsResult for JsResult +where + T: TryIntoJsResult, +{ + fn try_into_js_result(self, cx: &mut Context) -> JsResult { + self.and_then(|value| value.try_into_js_result(cx)) + } +} diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 83fade587d4..5eb5a571dac 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -2,10 +2,10 @@ use std::cell::RefCell; -use boa_engine::{js_string, Context, JsError, NativeFunction}; +use boa_engine::{js_string, Context, JsError, NativeFunction, TryIntoJsResult}; use crate::private::IntoJsFunctionSealed; -use crate::{IntoJsFunctionCopied, TryFromJsArgument, TryIntoJsResult, UnsafeIntoJsFunction}; +use crate::{IntoJsFunctionCopied, TryFromJsArgument, UnsafeIntoJsFunction}; /// A token to represent the context argument in the function signature. /// This should not be used directly and has no external meaning. diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 5bc8b72b4bc..809df0ee8b3 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -108,22 +108,6 @@ pub trait IntoJsFunctionCopied: private::IntoJsFunctionSealed NativeFunction; } -/// Create a [`JsResult`] from a Rust value. This trait is used to -/// convert Rust types to JS types, including [`JsResult`] of -/// Rust values and [`JsValue`]s. It is used to convert the -/// return value of a function in [`UnsafeIntoJsFunction`] and -/// [`IntoJsFunctionCopied`]. -pub trait TryIntoJsResult { - /// Try to convert a Rust value into a `JsResult`. - /// - /// # Errors - /// Any parsing errors that may occur during the conversion, or any - /// error that happened during the call to a function. - fn try_into_js_result(self, context: &mut Context) -> JsResult; -} - -mod try_into_js_result_impls; - /// Create a Rust value from a JS argument. This trait is used to /// convert arguments from JS to Rust types. It allows support /// for optional arguments or rest arguments. @@ -349,7 +333,7 @@ fn can_throw_exception() { let module = vec![( js_string!("doTheThrow"), IntoJsFunctionCopied::into_js_function_copied( - |message: JsValue| -> JsResult<()> { JsResult::Err(JsError::from_opaque(message)) }, + |message: JsValue| -> JsResult<()> { Err(JsError::from_opaque(message)) }, &mut context, ), )] diff --git a/core/interop/src/try_into_js_result_impls.rs b/core/interop/src/try_into_js_result_impls.rs deleted file mode 100644 index 1be5e1e92e8..00000000000 --- a/core/interop/src/try_into_js_result_impls.rs +++ /dev/null @@ -1,55 +0,0 @@ -//! Declare implementations of [`TryIntoJsResult`] trait for various types. -//! We cannot rely on a generic implementation based on `TryIntoJs` due -//! to a limitation of the Rust type system which prevents generalization -//! of traits based on an upstream crate. - -use crate::TryIntoJsResult; -use boa_engine::{Context, JsBigInt, JsResult, JsString, JsSymbol, JsValue}; - -impl TryIntoJsResult for Option { - fn try_into_js_result(self, context: &mut Context) -> JsResult { - match self { - Some(value) => value.try_into_js_result(context), - None => Ok(JsValue::undefined()), - } - } -} - -impl TryIntoJsResult for JsResult { - fn try_into_js_result(self, context: &mut Context) -> JsResult { - self.and_then(|value| value.try_into_js_result(context)) - } -} - -macro_rules! impl_try_into_js_result { - ($($t: ty),*) => { - $( - impl TryIntoJsResult for $t { - fn try_into_js_result(self, _context: &mut Context) -> JsResult { - Ok(JsValue::from(self)) - } - } - )* - }; -} - -impl_try_into_js_result!( - bool, - char, - i8, - i16, - i32, - i64, - u8, - u16, - u32, - u64, - usize, - f32, - f64, - JsBigInt, - JsString, - JsSymbol, - JsValue, - () -); From 35fea4e5ff5543f2b50a70534144dae89d18583e Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sun, 7 Apr 2024 21:00:13 -0700 Subject: [PATCH 12/13] Move JsRest to be at the end only of arguments and add JsAll --- core/interop/src/into_js_function_impls.rs | 156 +++++++++++++++------ core/interop/src/lib.rs | 119 ++++++++++++++-- 2 files changed, 219 insertions(+), 56 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 5eb5a571dac..93854adf6f4 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -5,7 +5,7 @@ use std::cell::RefCell; use boa_engine::{js_string, Context, JsError, NativeFunction, TryIntoJsResult}; use crate::private::IntoJsFunctionSealed; -use crate::{IntoJsFunctionCopied, TryFromJsArgument, UnsafeIntoJsFunction}; +use crate::{IntoJsFunctionCopied, JsRest, TryFromJsArgument, UnsafeIntoJsFunction}; /// A token to represent the context argument in the function signature. /// This should not be used directly and has no external meaning. @@ -21,13 +21,27 @@ macro_rules! impl_into_js_function { T: FnMut($($t,)*) -> R + 'static {} - impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* ContextArgToken), R> for T + impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* ContextArgToken,), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, T: FnMut($($t,)* &mut Context) -> R + 'static {} + impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* JsRest,), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: FnMut($($t,)* JsRest) -> R + 'static + {} + + impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* JsRest, ContextArgToken), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: FnMut($($t,)* JsRest, &mut Context) -> R + 'static + {} + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)*), R> for T where $($t: TryFromJsArgument + 'static,)* @@ -44,7 +58,33 @@ macro_rules! impl_into_js_function { let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* match s.try_borrow_mut() { - Ok(mut r) => r( $($id),* ).try_into_js_result(ctx), + Ok(mut r) => r( $($id,)* ).try_into_js_result(ctx), + Err(_) => { + Err(JsError::from_opaque(js_string!("recursive calls to this function not supported").into())) + } + } + }) + } + } + } + + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* JsRest,), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: FnMut($($t,)* JsRest) -> R + 'static, + { + #[allow(unused_variables)] + unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { + let s = RefCell::new(self); + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + match s.try_borrow_mut() { + Ok(mut r) => r( $($id,)* rest.into() ).try_into_js_result(ctx), Err(_) => { Err(JsError::from_opaque(js_string!("recursive calls to this function not supported").into())) } @@ -54,7 +94,7 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* ContextArgToken), R> for T + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* ContextArgToken,), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, @@ -76,6 +116,28 @@ macro_rules! impl_into_js_function { } } + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* JsRest, ContextArgToken), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: FnMut($($t,)* JsRest, &mut Context) -> R + 'static, + { + #[allow(unused_variables)] + unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { + let s = RefCell::new(self); + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s.borrow_mut()( $($id,)* rest.into(), ctx); + r.try_into_js_result(ctx) + }) + } + } + } + // Safe versions for `Fn(..) -> ...`. impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)*), R> for T where @@ -92,18 +154,18 @@ macro_rules! impl_into_js_function { $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* - let r = s( $($id),* ); + let r = s( $($id,)* ); r.try_into_js_result(ctx) }) } } } - impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* ContextArgToken), R> for T + impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* JsRest,), R> for T where $($t: TryFromJsArgument + 'static,)* R: TryIntoJsResult, - T: Fn($($t,)* &mut Context) -> R + 'static + Copy, + T: Fn($($t,)* JsRest) -> R + 'static + Copy, { #[allow(unused_variables)] fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { @@ -114,58 +176,64 @@ macro_rules! impl_into_js_function { $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; )* - let r = s( $($id,)* ctx); + let r = s( $($id,)* rest.into() ); r.try_into_js_result(ctx) }) } } } - }; -} - -impl IntoJsFunctionSealed<(), R> for T -where - R: TryIntoJsResult, - T: FnMut() -> R + 'static, -{ -} -impl UnsafeIntoJsFunction<(), R> for T -where - R: TryIntoJsResult, - T: FnMut() -> R + 'static, -{ - unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { - let s = RefCell::new(self); - unsafe { - NativeFunction::from_closure(move |_this, _args, ctx| { - let r = s.borrow_mut()(); - r.try_into_js_result(ctx) - }) + impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* ContextArgToken,), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: Fn($($t,)* &mut Context) -> R + 'static + Copy, + { + #[allow(unused_variables)] + fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { + let s = self; + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id,)* ctx); + r.try_into_js_result(ctx) + }) + } + } } - } -} -impl IntoJsFunctionCopied<(), R> for T -where - R: TryIntoJsResult, - T: Fn() -> R + 'static + Copy, -{ - fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { - let s = self; - unsafe { - NativeFunction::from_closure(move |_this, _args, ctx| { - let r = s(); - r.try_into_js_result(ctx) - }) + impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* JsRest, ContextArgToken), R> for T + where + $($t: TryFromJsArgument + 'static,)* + R: TryIntoJsResult, + T: Fn($($t,)* JsRest, &mut Context) -> R + 'static + Copy, + { + #[allow(unused_variables)] + fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { + let s = self; + unsafe { + NativeFunction::from_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id,)* rest.into(), ctx); + r.try_into_js_result(ctx) + }) + } + } } - } + }; } // Currently implemented up to 12 arguments. The empty argument list // is implemented separately above. // Consider that JsRest and JsThis are part of this list, but Context // is not, as it is a special specialization of the template. +impl_into_js_function!(); impl_into_js_function!(a: A); impl_into_js_function!(a: A, b: B); impl_into_js_function!(a: A, b: B, c: C); diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 809df0ee8b3..1ff4eca8da5 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -100,7 +100,11 @@ pub trait UnsafeIntoJsFunction: private::IntoJsFunctionSealed: private::IntoJsFunctionSealed + Copy { @@ -139,38 +143,65 @@ impl TryFromJsArgument for T { /// An argument that when used in a JS function will empty the list /// of JS arguments as `JsValue`s. This can be used for having the -/// rest of the arguments in a function. +/// rest of the arguments in a function. It should be the last +/// argument of your function, before the `Context` argument if any. +/// +/// For example, +/// ``` +/// # use boa_engine::{Context, JsValue}; +/// # use boa_interop::{IntoJsFunctionCopied, JsRest}; +/// # let mut context = Context::default(); +/// let sums = (|args: JsRest, context: &mut Context| -> i32 { +/// args.iter().map(|i| i.try_js_into::(context).unwrap()).sum::() +/// }).into_js_function_copied(&mut context); +/// +/// let result = sums.call( +/// &JsValue::undefined(), +/// &[JsValue::from(1), JsValue::from(2), JsValue::from(3)], +/// &mut context +/// ).unwrap(); +/// assert_eq!(result, JsValue::new(6)); +/// ``` #[derive(Debug, Clone)] pub struct JsRest(pub Vec); #[allow(unused)] impl JsRest { /// Consumes the `JsRest` and returns the inner list of `JsValue`. - fn into_inner(self) -> Vec { + #[must_use] + pub fn into_inner(self) -> Vec { self.0 } /// Returns an iterator over the inner list of `JsValue`. - fn iter(&self) -> impl Iterator { + pub fn iter(&self) -> impl Iterator { self.0.iter() } /// Returns a mutable iterator over the inner list of `JsValue`. - fn iter_mut(&mut self) -> impl Iterator { + pub fn iter_mut(&mut self) -> impl Iterator { self.0.iter_mut() } /// Returns the length of the inner list of `JsValue`. - fn len(&self) -> usize { + #[must_use] + pub fn len(&self) -> usize { self.0.len() } /// Returns `true` if the inner list of `JsValue` is empty. - fn is_empty(&self) -> bool { + #[must_use] + pub fn is_empty(&self) -> bool { self.0.is_empty() } } +impl From<&[JsValue]> for JsRest { + fn from(values: &[JsValue]) -> Self { + Self(values.to_vec()) + } +} + impl IntoIterator for JsRest { type Item = JsValue; type IntoIter = std::vec::IntoIter; @@ -180,13 +211,77 @@ impl IntoIterator for JsRest { } } -impl TryFromJsArgument for JsRest { +/// An argument that when used in a JS function will capture all +/// the arguments that can be converted to `T`. The first argument +/// that cannot be converted to `T` will stop the conversion. +/// +/// For example, +/// ``` +/// # use boa_engine::{Context, JsValue}; +/// # use boa_interop::{IntoJsFunctionCopied, JsAll}; +/// # let mut context = Context::default(); +/// let sums = (|args: JsAll, context: &mut Context| -> i32 { +/// args.iter().sum() +/// }).into_js_function_copied(&mut context); +/// +/// let result = sums.call( +/// &JsValue::undefined(), +/// &[JsValue::from(1), JsValue::from(2), JsValue::from(3), JsValue::Boolean(true), JsValue::from(4)], +/// &mut context +/// ).unwrap(); +/// assert_eq!(result, JsValue::new(6)); +/// ``` +#[derive(Debug, Clone)] +pub struct JsAll(pub Vec); + +impl JsAll { + /// Consumes the `JsAll` and returns the inner list of `T`. + #[must_use] + pub fn into_inner(self) -> Vec { + self.0 + } + + /// Returns an iterator over the inner list of `T`. + pub fn iter(&self) -> impl Iterator { + self.0.iter() + } + + /// Returns a mutable iterator over the inner list of `T`. + pub fn iter_mut(&mut self) -> impl Iterator { + self.0.iter_mut() + } + + /// Returns the length of the inner list of `T`. + #[must_use] + pub fn len(&self) -> usize { + self.0.len() + } + + /// Returns `true` if the inner list of `T` is empty. + #[must_use] + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + +impl TryFromJsArgument for JsAll { fn try_from_js_argument<'a>( - _: &'a JsValue, - rest: &'a [JsValue], - _context: &mut Context, + _this: &'a JsValue, + mut rest: &'a [JsValue], + context: &mut Context, ) -> JsResult<(Self, &'a [JsValue])> { - Ok((JsRest(rest.to_vec()), &[])) + let mut values = Vec::new(); + + while !rest.is_empty() { + match rest[0].try_js_into(context) { + Ok(value) => { + values.push(value); + rest = &rest[1..]; + } + Err(_) => break, + } + } + Ok((JsAll(values), rest)) } } From deda29b354578c72bc5ed27497a02c98599b25a2 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 8 Apr 2024 21:54:42 -0700 Subject: [PATCH 13/13] use from_copy_closure and remove unsafe --- core/interop/src/into_js_function_impls.rs | 72 ++++++++++------------ 1 file changed, 32 insertions(+), 40 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index 93854adf6f4..d0d3093195f 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -148,16 +148,14 @@ macro_rules! impl_into_js_function { #[allow(unused_variables)] fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { let s = self; - unsafe { - NativeFunction::from_closure(move |this, args, ctx| { - let rest = args; - $( - let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; - )* - let r = s( $($id,)* ); - r.try_into_js_result(ctx) - }) - } + NativeFunction::from_copy_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id,)* ); + r.try_into_js_result(ctx) + }) } } @@ -170,16 +168,14 @@ macro_rules! impl_into_js_function { #[allow(unused_variables)] fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { let s = self; - unsafe { - NativeFunction::from_closure(move |this, args, ctx| { - let rest = args; - $( - let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; - )* - let r = s( $($id,)* rest.into() ); - r.try_into_js_result(ctx) - }) - } + NativeFunction::from_copy_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id,)* rest.into() ); + r.try_into_js_result(ctx) + }) } } @@ -192,16 +188,14 @@ macro_rules! impl_into_js_function { #[allow(unused_variables)] fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { let s = self; - unsafe { - NativeFunction::from_closure(move |this, args, ctx| { - let rest = args; - $( - let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; - )* - let r = s( $($id,)* ctx); - r.try_into_js_result(ctx) - }) - } + NativeFunction::from_copy_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id,)* ctx); + r.try_into_js_result(ctx) + }) } } @@ -214,16 +208,14 @@ macro_rules! impl_into_js_function { #[allow(unused_variables)] fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { let s = self; - unsafe { - NativeFunction::from_closure(move |this, args, ctx| { - let rest = args; - $( - let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; - )* - let r = s( $($id,)* rest.into(), ctx); - r.try_into_js_result(ctx) - }) - } + NativeFunction::from_copy_closure(move |this, args, ctx| { + let rest = args; + $( + let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; + )* + let r = s( $($id,)* rest.into(), ctx); + r.try_into_js_result(ctx) + }) } } };