Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable passing String and custom struct boxed slices across ABI and implement TryFrom<JsValue> for custom structs #2734

85 changes: 85 additions & 0 deletions crates/backend/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,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 free_fn_const = Ident::new(&format!("{}__const", free_fn), free_fn.span());
(quote! {
#[automatically_derived]
Expand Down Expand Up @@ -266,6 +267,90 @@ 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<wasm_bindgen::JsValue> for #name {
type Error = ();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type Error = ();
type Error = JsValue;

It's not great practice to use () as an error type. Normally I'd probably suggest creating a new error type for this, but all of wasm-bindgen's existing TryFrom<JsValue> impls use JsValue for their error type and so I think it's a better idea to keep things consistent.


fn try_from(value: wasm_bindgen::JsValue)
-> wasm_bindgen::__rt::std::result::Result<Self, Self::Error> {
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(
<Self as wasm_bindgen::convert::FromWasmAbi>::from_abi(ptr)
)
}
}
}
}

impl wasm_bindgen::describe::WasmDescribeVector for #name {
fn describe_vector() {
use wasm_bindgen::__rt::std::boxed::Box;
<Box<[#name]> as wasm_bindgen::convert::JsValueVector>::describe();
}
}

impl wasm_bindgen::convert::VectorIntoWasmAbi for #name {
type Abi = <
wasm_bindgen::__rt::std::boxed::Box<[#name]>
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;

<Box<[#name]> as wasm_bindgen::convert::JsValueVector>::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 {
use wasm_bindgen::__rt::std::boxed::Box;
<Box<[#name]> as wasm_bindgen::convert::JsValueVector>::none()
}
}

impl wasm_bindgen::convert::VectorFromWasmAbi for #name {
type Abi = <
wasm_bindgen::__rt::std::boxed::Box<[#name]>
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;

<Box<[#name]> as wasm_bindgen::convert::JsValueVector>::from_abi(js)
}
}

})
.to_tokens(tokens);

Expand Down
27 changes: 27 additions & 0 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub struct ExportedClass {
typescript: String,
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
Expand Down Expand Up @@ -844,6 +845,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));",
Expand Down Expand Up @@ -2089,6 +2104,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
Expand Down Expand Up @@ -2918,6 +2937,14 @@ impl<'a> Context<'a> {
assert!(!variadic);
self.invoke_intrinsic(intrinsic, args, prelude)
}

AuxImport::UnwrapExportedClass(class) => {
assert!(kind == AdapterJsImportKind::Normal);
assert!(!variadic);
assert_eq!(args.len(), 1);
self.require_class_unwrap(class);
Ok(format!("{}.__unwrap({})", class, args[0]))
}
}
}

Expand Down
35 changes: 29 additions & 6 deletions crates/cli-support/src/wit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,17 +858,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<Descriptor>,
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(())
Expand Down
5 changes: 5 additions & 0 deletions crates/cli-support/src/wit/nonstandard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,11 @@ pub enum AuxImport {
/// This is an intrinsic function expected to be implemented with a JS glue
/// shim. Each intrinsic has its own expected signature and implementation.
Intrinsic(Intrinsic),

/// 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.
Expand Down
3 changes: 3 additions & 0 deletions crates/cli-support/src/wit/section.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,9 @@ fn check_standard_import(import: &AuxImport) -> Result<(), Error> {
format!("wasm-bindgen specific intrinsic `{}`", intrinsic.name())
}
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);
}
Expand Down
7 changes: 7 additions & 0 deletions crates/shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ pub fn free_function(struct_name: &str) -> String {
return 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()
}
Expand Down
45 changes: 45 additions & 0 deletions src/convert/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,16 @@ use core::mem::{self, ManuallyDrop};
use crate::convert::traits::WasmAbi;
use crate::convert::{FromWasmAbi, IntoWasmAbi, 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)]
Expand Down Expand Up @@ -465,6 +473,43 @@ impl<T: IntoWasmAbi, E: Into<JsValue>> ReturnWasmAbi for Result<T, E> {
}
}

if_std! {
impl<T> JsValueVector for Box<[T]> where
T: Into<JsValue> + TryFrom<JsValue>,
<T as TryFrom<JsValue>>::Error: core::fmt::Debug {
type ToAbi = <Box<[JsValue]> as IntoWasmAbi>::Abi;
type FromAbi = <Box<[JsValue]> 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();
Comment on lines +489 to +493
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels kind of wrong to be throwing away the initial allocation and making a new one here, but it works for an MVP and can be optimised later if necessary.

I have some ideas how this can be avoided by doing more work in JS, but I have no idea if that'd actually end up being faster, so let's just go with this for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly my thinking, let's not let perfect be the enemy of good.


IntoWasmAbi::into_abi(js_vals)
}

fn none() -> Self::ToAbi {
<Box<[JsValue]> as OptionIntoWasmAbi>::none()
}

unsafe fn from_abi(js: Self::FromAbi) -> Self {
let js_vals = <Vec<JsValue> as FromWasmAbi>::from_abi(js);

js_vals
.into_iter()
.filter_map(|x| x.try_into().ok())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like a bad idea to silently throw away array elements that fail to be converted; I'd prefer if this used unwrap_throw to propagate the error instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. No idea why I did this tbh. I thought I made it so it would panic.

.collect()
}
}
}

impl IntoWasmAbi for JsError {
type Abi = <JsValue as IntoWasmAbi>::Abi;

Expand Down
Loading