From 9f51d64c1ea17dac002ba121aa413414b31eb678 Mon Sep 17 00:00:00 2001 From: Billy Bradley Date: Tue, 30 Nov 2021 11:19:48 +0000 Subject: [PATCH 01/24] Enable passing String vectors and boxed slices across ABI This is accomplished via conversion of the Strings to/from JsValues. --- src/convert/slices.rs | 123 +++++++++++++++++++++++++++++++++++++++++- src/describe.rs | 11 +++- 2 files changed, 130 insertions(+), 4 deletions(-) diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 40ea6edc7c8..3afe75cc8cb 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -1,3 +1,4 @@ +use std::convert::{TryFrom, TryInto}; #[cfg(feature = "std")] use std::prelude::v1::*; @@ -10,6 +11,8 @@ use crate::convert::OptionIntoWasmAbi; use crate::convert::{ FromWasmAbi, IntoWasmAbi, LongRefFromWasmAbi, RefFromWasmAbi, RefMutFromWasmAbi, WasmAbi, }; +use crate::describe; +use crate::describe::WasmDescribe; use cfg_if::cfg_if; if_std! { @@ -77,6 +80,13 @@ if_std! { macro_rules! vectors { ($($t:ident)*) => ($( if_std! { + impl WasmDescribe for Box<[$t]> { + fn describe() { + describe::inform(describe::VECTOR); + $t::describe(); + } + } + impl IntoWasmAbi for Box<[$t]> { type Abi = WasmSlice; @@ -151,7 +161,7 @@ macro_rules! vectors { #[inline] unsafe fn ref_from_abi(js: WasmSlice) -> Box<[$t]> { - >::from_abi(js) + as FromWasmAbi>::from_abi(js) } } @@ -161,7 +171,7 @@ macro_rules! vectors { #[inline] unsafe fn ref_mut_from_abi(js: WasmMutSlice) -> MutSlice<$t> { - let contents = >::from_abi(js.slice); + let contents = as FromWasmAbi>::from_abi(js.slice); let js = JsValue::from_abi(js.idx); MutSlice { contents, js } } @@ -183,6 +193,115 @@ vectors! { u8 i8 u16 i16 u32 i32 u64 i64 usize isize f32 f64 } +/// Enables blanket implementations of `WasmDescribe`, `IntoWasmAbi`, +/// `FromWasmAbi` and `OptionIntoWasmAbi` functionality on boxed slices of +/// types which can be converted to and from `JsValue` without conflicting +/// implementations of those traits. +/// +/// Implementing these traits directly with blanket implementations would +/// be much more elegant, but unfortunately that's impossible because it +/// conflicts with the implementations for `Box<[T]> where T: JsObject`. +pub trait JsValueVector { + type ToAbi; + type FromAbi; + + fn describe(); + fn into_abi(self) -> Self::ToAbi; + fn none() -> Self::ToAbi; + unsafe fn from_abi(js: Self::FromAbi) -> Self; +} + +/* + * Generates implementations for traits necessary for passing types to and from + * JavaScript on boxed slices of values which can be converted to and from + * `JsValue`. + */ +macro_rules! js_value_vectors { + ($($t:ident)*) => ($( + if_std! { + impl WasmDescribe for Box<[$t]> { + fn describe() { + ::describe(); + } + } + + impl IntoWasmAbi for Box<[$t]> { + type Abi = ::ToAbi; + + fn into_abi(self) -> Self::Abi { + ::into_abi(self) + } + } + + impl OptionIntoWasmAbi for Box<[$t]> { + fn none() -> Self::Abi { + ::none() + } + } + + impl FromWasmAbi for Box<[$t]> { + type Abi = ::FromAbi; + + unsafe fn from_abi(js: Self::Abi) -> Self { + ::from_abi(js) + } + } + } + )*) +} + +if_std! { + impl JsValueVector for Box<[T]> where + T: Into + TryFrom, + >::Error: core::fmt::Debug { + type ToAbi = as IntoWasmAbi>::Abi; + type FromAbi = as FromWasmAbi>::Abi; + + fn describe() { + describe::inform(describe::VECTOR); + JsValue::describe(); + } + + fn into_abi(self) -> Self::ToAbi { + let js_vals: Box::<[JsValue]> = self + .into_vec() + .into_iter() + .map(|x| x.into()) + .collect(); + + IntoWasmAbi::into_abi(js_vals) + } + + fn none() -> Self::ToAbi { + as OptionIntoWasmAbi>::none() + } + + unsafe fn from_abi(js: Self::FromAbi) -> Self { + let js_vals = as FromWasmAbi>::from_abi(js); + + js_vals + .into_iter() + .map(|x| x.try_into().expect("Array element of wrong type")) + .collect() + } + } + + impl TryFrom for String { + type Error = (); + + fn try_from(value: JsValue) -> Result { + match value.as_string() { + Some(s) => Ok(s), + None => Err(()), + } + } + } +} + +js_value_vectors! { + String +} + cfg_if! { if #[cfg(feature = "enable-interning")] { #[inline] diff --git a/src/describe.rs b/src/describe.rs index be149d7a90b..5a45378c352 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -3,7 +3,7 @@ #![doc(hidden)] -use crate::{Clamped, JsError, JsValue}; +use crate::{Clamped, JsError, JsObject, JsValue}; use cfg_if::cfg_if; macro_rules! tys { @@ -145,7 +145,14 @@ if_std! { } } - impl WasmDescribe for Box<[T]> { + impl WasmDescribe for Box<[JsValue]> { + fn describe() { + inform(VECTOR); + JsValue::describe(); + } + } + + impl WasmDescribe for Box<[T]> where T: JsObject { fn describe() { inform(VECTOR); T::describe(); From 5d269523baf0222a306dcb9c5b4f26654144b1df Mon Sep 17 00:00:00 2001 From: Billy Bradley Date: Tue, 7 Dec 2021 13:22:39 +0000 Subject: [PATCH 02/24] Enable passing custom struct vectors over ABI This was done by converting the structs to/from JsValues. It was necessary to change the way relevant traits (e.g. WasmDescribe, IntoWasmAbi etc.) are implemented. It's impossible to implement these for `Box<[#name]>` in codegen.rs because implementing traits on generic types is only allowed in the crate in which the trait is defined. Naively adding a blanket implementation on the wasm-bindgen side doesn't work either because it conflicts with the implementation for JsObjects. The solution was to create traits like VectorIntoWasmAbi etc. that are defined on the concrete type and contain the implementation for IntoWasmAbi etc. for vectors of that type. JsObjects are blanket implemented as before, and struct types are implemented in codegen.rs. Due to the way these traits are defined, Rust requires implementing types to be Sized, so they can't be used for the existing String implementations. Converting structs from JsValues was accomplished by adding an unwrap function to the generated JavaScript class, and calling that from Rust. --- crates/backend/src/codegen.rs | 73 +++++++++ crates/cli-support/src/js/mod.rs | 28 ++++ crates/cli-support/src/wit/mod.rs | 35 ++++- crates/cli-support/src/wit/nonstandard.rs | 5 + crates/cli-support/src/wit/section.rs | 3 + crates/shared/src/lib.rs | 7 + src/convert/impls.rs | 47 +++++- src/convert/slices.rs | 171 ++++++++++------------ src/convert/traits.rs | 43 ++++++ src/describe.rs | 16 +- 10 files changed, 321 insertions(+), 107 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 08e6c31869c..1ab90db0abb 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -167,6 +167,7 @@ impl ToTokens for ast::Struct { let name_chars = name_str.chars().map(|c| c as u32); let new_fn = Ident::new(&shared::new_function(&name_str), Span::call_site()); let free_fn = Ident::new(&shared::free_function(&name_str), Span::call_site()); + let unwrap_fn = Ident::new(&shared::unwrap_function(&name_str), Span::call_site()); let wasm_bindgen = &self.wasm_bindgen; (quote! { #[automatically_derived] @@ -293,6 +294,78 @@ impl ToTokens for ast::Struct { #[inline] fn is_none(abi: &Self::Abi) -> bool { *abi == 0 } } + + #[allow(clippy::all)] + impl wasm_bindgen::__rt::core::convert::TryFrom for #name { + type Error = (); + + fn try_from(value: wasm_bindgen::JsValue) + -> wasm_bindgen::__rt::std::result::Result { + let js_ptr = wasm_bindgen::convert::IntoWasmAbi::into_abi(value); + + #[link(wasm_import_module = "__wbindgen_placeholder__")] + #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))] + extern "C" { + fn #unwrap_fn(ptr: u32) -> u32; + } + + #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))] + unsafe fn #unwrap_fn(_: u32) -> u32 { + panic!("cannot convert from JsValue outside of the wasm target") + } + + let ptr; + unsafe { + ptr = #unwrap_fn(js_ptr); + } + + if(ptr == 0) { + wasm_bindgen::__rt::std::result::Result::Err(()) + } else { + unsafe { + wasm_bindgen::__rt::std::result::Result::Ok( + ::from_abi(ptr) + ) + } + } + } + } + + impl wasm_bindgen::describe::WasmDescribeVector for #name { + use wasm_bindgen::__rt::std::boxed::Box; + + fn describe_vector() { + as wasm_bindgen::convert::JsValueVector>::describe(); + } + } + + impl wasm_bindgen::convert::VectorIntoWasmAbi for #name { + use wasm_bindgen::__rt::std::boxed::Box; + + type Abi = as wasm_bindgen::convert::JsValueVector>::ToAbi; + + fn vector_into_abi(vector: Box<[#name]>) -> Self::Abi { + as wasm_bindgen::convert::JsValueVector>::into_abi(vector) + } + } + + impl wasm_bindgen::convert::OptionVectorIntoWasmAbi for #name { + use wasm_bindgen::__rt::std::boxed::Box; + + fn vector_none() -> as wasm_bindgen::convert::JsValueVector>::ToAbi { + as wasm_bindgen::convert::JsValueVector>::none() + } + } + + impl wasm_bindgen::convert::VectorFromWasmAbi for #name { + use wasm_bindgen::__rt::std::boxed::Box; + + type Abi = as wasm_bindgen::convert::JsValueVector>::FromAbi; + + unsafe fn vector_from_abi(js: Self::Abi) -> Box<[#name]> { + as wasm_bindgen::convert::JsValueVector>::from_abi(js) + } + } }) .to_tokens(tokens); diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 56847262544..6eea1178b80 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -75,6 +75,7 @@ pub struct ExportedClass { generate_typescript: bool, has_constructor: bool, wrap_needed: bool, + unwrap_needed: bool, /// Whether to generate helper methods for inspecting the class is_inspectable: bool, /// All readable properties of the class @@ -935,6 +936,20 @@ impl<'a> Context<'a> { )); } + if class.unwrap_needed { + dst.push_str(&format!( + " + static __unwrap(jsValue) {{ + if (!(jsValue instanceof {})) {{ + return 0; + }} + return jsValue.__destroy_into_raw(); + }} + ", + name, + )); + } + if self.config.weak_refs { self.global(&format!( "const {}Finalization = new FinalizationRegistry(ptr => wasm.{}(ptr >>> 0));", @@ -2247,6 +2262,10 @@ impl<'a> Context<'a> { require_class(&mut self.exported_classes, name).wrap_needed = true; } + fn require_class_unwrap(&mut self, name: &str) { + require_class(&mut self.exported_classes, name).unwrap_needed = true; + } + fn add_module_import(&mut self, module: String, name: &str, actual: &str) { let rename = if name == actual { None @@ -3213,6 +3232,15 @@ impl<'a> Context<'a> { See https://rustwasm.github.io/wasm-bindgen/reference/cli.html#--split-linked-modules for details.", path)) } } + + AuxImport::UnwrapExportedClass(class) => { + assert!(kind == AdapterJsImportKind::Normal); + assert!(!variadic); + assert_eq!(args.len(), 1); + self.require_class_unwrap(class); + self.expose_take_object(); + Ok(format!("{}.__unwrap({})", class, args[0])) + } } } diff --git a/crates/cli-support/src/wit/mod.rs b/crates/cli-support/src/wit/mod.rs index 38402aafe9d..bfb6c42b814 100644 --- a/crates/cli-support/src/wit/mod.rs +++ b/crates/cli-support/src/wit/mod.rs @@ -934,17 +934,40 @@ impl<'a> Context<'a> { self.aux.structs.push(aux); let wrap_constructor = wasm_bindgen_shared::new_function(struct_.name); - if let Some((import_id, _id)) = self.function_imports.get(&wrap_constructor).cloned() { + self.add_aux_import_to_import_map( + &wrap_constructor, + vec![Descriptor::I32], + Descriptor::Externref, + AuxImport::WrapInExportedClass(struct_.name.to_string()), + )?; + + let unwrap_fn = wasm_bindgen_shared::unwrap_function(struct_.name); + self.add_aux_import_to_import_map( + &unwrap_fn, + vec![Descriptor::Externref], + Descriptor::I32, + AuxImport::UnwrapExportedClass(struct_.name.to_string()), + )?; + + Ok(()) + } + + fn add_aux_import_to_import_map( + &mut self, + fn_name: &String, + arguments: Vec, + ret: Descriptor, + aux_import: AuxImport, + ) -> Result<(), Error> { + if let Some((import_id, _id)) = self.function_imports.get(fn_name).cloned() { let signature = Function { shim_idx: 0, - arguments: vec![Descriptor::I32], - ret: Descriptor::Externref, + arguments, + ret, inner_ret: None, }; let id = self.import_adapter(import_id, signature, AdapterJsImportKind::Normal)?; - self.aux - .import_map - .insert(id, AuxImport::WrapInExportedClass(struct_.name.to_string())); + self.aux.import_map.insert(id, aux_import); } Ok(()) diff --git a/crates/cli-support/src/wit/nonstandard.rs b/crates/cli-support/src/wit/nonstandard.rs index d05761bfda0..fe8142d027e 100644 --- a/crates/cli-support/src/wit/nonstandard.rs +++ b/crates/cli-support/src/wit/nonstandard.rs @@ -336,6 +336,11 @@ pub enum AuxImport { /// The Option may contain the contents of the linked file, so it can be /// embedded. LinkTo(String, Option), + + /// This import is a generated shim which will attempt to unwrap JsValue to an + /// instance of the given exported class. The class name is one that is + /// exported from the Rust/wasm. + UnwrapExportedClass(String), } /// Values that can be imported verbatim to hook up to an import. diff --git a/crates/cli-support/src/wit/section.rs b/crates/cli-support/src/wit/section.rs index 85da8965291..4b22e6f97c6 100644 --- a/crates/cli-support/src/wit/section.rs +++ b/crates/cli-support/src/wit/section.rs @@ -273,6 +273,9 @@ fn check_standard_import(import: &AuxImport) -> Result<(), Error> { format!("wasm-bindgen specific link function for `{}`", path) } AuxImport::Closure { .. } => format!("creating a `Closure` wrapper"), + AuxImport::UnwrapExportedClass(name) => { + format!("unwrapping a pointer from a `{}` js class wrapper", name) + } }; bail!("import of {} requires JS glue", item); } diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index fb3e92cd95f..d2e3fa9962b 100644 --- a/crates/shared/src/lib.rs +++ b/crates/shared/src/lib.rs @@ -165,6 +165,13 @@ pub fn free_function(struct_name: &str) -> String { name } +pub fn unwrap_function(struct_name: &str) -> String { + let mut name = format!("__wbg_"); + name.extend(struct_name.chars().flat_map(|s| s.to_lowercase())); + name.push_str("_unwrap"); + return name; +} + pub fn free_function_export_name(function_name: &str) -> String { function_name.to_string() } diff --git a/src/convert/impls.rs b/src/convert/impls.rs index e8636fe9e66..c65636bbb5f 100644 --- a/src/convert/impls.rs +++ b/src/convert/impls.rs @@ -4,8 +4,16 @@ use core::mem::{self, ManuallyDrop}; use crate::convert::traits::WasmAbi; use crate::convert::{FromWasmAbi, IntoWasmAbi, LongRefFromWasmAbi, RefFromWasmAbi}; use crate::convert::{OptionFromWasmAbi, OptionIntoWasmAbi, ReturnWasmAbi}; +use crate::describe::{self, WasmDescribe}; use crate::{Clamped, JsError, JsValue}; +if_std! { + use std::boxed::Box; + use std::convert::{TryFrom, TryInto}; + use std::vec::Vec; + use crate::convert::JsValueVector; +} + unsafe impl WasmAbi for () {} #[repr(C, u32)] @@ -321,7 +329,7 @@ impl IntoWasmAbi for () { /// - u32/i32/f32/f64 fields at the "leaf fields" of the "field tree" /// - layout equivalent to a completely flattened repr(C) struct, constructed by an in order /// traversal of all the leaf fields in it. -/// +/// /// This means that you can't embed struct A(u32, f64) as struct B(u32, A); because the "completely /// flattened" struct AB(u32, u32, f64) would miss the 4 byte padding that is actually present /// within B and then as a consequence also miss the 4 byte padding within A that repr(C) inserts. @@ -386,3 +394,40 @@ impl IntoWasmAbi for JsError { self.value.into_abi() } } + +if_std! { + impl JsValueVector for Box<[T]> where + T: Into + TryFrom, + >::Error: core::fmt::Debug { + type ToAbi = as IntoWasmAbi>::Abi; + type FromAbi = as FromWasmAbi>::Abi; + + fn describe() { + describe::inform(describe::VECTOR); + JsValue::describe(); + } + + fn into_abi(self) -> Self::ToAbi { + let js_vals: Box::<[JsValue]> = self + .into_vec() + .into_iter() + .map(|x| x.into()) + .collect(); + + IntoWasmAbi::into_abi(js_vals) + } + + fn none() -> Self::ToAbi { + as OptionIntoWasmAbi>::none() + } + + unsafe fn from_abi(js: Self::FromAbi) -> Self { + let js_vals = as FromWasmAbi>::from_abi(js); + + js_vals + .into_iter() + .filter_map(|x| x.try_into().ok()) + .collect() + } + } +} diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 3afe75cc8cb..fed3e360e92 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -1,4 +1,3 @@ -use std::convert::{TryFrom, TryInto}; #[cfg(feature = "std")] use std::prelude::v1::*; @@ -11,13 +10,15 @@ use crate::convert::OptionIntoWasmAbi; use crate::convert::{ FromWasmAbi, IntoWasmAbi, LongRefFromWasmAbi, RefFromWasmAbi, RefMutFromWasmAbi, WasmAbi, }; -use crate::describe; -use crate::describe::WasmDescribe; +use crate::convert::{OptionVectorFromWasmAbi, OptionVectorIntoWasmAbi}; +use crate::convert::{VectorFromWasmAbi, VectorIntoWasmAbi}; +use crate::describe::{self, WasmDescribe, WasmDescribeVector}; use cfg_if::cfg_if; if_std! { - use core::mem; - use crate::convert::OptionFromWasmAbi; + use core::{mem}; + use std::convert::TryFrom; + use crate::convert::{OptionFromWasmAbi, JsValueVector}; } #[repr(C)] @@ -80,21 +81,21 @@ if_std! { macro_rules! vectors { ($($t:ident)*) => ($( if_std! { - impl WasmDescribe for Box<[$t]> { - fn describe() { + impl WasmDescribeVector for $t { + fn describe_vector() { describe::inform(describe::VECTOR); $t::describe(); } } - impl IntoWasmAbi for Box<[$t]> { + impl VectorIntoWasmAbi for $t { type Abi = WasmSlice; #[inline] - fn into_abi(self) -> WasmSlice { - let ptr = self.as_ptr(); - let len = self.len(); - mem::forget(self); + fn vector_into_abi(vector: Box<[$t]>) -> WasmSlice { + let ptr = vector.as_ptr(); + let len = vector.len(); + mem::forget(vector); WasmSlice { ptr: ptr.into_abi(), len: len as u32, @@ -102,25 +103,25 @@ macro_rules! vectors { } } - impl OptionIntoWasmAbi for Box<[$t]> { + impl OptionVectorIntoWasmAbi for $t { #[inline] - fn none() -> WasmSlice { null_slice() } + fn vector_none() -> WasmSlice { null_slice() } } - impl FromWasmAbi for Box<[$t]> { + impl VectorFromWasmAbi for $t { type Abi = WasmSlice; #[inline] - unsafe fn from_abi(js: WasmSlice) -> Self { + unsafe fn vector_from_abi(js: WasmSlice) -> Box<[$t]> { let ptr = <*mut $t>::from_abi(js.ptr); let len = js.len as usize; Vec::from_raw_parts(ptr, len, len).into_boxed_slice() } } - impl OptionFromWasmAbi for Box<[$t]> { + impl OptionVectorFromWasmAbi for $t { #[inline] - fn is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } + fn vector_is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } } } @@ -193,24 +194,6 @@ vectors! { u8 i8 u16 i16 u32 i32 u64 i64 usize isize f32 f64 } -/// Enables blanket implementations of `WasmDescribe`, `IntoWasmAbi`, -/// `FromWasmAbi` and `OptionIntoWasmAbi` functionality on boxed slices of -/// types which can be converted to and from `JsValue` without conflicting -/// implementations of those traits. -/// -/// Implementing these traits directly with blanket implementations would -/// be much more elegant, but unfortunately that's impossible because it -/// conflicts with the implementations for `Box<[T]> where T: JsObject`. -pub trait JsValueVector { - type ToAbi; - type FromAbi; - - fn describe(); - fn into_abi(self) -> Self::ToAbi; - fn none() -> Self::ToAbi; - unsafe fn from_abi(js: Self::FromAbi) -> Self; -} - /* * Generates implementations for traits necessary for passing types to and from * JavaScript on boxed slices of values which can be converted to and from @@ -219,12 +202,13 @@ pub trait JsValueVector { macro_rules! js_value_vectors { ($($t:ident)*) => ($( if_std! { - impl WasmDescribe for Box<[$t]> { - fn describe() { - ::describe(); + impl WasmDescribeVector for $t { + fn describe_vector() { + as JsValueVector>::describe(); } } + // Can't use VectorIntoWasmAbi etc. because $t isn't necessarily Sized impl IntoWasmAbi for Box<[$t]> { type Abi = ::ToAbi; @@ -234,7 +218,7 @@ macro_rules! js_value_vectors { } impl OptionIntoWasmAbi for Box<[$t]> { - fn none() -> Self::Abi { + fn none() -> ::ToAbi { ::none() } } @@ -251,41 +235,6 @@ macro_rules! js_value_vectors { } if_std! { - impl JsValueVector for Box<[T]> where - T: Into + TryFrom, - >::Error: core::fmt::Debug { - type ToAbi = as IntoWasmAbi>::Abi; - type FromAbi = as FromWasmAbi>::Abi; - - fn describe() { - describe::inform(describe::VECTOR); - JsValue::describe(); - } - - fn into_abi(self) -> Self::ToAbi { - let js_vals: Box::<[JsValue]> = self - .into_vec() - .into_iter() - .map(|x| x.into()) - .collect(); - - IntoWasmAbi::into_abi(js_vals) - } - - fn none() -> Self::ToAbi { - as OptionIntoWasmAbi>::none() - } - - unsafe fn from_abi(js: Self::FromAbi) -> Self { - let js_vals = as FromWasmAbi>::from_abi(js); - - js_vals - .into_iter() - .map(|x| x.try_into().expect("Array element of wrong type")) - .collect() - } - } - impl TryFrom for String { type Error = (); @@ -419,14 +368,42 @@ impl LongRefFromWasmAbi for str { if_std! { use crate::JsValue; - impl IntoWasmAbi for Box<[JsValue]> { + impl IntoWasmAbi for Box<[T]> { + type Abi = ::Abi; + + fn into_abi(self) -> Self::Abi { + T::vector_into_abi(self) + } + } + + impl OptionIntoWasmAbi for Box<[T]> { + fn none() -> ::Abi { + T::vector_none() + } + } + + impl FromWasmAbi for Box<[T]> { + type Abi = ::Abi; + + unsafe fn from_abi(js: Self::Abi) -> Self { + T::vector_from_abi(js) + } + } + + impl OptionFromWasmAbi for Box<[T]> { + fn is_none(slice: &::Abi) -> bool { + T::vector_is_none(slice) + } + } + + impl VectorIntoWasmAbi for JsValue { type Abi = WasmSlice; #[inline] - fn into_abi(self) -> WasmSlice { - let ptr = self.as_ptr(); - let len = self.len(); - mem::forget(self); + fn vector_into_abi(vector: Box<[Self]>) -> WasmSlice { + let ptr = vector.as_ptr(); + let len = vector.len(); + mem::forget(vector); WasmSlice { ptr: ptr.into_abi(), len: len as u32, @@ -434,35 +411,35 @@ if_std! { } } - impl OptionIntoWasmAbi for Box<[JsValue]> { + impl OptionVectorIntoWasmAbi for JsValue { #[inline] - fn none() -> WasmSlice { null_slice() } + fn vector_none() -> WasmSlice { null_slice() } } - impl FromWasmAbi for Box<[JsValue]> { + impl VectorFromWasmAbi for JsValue { type Abi = WasmSlice; #[inline] - unsafe fn from_abi(js: WasmSlice) -> Self { + unsafe fn vector_from_abi(js: WasmSlice) -> Box<[Self]> { let ptr = <*mut JsValue>::from_abi(js.ptr); let len = js.len as usize; Vec::from_raw_parts(ptr, len, len).into_boxed_slice() } } - impl OptionFromWasmAbi for Box<[JsValue]> { + impl OptionVectorFromWasmAbi for JsValue { #[inline] - fn is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } + fn vector_is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } } - impl IntoWasmAbi for Box<[T]> where T: JsObject { + impl VectorIntoWasmAbi for T where T: JsObject { type Abi = WasmSlice; #[inline] - fn into_abi(self) -> WasmSlice { - let ptr = self.as_ptr(); - let len = self.len(); - mem::forget(self); + fn vector_into_abi(vector: Box<[T]>) -> WasmSlice { + let ptr = vector.as_ptr(); + let len = vector.len(); + mem::forget(vector); WasmSlice { ptr: ptr.into_abi(), len: len as u32, @@ -470,16 +447,16 @@ if_std! { } } - impl OptionIntoWasmAbi for Box<[T]> where T: JsObject { + impl OptionVectorIntoWasmAbi for T where T: JsObject { #[inline] - fn none() -> WasmSlice { null_slice() } + fn vector_none() -> WasmSlice { null_slice() } } - impl FromWasmAbi for Box<[T]> where T: JsObject { + impl VectorFromWasmAbi for T where T: JsObject { type Abi = WasmSlice; #[inline] - unsafe fn from_abi(js: WasmSlice) -> Self { + unsafe fn vector_from_abi(js: WasmSlice) -> Box<[T]> { let ptr = <*mut JsValue>::from_abi(js.ptr); let len = js.len as usize; let vec: Vec = Vec::from_raw_parts(ptr, len, len).drain(..).map(|js_value| T::unchecked_from_js(js_value)).collect(); @@ -487,8 +464,8 @@ if_std! { } } - impl OptionFromWasmAbi for Box<[T]> where T: JsObject { + impl OptionVectorFromWasmAbi for T where T: JsObject { #[inline] - fn is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } + fn vector_is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } } } diff --git a/src/convert/traits.rs b/src/convert/traits.rs index fe31aab63a4..f8f6ffd2911 100644 --- a/src/convert/traits.rs +++ b/src/convert/traits.rs @@ -158,3 +158,46 @@ impl ReturnWasmAbi for T { self.into_abi() } } + +/// Enables blanket implementations of `WasmDescribe`, `IntoWasmAbi`, +/// `FromWasmAbi` and `OptionIntoWasmAbi` functionality on boxed slices of +/// types which can be converted to and from `JsValue` without conflicting +/// implementations of those traits. +/// +/// Implementing these traits directly with blanket implementations would +/// be much more elegant, but unfortunately that's impossible because it +/// conflicts with the implementations for `Box<[T]> where T: JsObject`. +pub trait JsValueVector { + type ToAbi; + type FromAbi; + + fn describe(); + fn into_abi(self) -> Self::ToAbi; + fn none() -> Self::ToAbi; + unsafe fn from_abi(js: Self::FromAbi) -> Self; +} + +if_std! { + use std::boxed::Box; + use core::marker::Sized; + + pub trait VectorIntoWasmAbi: WasmDescribeVector + Sized { + type Abi: WasmAbi; + + fn vector_into_abi(vector: Box<[Self]>) -> Self::Abi; + } + + pub trait OptionVectorIntoWasmAbi: VectorIntoWasmAbi { + fn vector_none() -> Self::Abi; + } + + pub trait VectorFromWasmAbi: WasmDescribeVector + Sized { + type Abi: WasmAbi; + + unsafe fn vector_from_abi(js: Self::Abi) -> Box<[Self]>; + } + + pub trait OptionVectorFromWasmAbi: VectorFromWasmAbi { + fn vector_is_none(abi: &Self::Abi) -> bool; + } +} diff --git a/src/describe.rs b/src/describe.rs index 5a45378c352..565be08e15d 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -57,6 +57,10 @@ pub trait WasmDescribe { fn describe(); } +pub trait WasmDescribeVector { + fn describe_vector(); +} + macro_rules! simple { ($($t:ident => $d:ident)*) => ($( impl WasmDescribe for $t { @@ -145,15 +149,21 @@ if_std! { } } - impl WasmDescribe for Box<[JsValue]> { - fn describe() { + impl WasmDescribeVector for JsValue { + fn describe_vector() { inform(VECTOR); JsValue::describe(); } } - impl WasmDescribe for Box<[T]> where T: JsObject { + impl WasmDescribe for Box<[T]> { fn describe() { + T::describe_vector(); + } + } + + impl WasmDescribeVector for T { + fn describe_vector() { inform(VECTOR); T::describe(); } From 9752b813947f247c6c0e911987ba1560cced4354 Mon Sep 17 00:00:00 2001 From: Billy Bradley Date: Tue, 7 Dec 2021 13:59:44 +0000 Subject: [PATCH 03/24] Remove unneeded require --- crates/cli-support/src/js/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 6eea1178b80..86dcc058119 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -3238,7 +3238,6 @@ impl<'a> Context<'a> { assert!(!variadic); assert_eq!(args.len(), 1); self.require_class_unwrap(class); - self.expose_take_object(); Ok(format!("{}.__unwrap({})", class, args[0])) } } From 734eef4c62d9f9d75d82320304b358ebe237f7df Mon Sep 17 00:00:00 2001 From: Billy Bradley Date: Tue, 7 Dec 2021 14:14:12 +0000 Subject: [PATCH 04/24] Move uses out of if_std --- src/convert/slices.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/convert/slices.rs b/src/convert/slices.rs index fed3e360e92..53f458e93e1 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -16,7 +16,7 @@ use crate::describe::{self, WasmDescribe, WasmDescribeVector}; use cfg_if::cfg_if; if_std! { - use core::{mem}; + use core::mem; use std::convert::TryFrom; use crate::convert::{OptionFromWasmAbi, JsValueVector}; } From 905c1c3f363a306331961a2717e4d22598f31b58 Mon Sep 17 00:00:00 2001 From: Billy Bradley Date: Tue, 7 Dec 2021 14:25:03 +0000 Subject: [PATCH 05/24] Add documentation --- src/convert/traits.rs | 8 ++++++++ src/describe.rs | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/convert/traits.rs b/src/convert/traits.rs index f8f6ffd2911..7e8077abca7 100644 --- a/src/convert/traits.rs +++ b/src/convert/traits.rs @@ -181,22 +181,30 @@ if_std! { use std::boxed::Box; use core::marker::Sized; + /// Trait for element types to implement IntoWasmAbi for vectors of + /// themselves. pub trait VectorIntoWasmAbi: WasmDescribeVector + Sized { type Abi: WasmAbi; fn vector_into_abi(vector: Box<[Self]>) -> Self::Abi; } + /// Trait for element types to implement OptionIntoWasmAbi for vectors + /// of themselves. pub trait OptionVectorIntoWasmAbi: VectorIntoWasmAbi { fn vector_none() -> Self::Abi; } + /// Trait for element types to implement FromWasmAbi for vectors of + /// themselves. pub trait VectorFromWasmAbi: WasmDescribeVector + Sized { type Abi: WasmAbi; unsafe fn vector_from_abi(js: Self::Abi) -> Box<[Self]>; } + /// Trait for element types to implement OptionFromWasmAbi for vectors + /// of themselves. pub trait OptionVectorFromWasmAbi: VectorFromWasmAbi { fn vector_is_none(abi: &Self::Abi) -> bool; } diff --git a/src/describe.rs b/src/describe.rs index 565be08e15d..c7e53b8398f 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -57,6 +57,8 @@ pub trait WasmDescribe { fn describe(); } +/// Trait for element types to implement IntoWasmAbi for vectors of +/// themselves. pub trait WasmDescribeVector { fn describe_vector(); } From c8a327fa163200b4df3d9db707103e3c3eae9ac9 Mon Sep 17 00:00:00 2001 From: Billy Bradley Date: Tue, 7 Dec 2021 14:36:21 +0000 Subject: [PATCH 06/24] Move incorrect use statements --- crates/backend/src/codegen.rs | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 1ab90db0abb..45e803b32b8 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -332,37 +332,48 @@ impl ToTokens for ast::Struct { } impl wasm_bindgen::describe::WasmDescribeVector for #name { - use wasm_bindgen::__rt::std::boxed::Box; - fn describe_vector() { + use wasm_bindgen::__rt::std::boxed::Box; as wasm_bindgen::convert::JsValueVector>::describe(); } } impl wasm_bindgen::convert::VectorIntoWasmAbi for #name { - use wasm_bindgen::__rt::std::boxed::Box; + type Abi = < + wasm_bindgen::__rt::std::boxed::Box<[#name]> + as wasm_bindgen::convert::JsValueVector + >::ToAbi; - type Abi = as wasm_bindgen::convert::JsValueVector>::ToAbi; + fn vector_into_abi( + vector: wasm_bindgen::__rt::std::boxed::Box<[#name]> + ) -> Self::Abi { + use wasm_bindgen::__rt::std::boxed::Box; - fn vector_into_abi(vector: Box<[#name]>) -> Self::Abi { as wasm_bindgen::convert::JsValueVector>::into_abi(vector) } } impl wasm_bindgen::convert::OptionVectorIntoWasmAbi for #name { - use wasm_bindgen::__rt::std::boxed::Box; - - fn vector_none() -> as wasm_bindgen::convert::JsValueVector>::ToAbi { + fn vector_none() -> < + wasm_bindgen::__rt::std::boxed::Box<[#name]> + as wasm_bindgen::convert::JsValueVector + >::ToAbi { + use wasm_bindgen::__rt::std::boxed::Box; as wasm_bindgen::convert::JsValueVector>::none() } } impl wasm_bindgen::convert::VectorFromWasmAbi for #name { - use wasm_bindgen::__rt::std::boxed::Box; + type Abi = < + wasm_bindgen::__rt::std::boxed::Box<[#name]> + as wasm_bindgen::convert::JsValueVector + >::FromAbi; - type Abi = as wasm_bindgen::convert::JsValueVector>::FromAbi; + unsafe fn vector_from_abi( + js: Self::Abi + ) -> wasm_bindgen::__rt::std::boxed::Box<[#name]> { + use wasm_bindgen::__rt::std::boxed::Box; - unsafe fn vector_from_abi(js: Self::Abi) -> Box<[#name]> { as wasm_bindgen::convert::JsValueVector>::from_abi(js) } } From cd89028a453219fdbe194c931590e2f7207d7f70 Mon Sep 17 00:00:00 2001 From: Billy Bradley Date: Tue, 21 Dec 2021 19:19:22 +0000 Subject: [PATCH 07/24] Fix mistake in comment --- src/describe.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/describe.rs b/src/describe.rs index c7e53b8398f..cb43cfe73d6 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -57,7 +57,7 @@ pub trait WasmDescribe { fn describe(); } -/// Trait for element types to implement IntoWasmAbi for vectors of +/// Trait for element types to implement WasmDescribe for vectors of /// themselves. pub trait WasmDescribeVector { fn describe_vector(); From 55b7c3eb89859ba51388f2edb0d59d02ffcd928a Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Wed, 9 Aug 2023 19:05:38 +1000 Subject: [PATCH 08/24] Throw on invalid array elements instead of silently removing them I put some work into making sure that you can tell what function the error message is coming from. You still have to dig into the call stack of the error message to see it, but hopefully that's not too big an ask? --- src/convert/impls.rs | 21 ++++++++++++++++----- tests/wasm/main.rs | 1 + tests/wasm/struct_vecs.js | 17 +++++++++++++++++ tests/wasm/struct_vecs.rs | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 tests/wasm/struct_vecs.js create mode 100644 tests/wasm/struct_vecs.rs diff --git a/src/convert/impls.rs b/src/convert/impls.rs index c65636bbb5f..0bf4528c25e 100644 --- a/src/convert/impls.rs +++ b/src/convert/impls.rs @@ -5,7 +5,7 @@ use crate::convert::traits::WasmAbi; use crate::convert::{FromWasmAbi, IntoWasmAbi, LongRefFromWasmAbi, RefFromWasmAbi}; use crate::convert::{OptionFromWasmAbi, OptionIntoWasmAbi, ReturnWasmAbi}; use crate::describe::{self, WasmDescribe}; -use crate::{Clamped, JsError, JsValue}; +use crate::{Clamped, JsError, JsValue, UnwrapThrowExt}; if_std! { use std::boxed::Box; @@ -424,10 +424,21 @@ if_std! { unsafe fn from_abi(js: Self::FromAbi) -> Self { let js_vals = as FromWasmAbi>::from_abi(js); - js_vals - .into_iter() - .filter_map(|x| x.try_into().ok()) - .collect() + let mut result = Vec::with_capacity(js_vals.len()); + for value in js_vals { + // We push elements one-by-one instead of using `collect` in order to improve + // error messages. When using `collect`, this `expect_throw` is buried in a + // giant chain of internal iterator functions, which results in the actual + // function that takes this `Vec` falling off the end of the call stack. + // So instead, make sure to call it directly within this function. + // + // This is only a problem in debug mode. Since this is the browser's error stack + // we're talking about, it can only see functions that actually make it to the + // final wasm binary (i.e., not inlined functions). All of those internal + // iterator functions get inlined in release mode, and so they don't show up. + result.push(value.try_into().expect_throw("array contains a value of the wrong type")); + } + result.into_boxed_slice() } } } diff --git a/tests/wasm/main.rs b/tests/wasm/main.rs index 508a9ea1f78..e2064f60f14 100644 --- a/tests/wasm/main.rs +++ b/tests/wasm/main.rs @@ -46,6 +46,7 @@ pub mod result_jserror; pub mod rethrow; pub mod simple; pub mod slice; +pub mod struct_vecs; pub mod structural; pub mod truthy_falsy; pub mod usize; diff --git a/tests/wasm/struct_vecs.js b/tests/wasm/struct_vecs.js new file mode 100644 index 00000000000..fe376632386 --- /dev/null +++ b/tests/wasm/struct_vecs.js @@ -0,0 +1,17 @@ +const wasm = require('wasm-bindgen-test.js'); +const assert = require('assert'); + +exports.pass_struct_vec = () => { + const el1 = new wasm.ArrayElement(); + const el2 = new wasm.ArrayElement(); + wasm.consume_struct_vec([el1, el2]); +}; + +exports.pass_invalid_struct_vec = () => { + try { + wasm.consume_struct_vec(['not a struct']); + } catch (e) { + assert.match(e.message, /array contains a value of the wrong type/) + assert.match(e.stack, /consume_struct_vec/) + } +}; diff --git a/tests/wasm/struct_vecs.rs b/tests/wasm/struct_vecs.rs new file mode 100644 index 00000000000..346d8ff149b --- /dev/null +++ b/tests/wasm/struct_vecs.rs @@ -0,0 +1,32 @@ +use wasm_bindgen::prelude::*; +use wasm_bindgen_test::*; + +#[wasm_bindgen(module = "tests/wasm/struct_vecs.js")] +extern "C" { + fn pass_struct_vec(); + fn pass_invalid_struct_vec(); +} + +#[wasm_bindgen] +pub struct ArrayElement; + +#[wasm_bindgen] +impl ArrayElement { + #[wasm_bindgen(constructor)] + pub fn new() -> ArrayElement { + ArrayElement + } +} + +#[wasm_bindgen] +pub fn consume_struct_vec(_: Vec) {} + +#[wasm_bindgen_test] +fn test_valid() { + pass_struct_vec(); +} + +#[wasm_bindgen_test] +fn test_invalid() { + pass_invalid_struct_vec(); +} From dba751ddc972a1f2e90b3c802df787c1a1cf0116 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 15:05:54 +1000 Subject: [PATCH 09/24] Get rid of `JsValueVector` The main reason for this, which I didn't mention before, is that I found it really confusing when I was originally reviewing this PR what the difference was between `JsValueVector` and `Vector{From,Into}WasmAbi`, since it really looks like another conversion trait at first glance. --- crates/backend/src/codegen.rs | 30 +++++++-------- src/convert/impls.rs | 71 +++++++++++++---------------------- src/convert/slices.rs | 17 +++++---- src/convert/traits.rs | 18 --------- 4 files changed, 49 insertions(+), 87 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 45e803b32b8..cdbd32fdb76 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -334,47 +334,43 @@ impl ToTokens for ast::Struct { impl wasm_bindgen::describe::WasmDescribeVector for #name { fn describe_vector() { use wasm_bindgen::__rt::std::boxed::Box; - as wasm_bindgen::convert::JsValueVector>::describe(); + as #wasm_bindgen::describe::WasmDescribe>::describe(); } } impl wasm_bindgen::convert::VectorIntoWasmAbi for #name { type Abi = < - wasm_bindgen::__rt::std::boxed::Box<[#name]> - as wasm_bindgen::convert::JsValueVector - >::ToAbi; + #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> + as #wasm_bindgen::convert::IntoWasmAbi + >::Abi; fn vector_into_abi( vector: wasm_bindgen::__rt::std::boxed::Box<[#name]> ) -> Self::Abi { - use wasm_bindgen::__rt::std::boxed::Box; - - as wasm_bindgen::convert::JsValueVector>::into_abi(vector) + #wasm_bindgen::convert::js_value_vector_into_abi(vector) } } impl wasm_bindgen::convert::OptionVectorIntoWasmAbi for #name { fn vector_none() -> < - wasm_bindgen::__rt::std::boxed::Box<[#name]> - as wasm_bindgen::convert::JsValueVector - >::ToAbi { + #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> + as #wasm_bindgen::convert::IntoWasmAbi + >::Abi { use wasm_bindgen::__rt::std::boxed::Box; - as wasm_bindgen::convert::JsValueVector>::none() + as #wasm_bindgen::convert::OptionIntoWasmAbi>::none() } } impl wasm_bindgen::convert::VectorFromWasmAbi for #name { type Abi = < - wasm_bindgen::__rt::std::boxed::Box<[#name]> - as wasm_bindgen::convert::JsValueVector - >::FromAbi; + #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> + as #wasm_bindgen::convert::FromWasmAbi + >::Abi; unsafe fn vector_from_abi( js: Self::Abi ) -> wasm_bindgen::__rt::std::boxed::Box<[#name]> { - use wasm_bindgen::__rt::std::boxed::Box; - - as wasm_bindgen::convert::JsValueVector>::from_abi(js) + #wasm_bindgen::convert::js_value_vector_from_abi(js) } } }) diff --git a/src/convert/impls.rs b/src/convert/impls.rs index 0bf4528c25e..82f7fa820aa 100644 --- a/src/convert/impls.rs +++ b/src/convert/impls.rs @@ -4,14 +4,13 @@ use core::mem::{self, ManuallyDrop}; use crate::convert::traits::WasmAbi; use crate::convert::{FromWasmAbi, IntoWasmAbi, LongRefFromWasmAbi, RefFromWasmAbi}; use crate::convert::{OptionFromWasmAbi, OptionIntoWasmAbi, ReturnWasmAbi}; -use crate::describe::{self, WasmDescribe}; use crate::{Clamped, JsError, JsValue, UnwrapThrowExt}; if_std! { use std::boxed::Box; use std::convert::{TryFrom, TryInto}; use std::vec::Vec; - use crate::convert::JsValueVector; + use std::fmt::Debug; } unsafe impl WasmAbi for () {} @@ -396,49 +395,33 @@ impl IntoWasmAbi for JsError { } if_std! { - impl JsValueVector for Box<[T]> where - T: Into + TryFrom, - >::Error: core::fmt::Debug { - type ToAbi = as IntoWasmAbi>::Abi; - type FromAbi = as FromWasmAbi>::Abi; - - fn describe() { - describe::inform(describe::VECTOR); - JsValue::describe(); - } - - fn into_abi(self) -> Self::ToAbi { - let js_vals: Box::<[JsValue]> = self - .into_vec() - .into_iter() - .map(|x| x.into()) - .collect(); - - IntoWasmAbi::into_abi(js_vals) - } - - fn none() -> Self::ToAbi { - as OptionIntoWasmAbi>::none() - } + pub fn js_value_vector_into_abi>(vector: Box<[T]>) -> as IntoWasmAbi>::Abi { + let js_vals: Box::<[JsValue]> = vector + .into_vec() + .into_iter() + .map(|x| x.into()) + .collect(); + + js_vals.into_abi() + } - unsafe fn from_abi(js: Self::FromAbi) -> Self { - let js_vals = as FromWasmAbi>::from_abi(js); - - let mut result = Vec::with_capacity(js_vals.len()); - for value in js_vals { - // We push elements one-by-one instead of using `collect` in order to improve - // error messages. When using `collect`, this `expect_throw` is buried in a - // giant chain of internal iterator functions, which results in the actual - // function that takes this `Vec` falling off the end of the call stack. - // So instead, make sure to call it directly within this function. - // - // This is only a problem in debug mode. Since this is the browser's error stack - // we're talking about, it can only see functions that actually make it to the - // final wasm binary (i.e., not inlined functions). All of those internal - // iterator functions get inlined in release mode, and so they don't show up. - result.push(value.try_into().expect_throw("array contains a value of the wrong type")); - } - result.into_boxed_slice() + pub unsafe fn js_value_vector_from_abi>(js: as FromWasmAbi>::Abi) -> Box<[T]> where T::Error: Debug { + let js_vals = as FromWasmAbi>::from_abi(js); + + let mut result = Vec::with_capacity(js_vals.len()); + for value in js_vals { + // We push elements one-by-one instead of using `collect` in order to improve + // error messages. When using `collect`, this `expect_throw` is buried in a + // giant chain of internal iterator functions, which results in the actual + // function that takes this `Vec` falling off the end of the call stack. + // So instead, make sure to call it directly within this function. + // + // This is only a problem in debug mode. Since this is the browser's error stack + // we're talking about, it can only see functions that actually make it to the + // final wasm binary (i.e., not inlined functions). All of those internal + // iterator functions get inlined in release mode, and so they don't show up. + result.push(value.try_into().expect_throw("array contains a value of the wrong type")); } + result.into_boxed_slice() } } diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 53f458e93e1..fdea7539843 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -18,7 +18,8 @@ use cfg_if::cfg_if; if_std! { use core::mem; use std::convert::TryFrom; - use crate::convert::{OptionFromWasmAbi, JsValueVector}; + use crate::convert::OptionFromWasmAbi; + use crate::convert::{js_value_vector_from_abi, js_value_vector_into_abi}; } #[repr(C)] @@ -204,30 +205,30 @@ macro_rules! js_value_vectors { if_std! { impl WasmDescribeVector for $t { fn describe_vector() { - as JsValueVector>::describe(); + >::describe(); } } // Can't use VectorIntoWasmAbi etc. because $t isn't necessarily Sized impl IntoWasmAbi for Box<[$t]> { - type Abi = ::ToAbi; + type Abi = as IntoWasmAbi>::Abi; fn into_abi(self) -> Self::Abi { - ::into_abi(self) + js_value_vector_into_abi(self) } } impl OptionIntoWasmAbi for Box<[$t]> { - fn none() -> ::ToAbi { - ::none() + fn none() -> as IntoWasmAbi>::Abi { + as OptionIntoWasmAbi>::none() } } impl FromWasmAbi for Box<[$t]> { - type Abi = ::FromAbi; + type Abi = as FromWasmAbi>::Abi; unsafe fn from_abi(js: Self::Abi) -> Self { - ::from_abi(js) + js_value_vector_from_abi(js) } } } diff --git a/src/convert/traits.rs b/src/convert/traits.rs index 7e8077abca7..e84482a7e5b 100644 --- a/src/convert/traits.rs +++ b/src/convert/traits.rs @@ -159,24 +159,6 @@ impl ReturnWasmAbi for T { } } -/// Enables blanket implementations of `WasmDescribe`, `IntoWasmAbi`, -/// `FromWasmAbi` and `OptionIntoWasmAbi` functionality on boxed slices of -/// types which can be converted to and from `JsValue` without conflicting -/// implementations of those traits. -/// -/// Implementing these traits directly with blanket implementations would -/// be much more elegant, but unfortunately that's impossible because it -/// conflicts with the implementations for `Box<[T]> where T: JsObject`. -pub trait JsValueVector { - type ToAbi; - type FromAbi; - - fn describe(); - fn into_abi(self) -> Self::ToAbi; - fn none() -> Self::ToAbi; - unsafe fn from_abi(js: Self::FromAbi) -> Self; -} - if_std! { use std::boxed::Box; use core::marker::Sized; From 84433207c64ae310dd4d6a1739b63f1c1f56c28c Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 15:09:30 +1000 Subject: [PATCH 10/24] Respect the configured `wasm_bindgen` crate path --- crates/backend/src/codegen.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index cdbd32fdb76..c1a8f994496 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -296,12 +296,12 @@ impl ToTokens for ast::Struct { } #[allow(clippy::all)] - impl wasm_bindgen::__rt::core::convert::TryFrom for #name { + impl #wasm_bindgen::__rt::core::convert::TryFrom<#wasm_bindgen::JsValue> for #name { type Error = (); - fn try_from(value: wasm_bindgen::JsValue) - -> wasm_bindgen::__rt::std::result::Result { - let js_ptr = wasm_bindgen::convert::IntoWasmAbi::into_abi(value); + fn try_from(value: #wasm_bindgen::JsValue) + -> #wasm_bindgen::__rt::std::result::Result { + let js_ptr = #wasm_bindgen::convert::IntoWasmAbi::into_abi(value); #[link(wasm_import_module = "__wbindgen_placeholder__")] #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))] @@ -320,48 +320,48 @@ impl ToTokens for ast::Struct { } if(ptr == 0) { - wasm_bindgen::__rt::std::result::Result::Err(()) + #wasm_bindgen::__rt::std::result::Result::Err(()) } else { unsafe { - wasm_bindgen::__rt::std::result::Result::Ok( - ::from_abi(ptr) + #wasm_bindgen::__rt::std::result::Result::Ok( + ::from_abi(ptr) ) } } } } - impl wasm_bindgen::describe::WasmDescribeVector for #name { + impl #wasm_bindgen::describe::WasmDescribeVector for #name { fn describe_vector() { - use wasm_bindgen::__rt::std::boxed::Box; + use #wasm_bindgen::__rt::std::boxed::Box; as #wasm_bindgen::describe::WasmDescribe>::describe(); } } - impl wasm_bindgen::convert::VectorIntoWasmAbi for #name { + impl #wasm_bindgen::convert::VectorIntoWasmAbi for #name { type Abi = < #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> as #wasm_bindgen::convert::IntoWasmAbi >::Abi; fn vector_into_abi( - vector: wasm_bindgen::__rt::std::boxed::Box<[#name]> + vector: #wasm_bindgen::__rt::std::boxed::Box<[#name]> ) -> Self::Abi { #wasm_bindgen::convert::js_value_vector_into_abi(vector) } } - impl wasm_bindgen::convert::OptionVectorIntoWasmAbi for #name { + impl #wasm_bindgen::convert::OptionVectorIntoWasmAbi for #name { fn vector_none() -> < #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> as #wasm_bindgen::convert::IntoWasmAbi >::Abi { - use wasm_bindgen::__rt::std::boxed::Box; + use #wasm_bindgen::__rt::std::boxed::Box; as #wasm_bindgen::convert::OptionIntoWasmAbi>::none() } } - impl wasm_bindgen::convert::VectorFromWasmAbi for #name { + impl #wasm_bindgen::convert::VectorFromWasmAbi for #name { type Abi = < #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> as #wasm_bindgen::convert::FromWasmAbi @@ -369,7 +369,7 @@ impl ToTokens for ast::Struct { unsafe fn vector_from_abi( js: Self::Abi - ) -> wasm_bindgen::__rt::std::boxed::Box<[#name]> { + ) -> #wasm_bindgen::__rt::std::boxed::Box<[#name]> { #wasm_bindgen::convert::js_value_vector_from_abi(js) } } From 0fc2ff9da83b00f39e76a98e0934bb2d5d8a1f78 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 15:12:09 +1000 Subject: [PATCH 11/24] Change the error type for String and rust structs' TryFrom impls to JsValue --- crates/backend/src/codegen.rs | 16 +++++++--------- src/convert/slices.rs | 4 ++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index c1a8f994496..3a8bb3f8119 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -297,11 +297,11 @@ impl ToTokens for ast::Struct { #[allow(clippy::all)] impl #wasm_bindgen::__rt::core::convert::TryFrom<#wasm_bindgen::JsValue> for #name { - type Error = (); + type Error = #wasm_bindgen::JsValue; fn try_from(value: #wasm_bindgen::JsValue) -> #wasm_bindgen::__rt::std::result::Result { - let js_ptr = #wasm_bindgen::convert::IntoWasmAbi::into_abi(value); + let idx = #wasm_bindgen::convert::IntoWasmAbi::into_abi(&value); #[link(wasm_import_module = "__wbindgen_placeholder__")] #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))] @@ -314,14 +314,12 @@ impl ToTokens for ast::Struct { panic!("cannot convert from JsValue outside of the wasm target") } - let ptr; - unsafe { - ptr = #unwrap_fn(js_ptr); - } - - if(ptr == 0) { - #wasm_bindgen::__rt::std::result::Result::Err(()) + let ptr = unsafe { #unwrap_fn(idx) }; + if ptr == 0 { + #wasm_bindgen::__rt::std::result::Result::Err(value) } else { + // Don't run `JsValue`'s destructor, `unwrap_fn` already did that for us. + #wasm_bindgen::__rt::std::mem::forget(value); unsafe { #wasm_bindgen::__rt::std::result::Result::Ok( ::from_abi(ptr) diff --git a/src/convert/slices.rs b/src/convert/slices.rs index fdea7539843..63650a76959 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -237,12 +237,12 @@ macro_rules! js_value_vectors { if_std! { impl TryFrom for String { - type Error = (); + type Error = JsValue; fn try_from(value: JsValue) -> Result { match value.as_string() { Some(s) => Ok(s), - None => Err(()), + None => Err(value), } } } From 7254342959600721434ad4675c447ddf1a5d2eb1 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 17:28:48 +1000 Subject: [PATCH 12/24] test string vecs too --- tests/wasm/main.rs | 1 + tests/wasm/string_vecs.js | 15 +++++++++++++++ tests/wasm/string_vecs.rs | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 tests/wasm/string_vecs.js create mode 100644 tests/wasm/string_vecs.rs diff --git a/tests/wasm/main.rs b/tests/wasm/main.rs index e2064f60f14..eebde19677e 100644 --- a/tests/wasm/main.rs +++ b/tests/wasm/main.rs @@ -46,6 +46,7 @@ pub mod result_jserror; pub mod rethrow; pub mod simple; pub mod slice; +pub mod string_vecs; pub mod struct_vecs; pub mod structural; pub mod truthy_falsy; diff --git a/tests/wasm/string_vecs.js b/tests/wasm/string_vecs.js new file mode 100644 index 00000000000..1b708917e51 --- /dev/null +++ b/tests/wasm/string_vecs.js @@ -0,0 +1,15 @@ +const wasm = require('wasm-bindgen-test.js'); +const assert = require('assert'); + +exports.pass_string_vec = () => { + wasm.consume_string_vec(["hello", "world"]); +}; + +exports.pass_invalid_string_vec = () => { + try { + wasm.consume_string_vec([42]); + } catch (e) { + assert.match(e.message, /array contains a value of the wrong type/) + assert.match(e.stack, /consume_string_vec/) + } +}; diff --git a/tests/wasm/string_vecs.rs b/tests/wasm/string_vecs.rs new file mode 100644 index 00000000000..dbf63e0da1b --- /dev/null +++ b/tests/wasm/string_vecs.rs @@ -0,0 +1,21 @@ +use wasm_bindgen::prelude::*; +use wasm_bindgen_test::*; + +#[wasm_bindgen(module = "tests/wasm/string_vecs.js")] +extern "C" { + fn pass_string_vec(); + fn pass_invalid_string_vec(); +} + +#[wasm_bindgen] +pub fn consume_string_vec(_: Vec) {} + +#[wasm_bindgen_test] +fn test_valid() { + pass_string_vec(); +} + +#[wasm_bindgen_test] +fn test_invalid() { + pass_invalid_string_vec(); +} From 263f6358bb231c5ef538f2b2e42fb7dbc6affda7 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 17:46:16 +1000 Subject: [PATCH 13/24] Refactor `String` impls I moved the `TryFrom` impl out of convert/slices.rs, it doesn't really belong there, and also got rid of the `js_value_vectors!` macro in favour of just implementing it for `String` directly; there's not much point in a macro you only use for one type. --- src/convert/slices.rs | 67 +++++++++++++------------------------------ src/lib.rs | 11 +++++++ 2 files changed, 31 insertions(+), 47 deletions(-) diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 63650a76959..66ee984a96c 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -195,63 +195,36 @@ vectors! { u8 i8 u16 i16 u32 i32 u64 i64 usize isize f32 f64 } -/* - * Generates implementations for traits necessary for passing types to and from - * JavaScript on boxed slices of values which can be converted to and from - * `JsValue`. - */ -macro_rules! js_value_vectors { - ($($t:ident)*) => ($( - if_std! { - impl WasmDescribeVector for $t { - fn describe_vector() { - >::describe(); - } - } - - // Can't use VectorIntoWasmAbi etc. because $t isn't necessarily Sized - impl IntoWasmAbi for Box<[$t]> { - type Abi = as IntoWasmAbi>::Abi; - - fn into_abi(self) -> Self::Abi { - js_value_vector_into_abi(self) - } - } +if_std! { + impl WasmDescribeVector for String { + fn describe_vector() { + >::describe(); + } + } - impl OptionIntoWasmAbi for Box<[$t]> { - fn none() -> as IntoWasmAbi>::Abi { - as OptionIntoWasmAbi>::none() - } - } + impl VectorIntoWasmAbi for String { + type Abi = as IntoWasmAbi>::Abi; - impl FromWasmAbi for Box<[$t]> { - type Abi = as FromWasmAbi>::Abi; + fn vector_into_abi(vector: Box<[Self]>) -> Self::Abi { + js_value_vector_into_abi(vector) + } + } - unsafe fn from_abi(js: Self::Abi) -> Self { - js_value_vector_from_abi(js) - } - } + impl OptionVectorIntoWasmAbi for String { + fn vector_none() -> as IntoWasmAbi>::Abi { + as OptionIntoWasmAbi>::none() } - )*) -} + } -if_std! { - impl TryFrom for String { - type Error = JsValue; + impl VectorFromWasmAbi for String { + type Abi = as FromWasmAbi>::Abi; - fn try_from(value: JsValue) -> Result { - match value.as_string() { - Some(s) => Ok(s), - None => Err(value), - } + unsafe fn vector_from_abi(js: Self::Abi) -> Box<[Self]> { + js_value_vector_from_abi(js) } } } -js_value_vectors! { - String -} - cfg_if! { if #[cfg(feature = "enable-interning")] { #[inline] diff --git a/src/lib.rs b/src/lib.rs index 1df9cd92b0d..44844f63b8b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -805,6 +805,17 @@ if_std! { JsValue::from_str(&s) } } + + impl TryFrom for String { + type Error = JsValue; + + fn try_from(value: JsValue) -> Result { + match value.as_string() { + Some(s) => Ok(s), + None => Err(value), + } + } + } } impl From for JsValue { From 761dc4acb05dcb95bfeb60876c1070e89c8f8974 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 18:38:53 +1000 Subject: [PATCH 14/24] Don't require manual `OptionVector{From,Into}WasmAbi` impls I noticed that strings and rust structs weren't implementing `OptionVectorFromWasmAbi`, so I tried to make a failing test and... it worked. That threw me for a loop for a bit until I realised that it was because I'd used `Vec`, and `Vec`'s impls of `Option{From,Into}WasmAbi` didn't actually rely on `Box<[T]>` implementing the traits: they just required that it implemented `{From,Into}WasmAbi` with an ABI of `WasmSlice`, since from there the element type doesn't matter. So then I thought 'well, why not do that for `Box<[T]>` too? so that's how this commit's pile of new tests came to be. --- crates/backend/src/codegen.rs | 10 ------- src/convert/slices.rs | 50 +++++------------------------------ src/convert/traits.rs | 12 --------- tests/wasm/slice.js | 21 +++++++++++++++ tests/wasm/slice.rs | 32 ++++++++++++++-------- tests/wasm/string_vecs.js | 10 ++++++- tests/wasm/string_vecs.rs | 10 ++++++- tests/wasm/struct_vecs.js | 8 +++++- tests/wasm/struct_vecs.rs | 10 ++++++- 9 files changed, 82 insertions(+), 81 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 3a8bb3f8119..734fff7587f 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -349,16 +349,6 @@ impl ToTokens for ast::Struct { } } - impl #wasm_bindgen::convert::OptionVectorIntoWasmAbi for #name { - fn vector_none() -> < - #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> - as #wasm_bindgen::convert::IntoWasmAbi - >::Abi { - use #wasm_bindgen::__rt::std::boxed::Box; - as #wasm_bindgen::convert::OptionIntoWasmAbi>::none() - } - } - impl #wasm_bindgen::convert::VectorFromWasmAbi for #name { type Abi = < #wasm_bindgen::__rt::std::boxed::Box<[#wasm_bindgen::JsValue]> diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 66ee984a96c..51c619d3e9c 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -10,14 +10,12 @@ use crate::convert::OptionIntoWasmAbi; use crate::convert::{ FromWasmAbi, IntoWasmAbi, LongRefFromWasmAbi, RefFromWasmAbi, RefMutFromWasmAbi, WasmAbi, }; -use crate::convert::{OptionVectorFromWasmAbi, OptionVectorIntoWasmAbi}; use crate::convert::{VectorFromWasmAbi, VectorIntoWasmAbi}; use crate::describe::{self, WasmDescribe, WasmDescribeVector}; use cfg_if::cfg_if; if_std! { use core::mem; - use std::convert::TryFrom; use crate::convert::OptionFromWasmAbi; use crate::convert::{js_value_vector_from_abi, js_value_vector_into_abi}; } @@ -104,11 +102,6 @@ macro_rules! vectors { } } - impl OptionVectorIntoWasmAbi for $t { - #[inline] - fn vector_none() -> WasmSlice { null_slice() } - } - impl VectorFromWasmAbi for $t { type Abi = WasmSlice; @@ -119,11 +112,6 @@ macro_rules! vectors { Vec::from_raw_parts(ptr, len, len).into_boxed_slice() } } - - impl OptionVectorFromWasmAbi for $t { - #[inline] - fn vector_is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } - } } impl<'a> IntoWasmAbi for &'a [$t] { @@ -210,12 +198,6 @@ if_std! { } } - impl OptionVectorIntoWasmAbi for String { - fn vector_none() -> as IntoWasmAbi>::Abi { - as OptionIntoWasmAbi>::none() - } - } - impl VectorFromWasmAbi for String { type Abi = as FromWasmAbi>::Abi; @@ -350,9 +332,9 @@ if_std! { } } - impl OptionIntoWasmAbi for Box<[T]> { - fn none() -> ::Abi { - T::vector_none() + impl OptionIntoWasmAbi for Box<[T]> where Self: IntoWasmAbi { + fn none() -> WasmSlice { + null_slice() } } @@ -364,9 +346,9 @@ if_std! { } } - impl OptionFromWasmAbi for Box<[T]> { - fn is_none(slice: &::Abi) -> bool { - T::vector_is_none(slice) + impl OptionFromWasmAbi for Box<[T]> where Self: FromWasmAbi { + fn is_none(slice: &WasmSlice) -> bool { + slice.ptr == 0 } } @@ -385,11 +367,6 @@ if_std! { } } - impl OptionVectorIntoWasmAbi for JsValue { - #[inline] - fn vector_none() -> WasmSlice { null_slice() } - } - impl VectorFromWasmAbi for JsValue { type Abi = WasmSlice; @@ -401,11 +378,6 @@ if_std! { } } - impl OptionVectorFromWasmAbi for JsValue { - #[inline] - fn vector_is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } - } - impl VectorIntoWasmAbi for T where T: JsObject { type Abi = WasmSlice; @@ -421,11 +393,6 @@ if_std! { } } - impl OptionVectorIntoWasmAbi for T where T: JsObject { - #[inline] - fn vector_none() -> WasmSlice { null_slice() } - } - impl VectorFromWasmAbi for T where T: JsObject { type Abi = WasmSlice; @@ -437,9 +404,4 @@ if_std! { vec.into_boxed_slice() } } - - impl OptionVectorFromWasmAbi for T where T: JsObject { - #[inline] - fn vector_is_none(slice: &WasmSlice) -> bool { slice.ptr == 0 } - } } diff --git a/src/convert/traits.rs b/src/convert/traits.rs index e84482a7e5b..51231a448d0 100644 --- a/src/convert/traits.rs +++ b/src/convert/traits.rs @@ -171,12 +171,6 @@ if_std! { fn vector_into_abi(vector: Box<[Self]>) -> Self::Abi; } - /// Trait for element types to implement OptionIntoWasmAbi for vectors - /// of themselves. - pub trait OptionVectorIntoWasmAbi: VectorIntoWasmAbi { - fn vector_none() -> Self::Abi; - } - /// Trait for element types to implement FromWasmAbi for vectors of /// themselves. pub trait VectorFromWasmAbi: WasmDescribeVector + Sized { @@ -184,10 +178,4 @@ if_std! { unsafe fn vector_from_abi(js: Self::Abi) -> Box<[Self]>; } - - /// Trait for element types to implement OptionFromWasmAbi for vectors - /// of themselves. - pub trait OptionVectorFromWasmAbi: VectorFromWasmAbi { - fn vector_is_none(abi: &Self::Abi) -> bool; - } } diff --git a/tests/wasm/slice.js b/tests/wasm/slice.js index c23535e9b1f..3e65dfc5238 100644 --- a/tests/wasm/slice.js +++ b/tests/wasm/slice.js @@ -6,39 +6,60 @@ exports.js_export = () => { i8[0] = 1; i8[1] = 2; assert.deepStrictEqual(wasm.export_i8(i8), i8); + assert.deepStrictEqual(wasm.export_optional_i8(i8), i8); const u8 = new Uint8Array(2); u8[0] = 1; u8[1] = 2; assert.deepStrictEqual(wasm.export_u8(u8), u8); + assert.deepStrictEqual(wasm.export_optional_u8(u8), u8); const i16 = new Int16Array(2); i16[0] = 1; i16[1] = 2; assert.deepStrictEqual(wasm.export_i16(i16), i16); + assert.deepStrictEqual(wasm.export_optional_i16(i16), i16); const u16 = new Uint16Array(2); u16[0] = 1; u16[1] = 2; assert.deepStrictEqual(wasm.export_u16(u16), u16); + assert.deepStrictEqual(wasm.export_optional_u16(u16), u16); const i32 = new Int32Array(2); i32[0] = 1; i32[1] = 2; assert.deepStrictEqual(wasm.export_i32(i32), i32); + assert.deepStrictEqual(wasm.export_optional_i32(i32), i32); assert.deepStrictEqual(wasm.export_isize(i32), i32); + assert.deepStrictEqual(wasm.export_optional_isize(i32), i32); const u32 = new Uint32Array(2); u32[0] = 1; u32[1] = 2; assert.deepStrictEqual(wasm.export_u32(u32), u32); + assert.deepStrictEqual(wasm.export_optional_u32(u32), u32); assert.deepStrictEqual(wasm.export_usize(u32), u32); + assert.deepStrictEqual(wasm.export_optional_usize(u32), u32); const f32 = new Float32Array(2); f32[0] = 1; f32[1] = 2; assert.deepStrictEqual(wasm.export_f32(f32), f32); + assert.deepStrictEqual(wasm.export_optional_f32(f32), f32); const f64 = new Float64Array(2); f64[0] = 1; f64[1] = 2; assert.deepStrictEqual(wasm.export_f64(f64), f64); + assert.deepStrictEqual(wasm.export_optional_f64(f64), f64); + + assert.strictEqual(wasm.export_optional_i8(undefined), undefined); + assert.strictEqual(wasm.export_optional_u8(undefined), undefined); + assert.strictEqual(wasm.export_optional_i16(undefined), undefined); + assert.strictEqual(wasm.export_optional_u16(undefined), undefined); + assert.strictEqual(wasm.export_optional_i32(undefined), undefined); + assert.strictEqual(wasm.export_optional_isize(undefined), undefined); + assert.strictEqual(wasm.export_optional_u32(undefined), undefined); + assert.strictEqual(wasm.export_optional_usize(undefined), undefined); + assert.strictEqual(wasm.export_optional_f32(undefined), undefined); + assert.strictEqual(wasm.export_optional_f64(undefined), undefined); }; const test_import = (a, b, c) => { diff --git a/tests/wasm/slice.rs b/tests/wasm/slice.rs index 6e659ce6f54..9f9066fe720 100644 --- a/tests/wasm/slice.rs +++ b/tests/wasm/slice.rs @@ -22,7 +22,7 @@ extern "C" { } macro_rules! export_macro { - ($(($i:ident, $n:ident))*) => ($( + ($(($i:ident, $n:ident, $optional_n:ident))*) => ($( #[wasm_bindgen] pub fn $n(a: &[$i]) -> Vec<$i> { assert_eq!(a.len(), 2); @@ -30,20 +30,30 @@ macro_rules! export_macro { assert_eq!(a[1], 2 as $i); a.to_vec() } + + #[wasm_bindgen] + pub fn $optional_n(a: Option>) -> Option> { + a.map(|a| { + assert_eq!(a.len(), 2); + assert_eq!(a[0], 1 as $i); + assert_eq!(a[1], 2 as $i); + a.to_vec() + }) + } )*) } export_macro! { - (i8, export_i8) - (u8, export_u8) - (i16, export_i16) - (u16, export_u16) - (i32, export_i32) - (u32, export_u32) - (isize, export_isize) - (usize, export_usize) - (f32, export_f32) - (f64, export_f64) + (i8, export_i8, export_optional_i8) + (u8, export_u8, export_optional_u8) + (i16, export_i16, export_optional_i16) + (u16, export_u16, export_optional_u16) + (i32, export_i32, export_optional_i32) + (u32, export_u32, export_optional_u32) + (isize, export_isize, export_optional_isize) + (usize, export_usize, export_optional_usize) + (f32, export_f32, export_optional_f32) + (f64, export_f64, export_optional_f64) } #[wasm_bindgen_test] diff --git a/tests/wasm/string_vecs.js b/tests/wasm/string_vecs.js index 1b708917e51..de9b0ef580a 100644 --- a/tests/wasm/string_vecs.js +++ b/tests/wasm/string_vecs.js @@ -2,7 +2,15 @@ const wasm = require('wasm-bindgen-test.js'); const assert = require('assert'); exports.pass_string_vec = () => { - wasm.consume_string_vec(["hello", "world"]); + assert.deepStrictEqual( + wasm.consume_string_vec(["hello", "world"]), + ["hello", "world", "Hello from Rust!"], + ); + assert.deepStrictEqual( + wasm.consume_optional_string_vec(["hello", "world"]), + ["hello", "world", "Hello from Rust!"], + ); + assert.strictEqual(wasm.consume_optional_string_vec(undefined), undefined); }; exports.pass_invalid_string_vec = () => { diff --git a/tests/wasm/string_vecs.rs b/tests/wasm/string_vecs.rs index dbf63e0da1b..1234d03d9b0 100644 --- a/tests/wasm/string_vecs.rs +++ b/tests/wasm/string_vecs.rs @@ -8,7 +8,15 @@ extern "C" { } #[wasm_bindgen] -pub fn consume_string_vec(_: Vec) {} +pub fn consume_string_vec(mut vec: Vec) -> Vec { + vec.push("Hello from Rust!".to_owned()); + vec +} + +#[wasm_bindgen] +pub fn consume_optional_string_vec(vec: Option>) -> Option> { + vec.map(consume_string_vec) +} #[wasm_bindgen_test] fn test_valid() { diff --git a/tests/wasm/struct_vecs.js b/tests/wasm/struct_vecs.js index fe376632386..73f4a85e643 100644 --- a/tests/wasm/struct_vecs.js +++ b/tests/wasm/struct_vecs.js @@ -4,7 +4,13 @@ const assert = require('assert'); exports.pass_struct_vec = () => { const el1 = new wasm.ArrayElement(); const el2 = new wasm.ArrayElement(); - wasm.consume_struct_vec([el1, el2]); + const ret = wasm.consume_struct_vec([el1, el2]); + assert.deepStrictEqual(ret.length, 3); + + const ret2 = wasm.consume_optional_struct_vec(ret); + assert.deepStrictEqual(ret2.length, 4); + + assert.strictEqual(wasm.consume_optional_struct_vec(undefined), undefined); }; exports.pass_invalid_struct_vec = () => { diff --git a/tests/wasm/struct_vecs.rs b/tests/wasm/struct_vecs.rs index 346d8ff149b..2440abc2037 100644 --- a/tests/wasm/struct_vecs.rs +++ b/tests/wasm/struct_vecs.rs @@ -19,7 +19,15 @@ impl ArrayElement { } #[wasm_bindgen] -pub fn consume_struct_vec(_: Vec) {} +pub fn consume_struct_vec(mut vec: Vec) -> Vec { + vec.push(ArrayElement); + vec +} + +#[wasm_bindgen] +pub fn consume_optional_struct_vec(vec: Option>) -> Option> { + vec.map(consume_struct_vec) +} #[wasm_bindgen_test] fn test_valid() { From 58667b29d6f6d123854b67112431b4822e6c0a39 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 18:46:58 +1000 Subject: [PATCH 15/24] fix clippy --- crates/shared/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index d2e3fa9962b..2b1ba52b3c9 100644 --- a/crates/shared/src/lib.rs +++ b/crates/shared/src/lib.rs @@ -166,10 +166,10 @@ pub fn free_function(struct_name: &str) -> String { } pub fn unwrap_function(struct_name: &str) -> String { - let mut name = format!("__wbg_"); + let mut name = "__wbg_".to_string(); name.extend(struct_name.chars().flat_map(|s| s.to_lowercase())); name.push_str("_unwrap"); - return name; + name } pub fn free_function_export_name(function_name: &str) -> String { From 8c96579f85dfd08e59e6d9d4e90fa73558004dcd Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 20:57:35 +1000 Subject: [PATCH 16/24] Fix generated typescript Since vecs of strings and rust structs were describing themselves as `Box<[JsValue]>`, they got typed as such - as `any[]`. This fixes that by using `NAMED_EXTERNREF` instead of just plain `EXTERNREF` with the type we want. This is maybe _slightly_ sketchy, since `NAMED_EXTERNREF` is meant for imported JS types, but ehhh it's fine. You can already put arbitrary TypeScript in there with `typescript_type`. --- crates/backend/src/codegen.rs | 9 ++++++--- src/convert/slices.rs | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 734fff7587f..c9657455a08 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -164,7 +164,7 @@ impl ToTokens for ast::Struct { let name = &self.rust_name; let name_str = self.js_name.to_string(); let name_len = name_str.len() as u32; - let name_chars = name_str.chars().map(|c| c as u32); + let name_chars: Vec = name_str.chars().map(|c| c as u32).collect(); let new_fn = Ident::new(&shared::new_function(&name_str), Span::call_site()); let free_fn = Ident::new(&shared::free_function(&name_str), Span::call_site()); let unwrap_fn = Ident::new(&shared::unwrap_function(&name_str), Span::call_site()); @@ -331,8 +331,11 @@ impl ToTokens for ast::Struct { impl #wasm_bindgen::describe::WasmDescribeVector for #name { fn describe_vector() { - use #wasm_bindgen::__rt::std::boxed::Box; - as #wasm_bindgen::describe::WasmDescribe>::describe(); + use #wasm_bindgen::describe::*; + inform(VECTOR); + inform(NAMED_EXTERNREF); + inform(#name_len); + #(inform(#name_chars);)* } } diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 51c619d3e9c..13dd4ad7d6b 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -11,7 +11,7 @@ use crate::convert::{ FromWasmAbi, IntoWasmAbi, LongRefFromWasmAbi, RefFromWasmAbi, RefMutFromWasmAbi, WasmAbi, }; use crate::convert::{VectorFromWasmAbi, VectorIntoWasmAbi}; -use crate::describe::{self, WasmDescribe, WasmDescribeVector}; +use crate::describe::*; use cfg_if::cfg_if; if_std! { @@ -82,7 +82,7 @@ macro_rules! vectors { if_std! { impl WasmDescribeVector for $t { fn describe_vector() { - describe::inform(describe::VECTOR); + inform(VECTOR); $t::describe(); } } @@ -186,7 +186,16 @@ vectors! { if_std! { impl WasmDescribeVector for String { fn describe_vector() { - >::describe(); + inform(VECTOR); + inform(NAMED_EXTERNREF); + // Trying to use an actual loop for this breaks the wasm interpreter. + inform(6); + inform('s' as u32); + inform('t' as u32); + inform('r' as u32); + inform('i' as u32); + inform('n' as u32); + inform('g' as u32); } } From af06b348fd4b808a201c208960c7939f55b685cb Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 21:08:22 +1000 Subject: [PATCH 17/24] reorder some impls This is the nitpickiest of nitpicks, but this is my PR goddammit and I can do what I want :) --- src/describe.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/describe.rs b/src/describe.rs index cb43cfe73d6..12a48554ac6 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -158,12 +158,6 @@ if_std! { } } - impl WasmDescribe for Box<[T]> { - fn describe() { - T::describe_vector(); - } - } - impl WasmDescribeVector for T { fn describe_vector() { inform(VECTOR); @@ -171,6 +165,12 @@ if_std! { } } + impl WasmDescribe for Box<[T]> { + fn describe() { + T::describe_vector(); + } + } + impl WasmDescribe for Vec where Box<[T]>: WasmDescribe { fn describe() { >::describe(); From a5c9fadb7e3bcfb383ade08830cd3a4a91924642 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 21:28:54 +1000 Subject: [PATCH 18/24] Update schema hash I didn't actually bump the schema version because it didn't change. If you don't use the `TryFrom` impl for Rust structs (or pass a `Vec` of them from JS to Rust), using an old CLI version will work fine; if you do though, you get a bit of a cryptic error message: ``` error: import of `__wbg_foo_unwrap` doesn't have an adapter listed ``` (That's from trying to pass a `Vec` from JS to Rust.) So idk, maybe that's worth bumping the schema version over anyway? --- crates/shared/src/schema_hash_approval.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/schema_hash_approval.rs b/crates/shared/src/schema_hash_approval.rs index 731dcea0214..4217bde716e 100644 --- a/crates/shared/src/schema_hash_approval.rs +++ b/crates/shared/src/schema_hash_approval.rs @@ -8,7 +8,7 @@ // If the schema in this library has changed then: // 1. Bump the version in `crates/shared/Cargo.toml` // 2. Change the `SCHEMA_VERSION` in this library to this new Cargo.toml version -const APPROVED_SCHEMA_FILE_HASH: &str = "12040133795598472740"; +const APPROVED_SCHEMA_FILE_HASH: &str = "5679641936258023729"; #[test] fn schema_version() { From 60f74d6e9fe067db87a1f98387d95d13ec2dfe2f Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 22:02:42 +1000 Subject: [PATCH 19/24] undo some unnecessary refactors --- src/convert/slices.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 13dd4ad7d6b..b356df5eb27 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -151,7 +151,7 @@ macro_rules! vectors { #[inline] unsafe fn ref_from_abi(js: WasmSlice) -> Box<[$t]> { - as FromWasmAbi>::from_abi(js) + >::from_abi(js) } } @@ -161,7 +161,7 @@ macro_rules! vectors { #[inline] unsafe fn ref_mut_from_abi(js: WasmMutSlice) -> MutSlice<$t> { - let contents = as FromWasmAbi>::from_abi(js.slice); + let contents = >::from_abi(js.slice); let js = JsValue::from_abi(js.idx); MutSlice { contents, js } } From efc958e792ef492ce04ed79dd2bca89855c6b842 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 22:03:17 +1000 Subject: [PATCH 20/24] don't pointlessly use assert.deepStrictEqual for numbers --- tests/wasm/struct_vecs.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/wasm/struct_vecs.js b/tests/wasm/struct_vecs.js index 73f4a85e643..23eb154fd80 100644 --- a/tests/wasm/struct_vecs.js +++ b/tests/wasm/struct_vecs.js @@ -5,10 +5,10 @@ exports.pass_struct_vec = () => { const el1 = new wasm.ArrayElement(); const el2 = new wasm.ArrayElement(); const ret = wasm.consume_struct_vec([el1, el2]); - assert.deepStrictEqual(ret.length, 3); + assert.strictEqual(ret.length, 3); const ret2 = wasm.consume_optional_struct_vec(ret); - assert.deepStrictEqual(ret2.length, 4); + assert.strictEqual(ret2.length, 4); assert.strictEqual(wasm.consume_optional_struct_vec(undefined), undefined); }; From 68f2d8ff5db3871d60215c7f67b2c26231cab444 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 22:18:58 +1000 Subject: [PATCH 21/24] Update the guide --- guide/src/SUMMARY.md | 2 +- .../{boxed-jsvalue-slice.md => boxed-slices.md} | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) rename guide/src/reference/types/{boxed-jsvalue-slice.md => boxed-slices.md} (58%) diff --git a/guide/src/SUMMARY.md b/guide/src/SUMMARY.md index 9ef9c260ff6..5d9c362902c 100644 --- a/guide/src/SUMMARY.md +++ b/guide/src/SUMMARY.md @@ -52,7 +52,7 @@ - [Imported JavaScript Types](./reference/types/imported-js-types.md) - [Exported Rust Types](./reference/types/exported-rust-types.md) - [`JsValue`](./reference/types/jsvalue.md) - - [`Box<[JsValue]>`](./reference/types/boxed-jsvalue-slice.md) + - [`Box<[T]>` and `Vec`](./reference/types/boxed-slices.md) - [`*const T` and `*mut T`](./reference/types/pointers.md) - [Numbers](./reference/types/numbers.md) - [`bool`](./reference/types/bool.md) diff --git a/guide/src/reference/types/boxed-jsvalue-slice.md b/guide/src/reference/types/boxed-slices.md similarity index 58% rename from guide/src/reference/types/boxed-jsvalue-slice.md rename to guide/src/reference/types/boxed-slices.md index 8060d194a7c..ed299462292 100644 --- a/guide/src/reference/types/boxed-jsvalue-slice.md +++ b/guide/src/reference/types/boxed-slices.md @@ -1,10 +1,19 @@ -# `Box<[JsValue]>` +# `Box<[T]>` and `Vec` | `T` parameter | `&T` parameter | `&mut T` parameter | `T` return value | `Option` parameter | `Option` return value | JavaScript representation | |:---:|:---:|:---:|:---:|:---:|:---:|:---:| | Yes | No | No | Yes | Yes | Yes | A JavaScript `Array` object | -Boxed slices of imported JS types and exported Rust types are also supported. `Vec` is supported wherever `Box<[T]>` is. +You can pass boxed slices and `Vec`s of several different types to and from JS: + +- `JsValue`s. +- Imported JavaScript types. +- Exported Rust types. +- `String`s. + +[You can also pass boxed slices of numbers to JS](boxed-number-slices.html), +except that they're converted to typed arrays (`Uint8Array`, `Int32Array`, etc.) +instead of regular arrays. ## Example Rust Usage From 5f1808a157916813ef646b0ece7247490e71d60e Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 10 Aug 2023 23:01:55 +1000 Subject: [PATCH 22/24] update reference tests --- crates/macro/ui-tests/missing-catch.stderr | 12 ++++++------ crates/macro/ui-tests/traits-not-implemented.stderr | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/macro/ui-tests/missing-catch.stderr b/crates/macro/ui-tests/missing-catch.stderr index 55466de06d6..5e54e41e7f7 100644 --- a/crates/macro/ui-tests/missing-catch.stderr +++ b/crates/macro/ui-tests/missing-catch.stderr @@ -8,9 +8,9 @@ error[E0277]: the trait bound `Result - Box<[f32]> - Box<[f64]> - Box<[i16]> - Box<[i32]> - Box<[i64]> - and 35 others + Clamped + Option + Option + Option + Option + and $N others diff --git a/crates/macro/ui-tests/traits-not-implemented.stderr b/crates/macro/ui-tests/traits-not-implemented.stderr index 5b8e122287f..714050a53b5 100644 --- a/crates/macro/ui-tests/traits-not-implemented.stderr +++ b/crates/macro/ui-tests/traits-not-implemented.stderr @@ -13,5 +13,5 @@ error[E0277]: the trait bound `A: IntoWasmAbi` is not satisfied &'a (dyn Fn(A, B, C, D, E) -> R + 'b) &'a (dyn Fn(A, B, C, D, E, F) -> R + 'b) &'a (dyn Fn(A, B, C, D, E, F, G) -> R + 'b) - and 84 others + and $N others = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) From 85167263f43825d519cd9dc18ace9ee47348ab47 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Sat, 26 Aug 2023 17:36:36 +1000 Subject: [PATCH 23/24] add WASI check --- crates/backend/src/codegen.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index c9657455a08..837245d5e6d 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -304,12 +304,12 @@ impl ToTokens for ast::Struct { let idx = #wasm_bindgen::convert::IntoWasmAbi::into_abi(&value); #[link(wasm_import_module = "__wbindgen_placeholder__")] - #[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))] + #[cfg(all(target_arch = "wasm32", not(any(target_os = "emscripten", target_os = "wasi"))))] extern "C" { fn #unwrap_fn(ptr: u32) -> u32; } - #[cfg(not(all(target_arch = "wasm32", not(target_os = "emscripten"))))] + #[cfg(not(all(target_arch = "wasm32", not(any(target_os = "emscripten", target_os = "wasi")))))] unsafe fn #unwrap_fn(_: u32) -> u32 { panic!("cannot convert from JsValue outside of the wasm target") } From e6df778f7405a03dca6fbd437c258db9d64078dd Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Mon, 4 Sep 2023 17:22:13 +1000 Subject: [PATCH 24/24] Extremely nitpicky tweaks --- src/convert/impls.rs | 6 ++++-- src/convert/traits.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/convert/impls.rs b/src/convert/impls.rs index 82f7fa820aa..637d9d979ad 100644 --- a/src/convert/impls.rs +++ b/src/convert/impls.rs @@ -9,8 +9,8 @@ use crate::{Clamped, JsError, JsValue, UnwrapThrowExt}; if_std! { use std::boxed::Box; use std::convert::{TryFrom, TryInto}; - use std::vec::Vec; use std::fmt::Debug; + use std::vec::Vec; } unsafe impl WasmAbi for () {} @@ -395,8 +395,10 @@ impl IntoWasmAbi for JsError { } if_std! { + // Note: this can't take `&[T]` because the `Into` impl needs + // ownership of `T`. pub fn js_value_vector_into_abi>(vector: Box<[T]>) -> as IntoWasmAbi>::Abi { - let js_vals: Box::<[JsValue]> = vector + let js_vals: Box<[JsValue]> = vector .into_vec() .into_iter() .map(|x| x.into()) diff --git a/src/convert/traits.rs b/src/convert/traits.rs index 51231a448d0..dc74caae15a 100644 --- a/src/convert/traits.rs +++ b/src/convert/traits.rs @@ -160,8 +160,8 @@ impl ReturnWasmAbi for T { } if_std! { - use std::boxed::Box; use core::marker::Sized; + use std::boxed::Box; /// Trait for element types to implement IntoWasmAbi for vectors of /// themselves.