From c721858338b27c6cbf74edbf655c3bbf52feac06 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 28 Mar 2024 22:41:16 -0700 Subject: [PATCH 01/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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) + }) } } }; From f6dd8a3fe365530d21ff4e4b8e3f741231d7cdc2 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 9 Apr 2024 12:20:45 -0700 Subject: [PATCH 14/22] Add a HostDefined struct to grab host defined in the argument list --- core/interop/src/lib.rs | 44 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 1ff4eca8da5..9cddca53359 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -1,8 +1,10 @@ //! Interop utilities between Boa and its host. +use boa_engine::{ + Context, js_string, JsError, JsResult, JsString, JsValue, Module, NativeFunction, NativeObject, +}; use boa_engine::module::SyntheticModuleInitializer; use boa_engine::value::TryFromJs; -use boa_engine::{Context, JsResult, JsString, JsValue, Module, NativeFunction}; pub mod loaders; @@ -301,6 +303,46 @@ impl TryFromJsArgument for JsThis { } } +/// Captures the host_defined value as a JS function argument, based on +/// its type. This will clone the HostDefined value. +/// +/// For example, +/// ``` +/// # use boa_engine::{Context, Finalize, JsData, JsValue, Trace}; +/// use boa_interop::{IntoJsFunctionCopied, HostDefined}; +/// +/// #[derive(Clone, Debug, Finalize, JsData, Trace)] +/// struct CustomHostDefinedStruct { +/// #[unsafe_ignore_trace] +/// pub counter: usize, +/// } +/// let mut context = Context::default(); +/// context.realm().host_defined_mut().insert(CustomHostDefinedStruct { counter: 123 }); +/// let f = (|HostDefined(host): HostDefined| { +/// host.counter + 1 +/// }).into_js_function_copied(&mut context); +/// +/// assert_eq!(f.call(&JsValue::undefined(), &[], &mut context), Ok(JsValue::new(124))); +/// +/// ``` +#[derive(Debug, Clone)] +pub struct HostDefined(pub T); + +impl TryFromJsArgument for HostDefined { + fn try_from_js_argument<'a>( + _this: &'a JsValue, + rest: &'a [JsValue], + context: &mut Context, + ) -> JsResult<(Self, &'a [JsValue])> { + match context.realm().host_defined().get::() { + Some(value) => Ok((HostDefined(value.clone()), rest)), + None => Err(JsError::from_opaque( + js_string!("Host defined value not found").into(), + )), + } + } +} + // Implement `IntoJsFunction` for functions with a various list of // arguments. mod into_js_function_impls; From fc0c7dcaa16ac29a83fb9b0ab4ebda7d191546cc Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 9 Apr 2024 12:24:40 -0700 Subject: [PATCH 15/22] cargo fmt and clippy --- core/interop/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 9cddca53359..6162e0fd468 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -1,10 +1,10 @@ //! Interop utilities between Boa and its host. -use boa_engine::{ - Context, js_string, JsError, JsResult, JsString, JsValue, Module, NativeFunction, NativeObject, -}; use boa_engine::module::SyntheticModuleInitializer; use boa_engine::value::TryFromJs; +use boa_engine::{ + js_string, Context, JsError, JsResult, JsString, JsValue, Module, NativeFunction, NativeObject, +}; pub mod loaders; @@ -303,8 +303,8 @@ impl TryFromJsArgument for JsThis { } } -/// Captures the host_defined value as a JS function argument, based on -/// its type. This will clone the HostDefined value. +/// Captures the `host_defined` value as a JS function argument, based on +/// its type. This will clone the `HostDefined` value. /// /// For example, /// ``` From cbaee86597b7a7f08e65e3f4310109fefbd87721 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 9 Apr 2024 12:38:37 -0700 Subject: [PATCH 16/22] Explain why we need Clone --- core/interop/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 6162e0fd468..22241107b99 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -306,6 +306,9 @@ impl TryFromJsArgument for JsThis { /// Captures the `host_defined` value as a JS function argument, based on /// its type. This will clone the `HostDefined` value. /// +/// The host defined type must implement [`Clone`], otherwise the borrow +/// checker would not be able to ensure the safety of the operation. +/// /// For example, /// ``` /// # use boa_engine::{Context, Finalize, JsData, JsValue, Trace}; From 644937bb4edd5905f8c667bd34d08bac39120c7d Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 9 Apr 2024 12:48:43 -0700 Subject: [PATCH 17/22] Remove the vector from JsRest<> and use a reference --- core/interop/src/into_js_function_impls.rs | 48 ++++++++++---------- core/interop/src/lib.rs | 53 +++++++++++----------- 2 files changed, 51 insertions(+), 50 deletions(-) diff --git a/core/interop/src/into_js_function_impls.rs b/core/interop/src/into_js_function_impls.rs index d0d3093195f..772a9914f85 100644 --- a/core/interop/src/into_js_function_impls.rs +++ b/core/interop/src/into_js_function_impls.rs @@ -16,35 +16,35 @@ macro_rules! impl_into_js_function { ($($id: ident: $t: ident),*) => { impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)*), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, T: FnMut($($t,)*) -> R + 'static {} impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* ContextArgToken,), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, T: FnMut($($t,)* &mut Context) -> R + 'static {} - impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* JsRest,), R> for T + impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* JsRest<'_>,), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, - T: FnMut($($t,)* JsRest) -> R + 'static + T: FnMut($($t,)* JsRest<'_>) -> R + 'static {} - impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* JsRest, ContextArgToken), R> for T + impl<$($t,)* R, T> IntoJsFunctionSealed<($($t,)* JsRest<'_>, ContextArgToken), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, - T: FnMut($($t,)* JsRest, &mut Context) -> R + 'static + T: FnMut($($t,)* JsRest<'_>, &mut Context) -> R + 'static {} impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)*), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, T: FnMut($($t,)*) -> R + 'static, { @@ -68,11 +68,11 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* JsRest,), R> for T + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* JsRest<'_>,), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, - T: FnMut($($t,)* JsRest) -> R + 'static, + T: FnMut($($t,)* JsRest<'_>) -> R + 'static, { #[allow(unused_variables)] unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { @@ -96,7 +96,7 @@ macro_rules! impl_into_js_function { impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* ContextArgToken,), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, T: FnMut($($t,)* &mut Context) -> R + 'static, { @@ -116,11 +116,11 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* JsRest, ContextArgToken), R> for T + impl<$($t,)* R, T> UnsafeIntoJsFunction<($($t,)* JsRest<'_>, ContextArgToken), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, - T: FnMut($($t,)* JsRest, &mut Context) -> R + 'static, + T: FnMut($($t,)* JsRest<'_>, &mut Context) -> R + 'static, { #[allow(unused_variables)] unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { @@ -141,7 +141,7 @@ macro_rules! impl_into_js_function { // Safe versions for `Fn(..) -> ...`. impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)*), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, T: Fn($($t,)*) -> R + 'static + Copy, { @@ -159,11 +159,11 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* JsRest,), R> for T + impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* JsRest<'_>,), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, - T: Fn($($t,)* JsRest) -> R + 'static + Copy, + T: Fn($($t,)* JsRest<'_>) -> R + 'static + Copy, { #[allow(unused_variables)] fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { @@ -181,7 +181,7 @@ macro_rules! impl_into_js_function { impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* ContextArgToken,), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, T: Fn($($t,)* &mut Context) -> R + 'static + Copy, { @@ -199,11 +199,11 @@ macro_rules! impl_into_js_function { } } - impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* JsRest, ContextArgToken), R> for T + impl<$($t,)* R, T> IntoJsFunctionCopied<($($t,)* JsRest<'_>, ContextArgToken), R> for T where - $($t: TryFromJsArgument + 'static,)* + $($t: for<'a> TryFromJsArgument<'a> + 'static,)* R: TryIntoJsResult, - T: Fn($($t,)* JsRest, &mut Context) -> R + 'static + Copy, + T: Fn($($t,)* JsRest<'_>, &mut Context) -> R + 'static + Copy, { #[allow(unused_variables)] fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 22241107b99..9da1bbe0f40 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -117,21 +117,21 @@ pub trait IntoJsFunctionCopied: private::IntoJsFunctionSealed: 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>( + fn try_from_js_argument( this: &'a JsValue, rest: &'a [JsValue], context: &mut Context, ) -> JsResult<(Self, &'a [JsValue])>; } -impl TryFromJsArgument for T { - fn try_from_js_argument<'a>( +impl<'a, T: TryFromJs> TryFromJsArgument<'a> for T { + fn try_from_js_argument( _: &'a JsValue, rest: &'a [JsValue], context: &mut Context, @@ -165,26 +165,27 @@ impl TryFromJsArgument for T { /// assert_eq!(result, JsValue::new(6)); /// ``` #[derive(Debug, Clone)] -pub struct JsRest(pub Vec); +pub struct JsRest<'a>(pub &'a [JsValue]); #[allow(unused)] -impl JsRest { +impl<'a> JsRest<'a> { /// Consumes the `JsRest` and returns the inner list of `JsValue`. #[must_use] - pub fn into_inner(self) -> Vec { + pub fn into_inner(self) -> &'a [JsValue] { self.0 } + /// Transforms the `JsRest` into a `Vec`. + #[must_use] + pub fn to_vec(self) -> Vec { + self.0.to_vec() + } + /// Returns an iterator over the inner list of `JsValue`. pub fn iter(&self) -> impl Iterator { self.0.iter() } - /// Returns a mutable iterator over the inner list of `JsValue`. - pub fn iter_mut(&mut self) -> impl Iterator { - self.0.iter_mut() - } - /// Returns the length of the inner list of `JsValue`. #[must_use] pub fn len(&self) -> usize { @@ -198,18 +199,18 @@ impl JsRest { } } -impl From<&[JsValue]> for JsRest { - fn from(values: &[JsValue]) -> Self { - Self(values.to_vec()) +impl<'a> From<&'a [JsValue]> for JsRest<'a> { + fn from(values: &'a [JsValue]) -> Self { + Self(values) } } -impl IntoIterator for JsRest { - type Item = JsValue; - type IntoIter = std::vec::IntoIter; +impl<'a> IntoIterator for JsRest<'a> { + type Item = &'a JsValue; + type IntoIter = std::slice::Iter<'a, JsValue>; fn into_iter(self) -> Self::IntoIter { - self.into_inner().into_iter() + self.into_inner().iter() } } @@ -266,8 +267,8 @@ impl JsAll { } } -impl TryFromJsArgument for JsAll { - fn try_from_js_argument<'a>( +impl<'a, T: TryFromJs> TryFromJsArgument<'a> for JsAll { + fn try_from_js_argument( _this: &'a JsValue, mut rest: &'a [JsValue], context: &mut Context, @@ -293,8 +294,8 @@ impl TryFromJsArgument for JsAll { #[derive(Debug, Clone)] pub struct JsThis(pub T); -impl TryFromJsArgument for JsThis { - fn try_from_js_argument<'a>( +impl<'a, T: TryFromJs> TryFromJsArgument<'a> for JsThis { + fn try_from_js_argument( this: &'a JsValue, rest: &'a [JsValue], context: &mut Context, @@ -331,8 +332,8 @@ impl TryFromJsArgument for JsThis { #[derive(Debug, Clone)] pub struct HostDefined(pub T); -impl TryFromJsArgument for HostDefined { - fn try_from_js_argument<'a>( +impl<'a, T: NativeObject + Clone> TryFromJsArgument<'a> for HostDefined { + fn try_from_js_argument( _this: &'a JsValue, rest: &'a [JsValue], context: &mut Context, @@ -398,7 +399,7 @@ pub fn into_js_module() { UnsafeIntoJsFunction::into_js_function_unsafe( { let counter = dad_count.clone(); - move |args: JsRest, context: &mut Context| { + move |args: JsRest<'_>, context: &mut Context| { *counter.borrow_mut() += args .into_iter() .map(|i| i.try_js_into::(context).unwrap()) From 2c2e1fb0607fe122a3718192ad94c3e7a32e214b Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 10 Apr 2024 11:19:13 -0700 Subject: [PATCH 18/22] Add HostDefined to context as Data, and rename the injector in boa_interop --- core/engine/src/context/mod.rs | 31 +++++++++++++++++++++- core/interop/src/lib.rs | 47 ++++++++++++++++++---------------- 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/core/engine/src/context/mod.rs b/core/engine/src/context/mod.rs index c69ebd5ec86..32df528b35d 100644 --- a/core/engine/src/context/mod.rs +++ b/core/engine/src/context/mod.rs @@ -25,7 +25,7 @@ use crate::{ realm::Realm, script::Script, vm::{ActiveRunnable, CallFrame, Vm}, - JsNativeError, JsResult, JsString, JsValue, Source, + HostDefined, JsNativeError, JsResult, JsString, JsValue, NativeObject, Source, }; use self::intrinsics::StandardConstructor; @@ -115,6 +115,8 @@ pub struct Context { /// Unique identifier for each parser instance used during the context lifetime. parser_identifier: u32, + + data: HostDefined, } impl std::fmt::Debug for Context { @@ -585,6 +587,32 @@ impl Context { pub fn can_block(&self) -> bool { self.can_block } + + /// Insert a type into the context-specific [`HostDefined`] field. + #[inline] + pub fn insert_data(&mut self, value: T) -> Option> { + self.data.insert(value) + } + + /// Check if the context-specific [`HostDefined`] has type T. + #[inline] + #[must_use] + pub fn has_data(&self) -> bool { + self.data.has::() + } + + /// Remove type T from the context-specific [`HostDefined`], if it exists. + #[inline] + pub fn remove_data(&mut self) -> Option> { + self.data.remove::() + } + + /// Get type T from the context-specific [`HostDefined`], if it exists. + #[inline] + #[must_use] + pub fn get_data(&self) -> Option<&T> { + self.data.get::() + } } // ==== Private API ==== @@ -1070,6 +1098,7 @@ impl ContextBuilder { root_shape, parser_identifier: 0, can_block: self.can_block, + data: HostDefined::default(), }; builtins::set_default_global_bindings(&mut context)?; diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 9da1bbe0f40..cd7e1a48347 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -304,16 +304,18 @@ impl<'a, T: TryFromJs> TryFromJsArgument<'a> for JsThis { } } -/// Captures the `host_defined` value as a JS function argument, based on -/// its type. This will clone the `HostDefined` value. +/// Captures a [`ContextData`] data from the [`Context`] as a JS function argument, +/// based on its type. /// /// The host defined type must implement [`Clone`], otherwise the borrow -/// checker would not be able to ensure the safety of the operation. +/// checker would not be able to ensure the safety of the context while +/// making the function call. Because of this, it is recommended to use +/// types that are cheap to clone. /// /// For example, /// ``` /// # use boa_engine::{Context, Finalize, JsData, JsValue, Trace}; -/// use boa_interop::{IntoJsFunctionCopied, HostDefined}; +/// use boa_interop::{IntoJsFunctionCopied, ContextData}; /// /// #[derive(Clone, Debug, Finalize, JsData, Trace)] /// struct CustomHostDefinedStruct { @@ -321,25 +323,24 @@ impl<'a, T: TryFromJs> TryFromJsArgument<'a> for JsThis { /// pub counter: usize, /// } /// let mut context = Context::default(); -/// context.realm().host_defined_mut().insert(CustomHostDefinedStruct { counter: 123 }); -/// let f = (|HostDefined(host): HostDefined| { +/// context.insert_data(CustomHostDefinedStruct { counter: 123 }); +/// let f = (|ContextData(host): ContextData| { /// host.counter + 1 /// }).into_js_function_copied(&mut context); /// /// assert_eq!(f.call(&JsValue::undefined(), &[], &mut context), Ok(JsValue::new(124))); -/// /// ``` #[derive(Debug, Clone)] -pub struct HostDefined(pub T); +pub struct ContextData(pub T); -impl<'a, T: NativeObject + Clone> TryFromJsArgument<'a> for HostDefined { +impl<'a, T: NativeObject + Clone> TryFromJsArgument<'a> for ContextData { fn try_from_js_argument( _this: &'a JsValue, rest: &'a [JsValue], context: &mut Context, ) -> JsResult<(Self, &'a [JsValue])> { - match context.realm().host_defined().get::() { - Some(value) => Ok((HostDefined(value.clone()), rest)), + match context.get_data::() { + Some(value) => Ok((ContextData(value.clone()), rest)), None => Err(JsError::from_opaque( js_string!("Host defined value not found").into(), )), @@ -355,9 +356,12 @@ mod into_js_function_impls; #[allow(clippy::missing_panics_doc)] pub fn into_js_module() { use boa_engine::{js_string, JsValue, Source}; + use boa_gc::{Gc, GcRefCell}; use std::cell::RefCell; use std::rc::Rc; + type ResultType = Gc>; + let loader = Rc::new(loaders::HashMapModuleLoader::new()); let mut context = Context::builder() .module_loader(loader.clone()) @@ -367,7 +371,9 @@ pub fn into_js_module() { 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())); + + context.insert_data(Gc::new(GcRefCell::new(JsValue::undefined()))); + let module = unsafe { vec![ ( @@ -411,15 +417,10 @@ pub fn into_js_module() { ), ( js_string!("send"), - UnsafeIntoJsFunction::into_js_function_unsafe( - { - let result = result.clone(); - move |value: JsValue| { - *result.borrow_mut() = value; - } - }, - &mut context, - ), + (move |value: JsValue, ContextData(result): ContextData| { + *result.borrow_mut() = value; + }) + .into_js_function_copied(&mut context), ), ] } @@ -454,10 +455,12 @@ pub fn into_js_module() { promise_result.state() ); + let result = context.get_data::().unwrap().borrow().clone(); + 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)); + assert_eq!(result.try_js_into(&mut context), Ok(1u32)); } #[test] From fffb5266c078977f84fe1b3491aabc4569be8ee1 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 11 Apr 2024 08:18:39 -0700 Subject: [PATCH 19/22] Use TypeError instead if the type was not found in context --- core/interop/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index cd7e1a48347..9e3e0d79f19 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -3,7 +3,8 @@ use boa_engine::module::SyntheticModuleInitializer; use boa_engine::value::TryFromJs; use boa_engine::{ - js_string, Context, JsError, JsResult, JsString, JsValue, Module, NativeFunction, NativeObject, + js_string, Context, JsError, JsNativeError, JsResult, JsString, JsValue, Module, + NativeFunction, NativeObject, }; pub mod loaders; @@ -341,8 +342,8 @@ impl<'a, T: NativeObject + Clone> TryFromJsArgument<'a> for ContextData { ) -> JsResult<(Self, &'a [JsValue])> { match context.get_data::() { Some(value) => Ok((ContextData(value.clone()), rest)), - None => Err(JsError::from_opaque( - js_string!("Host defined value not found").into(), + None => Err(JsError::from_native( + JsNativeError::typ().with_cause("Context data not found"), )), } } From 05f178190c938186e16a6909b71007b03bbfa2ab Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 11 Apr 2024 13:34:05 -0700 Subject: [PATCH 20/22] Update core/interop/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Julián Espina --- core/interop/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 9e3e0d79f19..715392aae71 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -342,9 +342,10 @@ impl<'a, T: NativeObject + Clone> TryFromJsArgument<'a> for ContextData { ) -> JsResult<(Self, &'a [JsValue])> { match context.get_data::() { Some(value) => Ok((ContextData(value.clone()), rest)), - None => Err(JsError::from_native( - JsNativeError::typ().with_cause("Context data not found"), - )), + None => Err(JsNativeError::typ() + .with_message("Context data not found") + .into(), + ), } } } From 73e71f2f0bbab3fe889b59af7254159ea279f831 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 11 Apr 2024 13:36:11 -0700 Subject: [PATCH 21/22] cargo fmt --- core/interop/src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 715392aae71..6f9083f23ad 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -3,7 +3,7 @@ use boa_engine::module::SyntheticModuleInitializer; use boa_engine::value::TryFromJs; use boa_engine::{ - js_string, Context, JsError, JsNativeError, JsResult, JsString, JsValue, Module, + Context, JsNativeError, JsResult, JsString, JsValue, Module, NativeFunction, NativeObject, }; @@ -344,8 +344,7 @@ impl<'a, T: NativeObject + Clone> TryFromJsArgument<'a> for ContextData { Some(value) => Ok((ContextData(value.clone()), rest)), None => Err(JsNativeError::typ() .with_message("Context data not found") - .into(), - ), + .into()), } } } From 911052e57d80c86c11893d5d87b5039df2eda10d Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 11 Apr 2024 13:39:01 -0700 Subject: [PATCH 22/22] cargo fmt --- core/interop/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index 6f9083f23ad..b03d8adc213 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -3,8 +3,7 @@ use boa_engine::module::SyntheticModuleInitializer; use boa_engine::value::TryFromJs; use boa_engine::{ - Context, JsNativeError, JsResult, JsString, JsValue, Module, - NativeFunction, NativeObject, + Context, JsNativeError, JsResult, JsString, JsValue, Module, NativeFunction, NativeObject, }; pub mod loaders;