From e1401127e2ad34336e04dc12cedaaf9d6dbb3b0d Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Wed, 6 Nov 2024 14:23:16 +0100 Subject: [PATCH 1/7] Add tests for static getters/setters --- .../tests/reference/import-getter-setter.d.ts | 3 + .../tests/reference/import-getter-setter.js | 75 +++++++++++++++++++ .../tests/reference/import-getter-setter.rs | 35 +++++++++ .../tests/reference/import-getter-setter.wat | 9 +++ 4 files changed, 122 insertions(+) create mode 100644 crates/cli/tests/reference/import-getter-setter.d.ts create mode 100644 crates/cli/tests/reference/import-getter-setter.js create mode 100644 crates/cli/tests/reference/import-getter-setter.rs create mode 100644 crates/cli/tests/reference/import-getter-setter.wat diff --git a/crates/cli/tests/reference/import-getter-setter.d.ts b/crates/cli/tests/reference/import-getter-setter.d.ts new file mode 100644 index 00000000000..091dbcc51a6 --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.d.ts @@ -0,0 +1,3 @@ +/* tslint:disable */ +/* eslint-disable */ +export function exported(): void; diff --git a/crates/cli/tests/reference/import-getter-setter.js b/crates/cli/tests/reference/import-getter-setter.js new file mode 100644 index 00000000000..fd992fe1533 --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.js @@ -0,0 +1,75 @@ +let wasm; +export function __wbg_set_wasm(val) { + wasm = val; +} + + +const heap = new Array(128).fill(undefined); + +heap.push(undefined, null, true, false); + +let heap_next = heap.length; + +function addHeapObject(obj) { + if (heap_next === heap.length) heap.push(heap.length + 1); + const idx = heap_next; + heap_next = heap[idx]; + + heap[idx] = obj; + return idx; +} + +function getObject(idx) { return heap[idx]; } + +function dropObject(idx) { + if (idx < 132) return; + heap[idx] = heap_next; + heap_next = idx; +} + +function takeObject(idx) { + const ret = getObject(idx); + dropObject(idx); + return ret; +} + +export function exported() { + wasm.exported(); +} + +export function __wbg_bar2_84566b6bcf547b19() { + const ret = Foo.bar2(); + return ret; +}; + +export function __wbg_getfoo_eae0175044c62418() { + const ret = Foo.get_foo(); + return ret; +}; + +export function __wbg_new_d728785ba7e8df96() { + const ret = new SomeClass(); + return addHeapObject(ret); +}; + +export function __wbg_setbar2_c8b4a150c6accea2(arg0) { + Foo.set_bar2(arg0 >>> 0); +}; + +export function __wbg_setfoo_6c6b6af72f779234(arg0) { + Foo.set_foo(arg0 >>> 0); +}; + +export function __wbg_setsignal_d386d151e7775c3f(arg0, arg1) { + getObject(arg0).signal = arg1 >>> 0; +}; + +export function __wbg_signal_b82e5486ce265c55(arg0) { + const ret = getObject(arg0).signal; + return ret; +}; + +export function __wbindgen_object_drop_ref(arg0) { + takeObject(arg0); +}; + diff --git a/crates/cli/tests/reference/import-getter-setter.rs b/crates/cli/tests/reference/import-getter-setter.rs new file mode 100644 index 00000000000..d09b2c2cd0b --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.rs @@ -0,0 +1,35 @@ +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(js_namespace = Foo, getter = bar)] + fn get_foo() -> u32; + #[wasm_bindgen(js_namespace = Foo, setter = bar)] + fn set_foo(value: u32); + #[wasm_bindgen(js_namespace = Foo, getter, js_name = bar2)] + fn get_foo2() -> u32; + #[wasm_bindgen(js_namespace = Foo, setter, js_name = bar2)] + fn set_foo2(value: u32); + + #[wasm_bindgen] + #[derive(Debug, Clone, PartialEq)] + pub type SomeClass; + #[wasm_bindgen(method, getter, js_class = "SomeClass", js_name = signal)] + pub fn signal(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter, js_class = "SomeClass", js_name = signal)] + pub fn set_signal(this: &SomeClass, value: u32); + // #[wasm_bindgen(getter, js_class = "SomeClass", js_name = static_signal)] + // pub fn static_controller() -> SomeClass; + #[wasm_bindgen(constructor, js_class = "SomeClass")] + pub fn new() -> SomeClass; +} + +#[wasm_bindgen] +pub fn exported() { + set_foo(get_foo()); + set_foo2(get_foo2()); + + let a = SomeClass::new(); + a.set_signal(a.signal()); + // let _ = static_signal(); +} diff --git a/crates/cli/tests/reference/import-getter-setter.wat b/crates/cli/tests/reference/import-getter-setter.wat new file mode 100644 index 00000000000..e05a3a8c0ac --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.wat @@ -0,0 +1,9 @@ +(module $reference_test.wasm + (type (;0;) (func)) + (func $exported (;0;) (type 0)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "exported" (func $exported)) + (@custom "target_features" (after code) "\04+\0amultivalue+\0fmutable-globals+\0freference-types+\08sign-ext") +) + From 9772126490245f0abbb64aedac91fd99d4f6056c Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 14 Nov 2024 13:23:19 +0100 Subject: [PATCH 2/7] WIP --- crates/backend/src/ast.rs | 4 ++-- crates/backend/src/encode.rs | 23 +++++------------------ crates/shared/src/lib.rs | 18 +++++++++--------- 3 files changed, 16 insertions(+), 29 deletions(-) diff --git a/crates/backend/src/ast.rs b/crates/backend/src/ast.rs index 0991dac66cf..ef3ff935654 100644 --- a/crates/backend/src/ast.rs +++ b/crates/backend/src/ast.rs @@ -248,9 +248,9 @@ pub enum OperationKind { /// A standard method, nothing special Regular, /// A method for getting the value of the provided Ident or String - Getter(Option), + Getter, /// A method for setting the value of the provided Ident or String - Setter(Option), + Setter, /// A dynamically intercepted getter IndexingGetter, /// A dynamically intercepted setter diff --git a/crates/backend/src/encode.rs b/crates/backend/src/encode.rs index baae8d72533..5b3ea5ed615 100644 --- a/crates/backend/src/encode.rs +++ b/crates/backend/src/encode.rs @@ -203,7 +203,7 @@ fn shared_export<'a>( intern: &'a Interner, ) -> Result, Diagnostic> { let consumed = matches!(export.method_self, Some(ast::MethodSelf::ByValue)); - let method_kind = from_ast_method_kind(&export.function, intern, &export.method_kind)?; + let method_kind = from_ast_method_kind(&export.method_kind)?; Ok(Export { class: export.js_class.as_deref(), comments: export.comments.iter().map(|s| &**s).collect(), @@ -321,7 +321,7 @@ fn shared_import_function<'a>( ) -> Result, Diagnostic> { let method = match &i.kind { ast::ImportFunctionKind::Method { class, kind, .. } => { - let kind = from_ast_method_kind(&i.function, intern, kind)?; + let kind = from_ast_method_kind(kind)?; Some(MethodData { class, kind }) } ast::ImportFunctionKind::Normal => None, @@ -584,28 +584,15 @@ macro_rules! encode_api { } wasm_bindgen_shared::shared_api!(encode_api); -fn from_ast_method_kind<'a>( - function: &'a ast::Function, - intern: &'a Interner, - method_kind: &'a ast::MethodKind, -) -> Result, Diagnostic> { +fn from_ast_method_kind(method_kind: &ast::MethodKind) -> Result { Ok(match method_kind { ast::MethodKind::Constructor => MethodKind::Constructor, ast::MethodKind::Operation(ast::Operation { is_static, kind }) => { let is_static = *is_static; let kind = match kind { - ast::OperationKind::Getter(g) => { - let g = g.as_ref().map(|g| intern.intern_str(g)); - OperationKind::Getter(g.unwrap_or_else(|| function.infer_getter_property())) - } ast::OperationKind::Regular => OperationKind::Regular, - ast::OperationKind::Setter(s) => { - let s = s.as_ref().map(|s| intern.intern_str(s)); - OperationKind::Setter(match s { - Some(s) => s, - None => intern.intern_str(&function.infer_setter_property()?), - }) - } + ast::OperationKind::Getter => OperationKind::Getter, + ast::OperationKind::Setter => OperationKind::Setter, ast::OperationKind::IndexingGetter => OperationKind::IndexingGetter, ast::OperationKind::IndexingSetter => OperationKind::IndexingSetter, ast::OperationKind::IndexingDeleter => OperationKind::IndexingDeleter, diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index 13b893e4685..061710307d1 100644 --- a/crates/shared/src/lib.rs +++ b/crates/shared/src/lib.rs @@ -66,23 +66,23 @@ macro_rules! shared_api { struct MethodData<'a> { class: &'a str, - kind: MethodKind<'a>, + kind: MethodKind, } - enum MethodKind<'a> { + enum MethodKind { Constructor, - Operation(Operation<'a>), + Operation(Operation), } - struct Operation<'a> { + struct Operation { is_static: bool, - kind: OperationKind<'a>, + kind: OperationKind, } - enum OperationKind<'a> { + enum OperationKind { Regular, - Getter(&'a str), - Setter(&'a str), + Getter, + Setter, IndexingGetter, IndexingSetter, IndexingDeleter, @@ -116,7 +116,7 @@ macro_rules! shared_api { comments: Vec<&'a str>, consumed: bool, function: Function<'a>, - method_kind: MethodKind<'a>, + method_kind: MethodKind, start: bool, } From c6fa1ea30943829d0ce293fcbe3f3aa1e2389fd8 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 14 Nov 2024 15:10:01 +0100 Subject: [PATCH 3/7] Final --- crates/backend/src/ast.rs | 2 - crates/cli-support/src/wit/mod.rs | 59 +++++----- crates/cli/tests/reference/getter-setter.js | 4 +- crates/cli/tests/reference/getter-setter.wat | 8 +- .../tests/reference/import-getter-setter.js | 39 ++++++- .../tests/reference/import-getter-setter.rs | 27 ++++- crates/macro-support/src/parser.rs | 103 +++++++++++------- 7 files changed, 159 insertions(+), 83 deletions(-) diff --git a/crates/backend/src/ast.rs b/crates/backend/src/ast.rs index ef3ff935654..36b54fac062 100644 --- a/crates/backend/src/ast.rs +++ b/crates/backend/src/ast.rs @@ -361,8 +361,6 @@ pub struct Function { pub name: String, /// The span of the function's name in Rust code pub name_span: Span, - /// Whether the function has a js_name attribute - pub renamed_via_js_name: bool, /// The arguments to the function pub arguments: Vec, /// The return type of the function, if provided diff --git a/crates/cli-support/src/wit/mod.rs b/crates/cli-support/src/wit/mod.rs index 07a09860e0c..46a0aabbb97 100644 --- a/crates/cli-support/src/wit/mod.rs +++ b/crates/cli-support/src/wit/mod.rs @@ -500,8 +500,18 @@ impl<'a> Context<'a> { } let (name, kind) = match op.kind { - decode::OperationKind::Getter(f) => (f, AuxExportedMethodKind::Getter), - decode::OperationKind::Setter(f) => (f, AuxExportedMethodKind::Setter), + decode::OperationKind::Getter => { + (export.function.name, AuxExportedMethodKind::Getter) + } + decode::OperationKind::Setter => { + let Some(name) = export.function.name.strip_prefix("set_") else { + bail!( + "The setter function `{}` must start with `set_`", + export.function.name + ); + }; + (name, AuxExportedMethodKind::Setter) + } _ => (export.function.name, AuxExportedMethodKind::Method), }; @@ -680,62 +690,55 @@ impl<'a> Context<'a> { mut class: JsImport, function: &decode::Function<'_>, structural: bool, - op: &decode::Operation<'_>, + op: &decode::Operation, ) -> Result<(AuxImport, bool), Error> { + let name = function.name.to_string(); + match op.kind { decode::OperationKind::Regular => { if op.is_static { - Ok(( - AuxImport::ValueWithThis(class, function.name.to_string()), - false, - )) + Ok((AuxImport::ValueWithThis(class, name), false)) } else if structural { - Ok(( - AuxImport::StructuralMethod(function.name.to_string()), - false, - )) + Ok((AuxImport::StructuralMethod(name), false)) } else { class.fields.push("prototype".to_string()); - class.fields.push(function.name.to_string()); + class.fields.push(name); Ok((AuxImport::Value(AuxValue::Bare(class)), true)) } } - decode::OperationKind::Getter(field) => { + decode::OperationKind::Getter => { if structural { if op.is_static { - Ok(( - AuxImport::StructuralClassGetter(class, field.to_string()), - false, - )) + Ok((AuxImport::StructuralClassGetter(class, name), false)) } else { - Ok((AuxImport::StructuralGetter(field.to_string()), false)) + Ok((AuxImport::StructuralGetter(name), false)) } } else { let val = if op.is_static { - AuxValue::ClassGetter(class, field.to_string()) + AuxValue::ClassGetter(class, name) } else { - AuxValue::Getter(class, field.to_string()) + AuxValue::Getter(class, name) }; Ok((AuxImport::Value(val), true)) } } - decode::OperationKind::Setter(field) => { + decode::OperationKind::Setter => { + let Some(name) = name.strip_prefix("set_").map(|s| s.to_string()) else { + bail!("The setter function `{}` must start with `set_`", name); + }; if structural { if op.is_static { - Ok(( - AuxImport::StructuralClassSetter(class, field.to_string()), - false, - )) + Ok((AuxImport::StructuralClassSetter(class, name), false)) } else { - Ok((AuxImport::StructuralSetter(field.to_string()), false)) + Ok((AuxImport::StructuralSetter(name), false)) } } else { let val = if op.is_static { - AuxValue::ClassSetter(class, field.to_string()) + AuxValue::ClassSetter(class, name) } else { - AuxValue::Setter(class, field.to_string()) + AuxValue::Setter(class, name) }; Ok((AuxImport::Value(val), true)) } diff --git a/crates/cli/tests/reference/getter-setter.js b/crates/cli/tests/reference/getter-setter.js index 6c05b3065a9..fdb837abe1f 100644 --- a/crates/cli/tests/reference/getter-setter.js +++ b/crates/cli/tests/reference/getter-setter.js @@ -180,14 +180,14 @@ export class Foo { * @returns {boolean | undefined} */ static get x() { - const ret = wasm.foo_x_static(); + const ret = wasm.foo_x(); return ret === 0xFFFFFF ? undefined : ret !== 0; } /** * @param {boolean | undefined} [value] */ static set x(value) { - wasm.foo_set_x_static(isLikeNone(value) ? 0xFFFFFF : value ? 1 : 0); + wasm.foo_set_x(isLikeNone(value) ? 0xFFFFFF : value ? 1 : 0); } } diff --git a/crates/cli/tests/reference/getter-setter.wat b/crates/cli/tests/reference/getter-setter.wat index 3daf51bf7a5..d9efb967420 100644 --- a/crates/cli/tests/reference/getter-setter.wat +++ b/crates/cli/tests/reference/getter-setter.wat @@ -20,9 +20,9 @@ (func $foo_lone_getter (;9;) (type 3) (param i32) (result f64)) (func $__wbg_set_foo_x (;10;) (type 4) (param i32 i32)) (func $foo_weird (;11;) (type 2) (param i32) (result i32)) - (func $foo_x_static (;12;) (type 0) (result i32)) + (func $foo_x (;12;) (type 0) (result i32)) (func $__wbg_foo_free (;13;) (type 4) (param i32 i32)) - (func $foo_set_x_static (;14;) (type 1) (param i32)) + (func $foo_set_x (;14;) (type 1) (param i32)) (memory (;0;) 17) (export "memory" (memory 0)) (export "__wbg_foo_free" (func $__wbg_foo_free)) @@ -36,8 +36,8 @@ (export "foo_set_lone_setter" (func $foo_set_lone_setter)) (export "foo_weird" (func $foo_weird)) (export "foo_set_weird" (func $foo_set_weird)) - (export "foo_x_static" (func $foo_x_static)) - (export "foo_set_x_static" (func $foo_set_x_static)) + (export "foo_x" (func $foo_x)) + (export "foo_set_x" (func $foo_set_x)) (export "__wbindgen_malloc" (func $__wbindgen_malloc)) (export "__wbindgen_realloc" (func $__wbindgen_realloc)) (@custom "target_features" (after code) "\04+\0amultivalue+\0fmutable-globals+\0freference-types+\08sign-ext") diff --git a/crates/cli/tests/reference/import-getter-setter.js b/crates/cli/tests/reference/import-getter-setter.js index fd992fe1533..e03646fc8b6 100644 --- a/crates/cli/tests/reference/import-getter-setter.js +++ b/crates/cli/tests/reference/import-getter-setter.js @@ -8,6 +8,8 @@ const heap = new Array(128).fill(undefined); heap.push(undefined, null, true, false); +function getObject(idx) { return heap[idx]; } + let heap_next = heap.length; function addHeapObject(obj) { @@ -19,8 +21,6 @@ function addHeapObject(obj) { return idx; } -function getObject(idx) { return heap[idx]; } - function dropObject(idx) { if (idx < 132) return; heap[idx] = heap_next; @@ -37,13 +37,18 @@ export function exported() { wasm.exported(); } +export function __wbg_a_f17802a22332a667(arg0) { + const ret = getObject(arg0).a; + return ret; +}; + export function __wbg_bar2_84566b6bcf547b19() { const ret = Foo.bar2(); return ret; }; -export function __wbg_getfoo_eae0175044c62418() { - const ret = Foo.get_foo(); +export function __wbg_bar_eae0175044c62418() { + const ret = Foo.bar(); return ret; }; @@ -52,23 +57,45 @@ export function __wbg_new_d728785ba7e8df96() { return addHeapObject(ret); }; +export function __wbg_prop2_edf1002f8e41a5da(arg0) { + const ret = getObject(arg0).prop2; + return ret; +}; + +export function __wbg_seta_40f909ae19a05c10(arg0, arg1) { + getObject(arg0).a = arg1 >>> 0; +}; + export function __wbg_setbar2_c8b4a150c6accea2(arg0) { Foo.set_bar2(arg0 >>> 0); }; -export function __wbg_setfoo_6c6b6af72f779234(arg0) { - Foo.set_foo(arg0 >>> 0); +export function __wbg_setbar_6c6b6af72f779234(arg0) { + Foo.set_bar(arg0 >>> 0); +}; + +export function __wbg_setprop2_2d160a2b6600e990(arg0, arg1) { + getObject(arg0).prop2 = arg1 >>> 0; }; export function __wbg_setsignal_d386d151e7775c3f(arg0, arg1) { getObject(arg0).signal = arg1 >>> 0; }; +export function __wbg_setsomeprop_afbca63b5d0f4c92(arg0, arg1) { + getObject(arg0).some_prop = arg1 >>> 0; +}; + export function __wbg_signal_b82e5486ce265c55(arg0) { const ret = getObject(arg0).signal; return ret; }; +export function __wbg_someprop_26178791e2719528(arg0) { + const ret = getObject(arg0).some_prop; + return ret; +}; + export function __wbindgen_object_drop_ref(arg0) { takeObject(arg0); }; diff --git a/crates/cli/tests/reference/import-getter-setter.rs b/crates/cli/tests/reference/import-getter-setter.rs index d09b2c2cd0b..bb9ef8cea40 100644 --- a/crates/cli/tests/reference/import-getter-setter.rs +++ b/crates/cli/tests/reference/import-getter-setter.rs @@ -14,14 +14,36 @@ extern "C" { #[wasm_bindgen] #[derive(Debug, Clone, PartialEq)] pub type SomeClass; + #[wasm_bindgen(method, getter, js_class = "SomeClass", js_name = signal)] pub fn signal(this: &SomeClass) -> u32; #[wasm_bindgen(method, setter, js_class = "SomeClass", js_name = signal)] pub fn set_signal(this: &SomeClass, value: u32); - // #[wasm_bindgen(getter, js_class = "SomeClass", js_name = static_signal)] + + #[wasm_bindgen(method, getter, js_class = "SomeClass")] + pub fn some_prop(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter, js_class = "SomeClass")] + pub fn set_some_prop(this: &SomeClass, value: u32); + + #[wasm_bindgen(method, getter = prop2, js_class = "SomeClass")] + pub fn another(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter = prop2, js_class = "SomeClass")] + pub fn set_another(this: &SomeClass, value: u32); + + // #[wasm_bindgen(getter, js_class = "SomeClass")] // pub fn static_controller() -> SomeClass; + // #[wasm_bindgen(getter, js_class = "SomeClass")] + // pub fn set_static_controller(value: &SomeClass); + #[wasm_bindgen(constructor, js_class = "SomeClass")] pub fn new() -> SomeClass; + + // js_name conflicts with the getter/setter name + #[wasm_bindgen(method, getter = a, js_class = "SomeClass", js_name = b)] + pub fn c(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter = a, js_class = "SomeClass", js_name = b)] + pub fn set_c(this: &SomeClass, value: u32); + } #[wasm_bindgen] @@ -31,5 +53,8 @@ pub fn exported() { let a = SomeClass::new(); a.set_signal(a.signal()); + a.set_some_prop(a.some_prop()); + a.set_another(a.another()); + a.set_c(a.c()); // let _ = static_signal(); } diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index 72bd849d859..d5c6e3b8ed3 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -67,8 +67,8 @@ macro_rules! attrgen { (module, Module(Span, String, Span)), (raw_module, RawModule(Span, String, Span)), (inline_js, InlineJs(Span, String, Span)), - (getter, Getter(Span, Option)), - (setter, Setter(Span, Option)), + (getter, Getter(Span, Option, Span)), + (setter, Setter(Span, Option, Span)), (indexing_getter, IndexingGetter(Span)), (indexing_setter, IndexingSetter(Span)), (indexing_deleter, IndexingDeleter(Span)), @@ -159,6 +159,20 @@ macro_rules! methods { } }; + (@method $name:ident, $variant:ident(Span, Option, Span)) => { + fn $name(&self) -> Option<(Option<&str>, Span)> { + self.attrs + .iter() + .find_map(|a| match &a.1 { + BindgenAttr::$variant(_, s, span) => { + a.0.set(true); + Some((s.as_ref().map(|s| &s[..]), *span)) + } + _ => None, + }) + } + }; + (@method $name:ident, $variant:ident(Span, Vec, Vec)) => { fn $name(&self) -> Option<(&[String], &[Span])> { self.attrs @@ -304,17 +318,18 @@ impl Parse for BindgenAttr { return Ok(BindgenAttr::$variant(attr_span, ident)) }); - (@parser $variant:ident(Span, Option)) => ({ + (@parser $variant:ident(Span, Option, Span)) => ({ if input.parse::().is_ok() { - if input.peek(syn::LitStr) { - let litstr = input.parse::()?; - return Ok(BindgenAttr::$variant(attr_span, Some(litstr.value()))) - } - - let ident = input.parse::()?.0; - return Ok(BindgenAttr::$variant(attr_span, Some(ident.to_string()))) + let (val, span) = match input.parse::() { + Ok(str) => (str.value(), str.span()), + Err(_) => { + let ident = input.parse::()?.0; + (ident.to_string(), ident.span()) + } + }; + return Ok(BindgenAttr::$variant(attr_span, Some(val), span)) } else { - return Ok(BindgenAttr::$variant(attr_span, None)); + return Ok(BindgenAttr::$variant(attr_span, None, attr_span)); } }); @@ -1004,39 +1019,47 @@ fn function_from_decl( syn::ReturnType::Type(_, ty) => Some(replace_self(*ty)), }; - let (name, name_span, renamed_via_js_name) = - if let Some((js_name, js_name_span)) = opts.js_name() { - let kind = operation_kind(opts); - let prefix = match kind { - OperationKind::Setter(_) => "set_", - _ => "", - }; - let name = if prefix.is_empty() - && opts.method().is_none() - && is_js_keyword(js_name, skip_keywords) - { - format!("_{}", js_name) + let kind = operation_kind(opts); + let attr_name = match kind { + OperationKind::Getter => { + if let Some((Some(name), span)) = opts.getter() { + Some((name, span)) } else { - format!("{}{}", prefix, js_name) - }; - (name, js_name_span, true) - } else { - let name = if !is_from_impl - && opts.method().is_none() - && is_js_keyword(&decl_name.to_string(), skip_keywords) - { - format!("_{}", decl_name.unraw()) + opts.js_name() + } + } + OperationKind::Setter => { + if let Some((Some(name), span)) = opts.setter() { + Some((name, span)) } else { - decl_name.unraw().to_string() - }; - (name, decl_name.span(), false) + opts.js_name() + } + } + _ => opts.js_name(), + }; + + let (mut name, name_span) = if let Some((js_name, js_name_span)) = attr_name { + let prefix = match kind { + OperationKind::Setter => "set_", + _ => "", }; + (format!("{}{}", prefix, js_name), js_name_span) + } else { + (decl_name.unraw().to_string(), decl_name.span()) + }; + + if !is_from_impl + && opts.method().is_none() + && is_js_keyword(&decl_name.to_string(), skip_keywords) + { + name = format!("_{}", name) + } + Ok(( ast::Function { arguments, name_span, name, - renamed_via_js_name, ret, rust_attrs: attrs, rust_vis: vis, @@ -1920,11 +1943,11 @@ pub fn check_unused_attrs(tokens: &mut TokenStream) { fn operation_kind(opts: &BindgenAttrs) -> ast::OperationKind { let mut operation_kind = ast::OperationKind::Regular; - if let Some(g) = opts.getter() { - operation_kind = ast::OperationKind::Getter(g.clone()); + if opts.getter().is_some() { + operation_kind = ast::OperationKind::Getter; } - if let Some(s) = opts.setter() { - operation_kind = ast::OperationKind::Setter(s.clone()); + if opts.setter().is_some() { + operation_kind = ast::OperationKind::Setter; } if opts.indexing_getter().is_some() { operation_kind = ast::OperationKind::IndexingGetter; From 7a74a494f17b503434c605eb5169b6b2a6bbfc4e Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 14 Nov 2024 15:18:26 +0100 Subject: [PATCH 4/7] Add test for imported getters and setters --- .../tests/reference/import-getter-setter.d.ts | 3 + .../tests/reference/import-getter-setter.js | 102 ++++++++++++++++++ .../tests/reference/import-getter-setter.rs | 59 ++++++++++ .../tests/reference/import-getter-setter.wat | 9 ++ 4 files changed, 173 insertions(+) create mode 100644 crates/cli/tests/reference/import-getter-setter.d.ts create mode 100644 crates/cli/tests/reference/import-getter-setter.js create mode 100644 crates/cli/tests/reference/import-getter-setter.rs create mode 100644 crates/cli/tests/reference/import-getter-setter.wat diff --git a/crates/cli/tests/reference/import-getter-setter.d.ts b/crates/cli/tests/reference/import-getter-setter.d.ts new file mode 100644 index 00000000000..091dbcc51a6 --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.d.ts @@ -0,0 +1,3 @@ +/* tslint:disable */ +/* eslint-disable */ +export function exported(): void; diff --git a/crates/cli/tests/reference/import-getter-setter.js b/crates/cli/tests/reference/import-getter-setter.js new file mode 100644 index 00000000000..0f62dbdb2e0 --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.js @@ -0,0 +1,102 @@ +let wasm; +export function __wbg_set_wasm(val) { + wasm = val; +} + + +const heap = new Array(128).fill(undefined); + +heap.push(undefined, null, true, false); + +function getObject(idx) { return heap[idx]; } + +let heap_next = heap.length; + +function addHeapObject(obj) { + if (heap_next === heap.length) heap.push(heap.length + 1); + const idx = heap_next; + heap_next = heap[idx]; + + heap[idx] = obj; + return idx; +} + +function dropObject(idx) { + if (idx < 132) return; + heap[idx] = heap_next; + heap_next = idx; +} + +function takeObject(idx) { + const ret = getObject(idx); + dropObject(idx); + return ret; +} + +export function exported() { + wasm.exported(); +} + +export function __wbg_another_edf1002f8e41a5da(arg0) { + const ret = getObject(arg0).prop2; + return ret; +}; + +export function __wbg_b_f17802a22332a667(arg0) { + const ret = getObject(arg0).a; + return ret; +}; + +export function __wbg_bar2_84566b6bcf547b19() { + const ret = Bar.bar2(); + return ret; +}; + +export function __wbg_getfoo_eae0175044c62418() { + const ret = Bar.get_foo(); + return ret; +}; + +export function __wbg_new_d728785ba7e8df96() { + const ret = new SomeClass(); + return addHeapObject(ret); +}; + +export function __wbg_setanother_2d160a2b6600e990(arg0, arg1) { + getObject(arg0).prop2 = arg1 >>> 0; +}; + +export function __wbg_setb_40f909ae19a05c10(arg0, arg1) { + getObject(arg0).a = arg1 >>> 0; +}; + +export function __wbg_setbar2_c8b4a150c6accea2(arg0) { + Bar.set_bar2(arg0 >>> 0); +}; + +export function __wbg_setfoo_6c6b6af72f779234(arg0) { + Bar.set_foo(arg0 >>> 0); +}; + +export function __wbg_setsignal_d386d151e7775c3f(arg0, arg1) { + getObject(arg0).signal = arg1 >>> 0; +}; + +export function __wbg_setsomeprop_afbca63b5d0f4c92(arg0, arg1) { + getObject(arg0).some_prop = arg1 >>> 0; +}; + +export function __wbg_signal_b82e5486ce265c55(arg0) { + const ret = getObject(arg0).signal; + return ret; +}; + +export function __wbg_someprop_26178791e2719528(arg0) { + const ret = getObject(arg0).some_prop; + return ret; +}; + +export function __wbindgen_object_drop_ref(arg0) { + takeObject(arg0); +}; + diff --git a/crates/cli/tests/reference/import-getter-setter.rs b/crates/cli/tests/reference/import-getter-setter.rs new file mode 100644 index 00000000000..413616b7600 --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.rs @@ -0,0 +1,59 @@ +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(js_namespace = Bar, getter = bar)] + fn get_foo() -> u32; + #[wasm_bindgen(js_namespace = Bar, setter = bar)] + fn set_foo(value: u32); + #[wasm_bindgen(js_namespace = Bar, getter, js_name = bar2)] + fn get_foo2() -> u32; + #[wasm_bindgen(js_namespace = Bar, setter, js_name = bar2)] + fn set_foo2(value: u32); + + #[wasm_bindgen] + #[derive(Debug, Clone, PartialEq)] + pub type SomeClass; + + #[wasm_bindgen(method, getter, js_class = "SomeClass", js_name = signal)] + pub fn signal(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter, js_class = "SomeClass", js_name = signal)] + pub fn set_signal(this: &SomeClass, value: u32); + + #[wasm_bindgen(method, getter, js_class = "SomeClass")] + pub fn some_prop(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter, js_class = "SomeClass")] + pub fn set_some_prop(this: &SomeClass, value: u32); + + #[wasm_bindgen(method, getter = prop2, js_class = "SomeClass")] + pub fn another(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter = prop2, js_class = "SomeClass")] + pub fn set_another(this: &SomeClass, value: u32); + + // #[wasm_bindgen(getter, js_class = "SomeClass")] + // pub fn static_controller() -> SomeClass; + // #[wasm_bindgen(getter, js_class = "SomeClass")] + // pub fn set_static_controller(value: &SomeClass); + + #[wasm_bindgen(constructor, js_class = "SomeClass")] + pub fn new() -> SomeClass; + + // js_name conflicts with the getter/setter name + #[wasm_bindgen(method, getter = a, js_class = "SomeClass", js_name = b)] + pub fn c(this: &SomeClass) -> u32; + #[wasm_bindgen(method, setter = a, js_class = "SomeClass", js_name = b)] + pub fn set_c(this: &SomeClass, value: u32); +} + +#[wasm_bindgen] +pub fn exported() { + set_foo(get_foo()); + set_foo2(get_foo2()); + + let a = SomeClass::new(); + a.set_signal(a.signal()); + a.set_some_prop(a.some_prop()); + a.set_another(a.another()); + a.set_c(a.c()); + // let _ = static_signal(); +} diff --git a/crates/cli/tests/reference/import-getter-setter.wat b/crates/cli/tests/reference/import-getter-setter.wat new file mode 100644 index 00000000000..e05a3a8c0ac --- /dev/null +++ b/crates/cli/tests/reference/import-getter-setter.wat @@ -0,0 +1,9 @@ +(module $reference_test.wasm + (type (;0;) (func)) + (func $exported (;0;) (type 0)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "exported" (func $exported)) + (@custom "target_features" (after code) "\04+\0amultivalue+\0fmutable-globals+\0freference-types+\08sign-ext") +) + From 4af8f8a9d00f3390859ae7bd3ff5f19b2ba2d594 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 14 Nov 2024 21:08:47 +0100 Subject: [PATCH 5/7] New hashes --- .../tests/reference/import-getter-setter.js | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/cli/tests/reference/import-getter-setter.js b/crates/cli/tests/reference/import-getter-setter.js index e03646fc8b6..206215b449e 100644 --- a/crates/cli/tests/reference/import-getter-setter.js +++ b/crates/cli/tests/reference/import-getter-setter.js @@ -37,61 +37,61 @@ export function exported() { wasm.exported(); } -export function __wbg_a_f17802a22332a667(arg0) { +export function __wbg_a_266c81b129cbc216(arg0) { const ret = getObject(arg0).a; return ret; }; -export function __wbg_bar2_84566b6bcf547b19() { +export function __wbg_bar2_38c86771c0e03476() { const ret = Foo.bar2(); return ret; }; -export function __wbg_bar_eae0175044c62418() { +export function __wbg_bar_690459206923b526() { const ret = Foo.bar(); return ret; }; -export function __wbg_new_d728785ba7e8df96() { +export function __wbg_new_98ff9abc2a3e2736() { const ret = new SomeClass(); return addHeapObject(ret); }; -export function __wbg_prop2_edf1002f8e41a5da(arg0) { +export function __wbg_prop2_79dcbfe47962d7a7(arg0) { const ret = getObject(arg0).prop2; return ret; }; -export function __wbg_seta_40f909ae19a05c10(arg0, arg1) { +export function __wbg_seta_eda0c18669c4ad53(arg0, arg1) { getObject(arg0).a = arg1 >>> 0; }; -export function __wbg_setbar2_c8b4a150c6accea2(arg0) { +export function __wbg_setbar2_d99cb80edd0e1959(arg0) { Foo.set_bar2(arg0 >>> 0); }; -export function __wbg_setbar_6c6b6af72f779234(arg0) { +export function __wbg_setbar_029452b4d4645d79(arg0) { Foo.set_bar(arg0 >>> 0); }; -export function __wbg_setprop2_2d160a2b6600e990(arg0, arg1) { +export function __wbg_setprop2_51e596d4d035bc4d(arg0, arg1) { getObject(arg0).prop2 = arg1 >>> 0; }; -export function __wbg_setsignal_d386d151e7775c3f(arg0, arg1) { +export function __wbg_setsignal_bd536e517c35da41(arg0, arg1) { getObject(arg0).signal = arg1 >>> 0; }; -export function __wbg_setsomeprop_afbca63b5d0f4c92(arg0, arg1) { +export function __wbg_setsomeprop_965004b0138eb32c(arg0, arg1) { getObject(arg0).some_prop = arg1 >>> 0; }; -export function __wbg_signal_b82e5486ce265c55(arg0) { +export function __wbg_signal_89fe6c5b19fec3df(arg0) { const ret = getObject(arg0).signal; return ret; }; -export function __wbg_someprop_26178791e2719528(arg0) { +export function __wbg_someprop_fd4fc05f44bf5de2(arg0) { const ret = getObject(arg0).some_prop; return ret; }; From d130122dbe3c071bdb8c49a51c67a3e5b01b1f04 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 14 Nov 2024 21:59:17 +0100 Subject: [PATCH 6/7] Updated schema hash --- 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 ebc7341bb3c..899eea8529c 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 = "211103844299778814"; +const APPROVED_SCHEMA_FILE_HASH: &str = "10652529958333964148"; #[test] fn schema_version() { From c7fb7992148c3981d1a3309769cff05d3db5d034 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 14 Nov 2024 22:06:02 +0100 Subject: [PATCH 7/7] Fixed error not being emitted and removed unused code --- crates/backend/src/ast.rs | 24 ------------------------ crates/macro-support/src/parser.rs | 6 +++++- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/crates/backend/src/ast.rs b/crates/backend/src/ast.rs index 36b54fac062..d4b30ded445 100644 --- a/crates/backend/src/ast.rs +++ b/crates/backend/src/ast.rs @@ -542,27 +542,3 @@ impl ImportKind { } } } - -impl Function { - /// If the rust object has a `fn xxx(&self) -> MyType` method, get the name for a getter in - /// javascript (in this case `xxx`, so you can write `val = obj.xxx`) - pub fn infer_getter_property(&self) -> &str { - &self.name - } - - /// If the rust object has a `fn set_xxx(&mut self, MyType)` style method, get the name - /// for a setter in javascript (in this case `xxx`, so you can write `obj.xxx = val`) - pub fn infer_setter_property(&self) -> Result { - let name = self.name.to_string(); - - // Otherwise we infer names based on the Rust function name. - if !name.starts_with("set_") { - bail_span!( - syn::token::Pub(self.name_span), - "setters must start with `set_`, found: {}", - name, - ); - } - Ok(name[4..].to_string()) - } -} diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index 9172cec6352..e0bcc8a726e 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -1045,7 +1045,11 @@ fn function_from_decl( }; (format!("{}{}", prefix, js_name), js_name_span) } else { - (decl_name.unraw().to_string(), decl_name.span()) + let name = decl_name.unraw().to_string(); + if matches!(kind, OperationKind::Setter) && !name.starts_with("set_") { + bail_span!(decl_name, "setters must start with `set_`, found: {}", name); + } + (name, decl_name.span()) }; if !is_from_impl