From d74450f6b06e7968e5eef176047d67293f85bf6f Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Tue, 28 Nov 2023 10:40:19 +1100 Subject: [PATCH 1/4] Remove span in generated field getter Fixes #3707 Previously, `(*js).borrow().#rust_name` was being given the span of `rust_name`, which seems like a fairly harmless thing to do at first glance. However, it turns out that the span of a token also affects its hygiene - turns out proc macros have hygiene too, not just declarative macros! This caused a problem because the declaration of `js` had a span of `Span::call_site()`, but it was being accessed with `rust_name`'s span, which might be a different context where `js` doesn't exist. Usually this is fine because `Span::call_site()` is in the same context as the struct definition: normal code. However, when you put `#[wasm_bindgen]` inside a `macro_rules!`, `Span::call_site()` is now in the context of that `macro_rules!`, while `rust_name`'s span is still in normal code which can't access variables from inside `macro_rules!`. To fix that I've just removed the span from `(*js).borrow().#rust_name`, making it `Span::call_site()`. I don't think this should have any effect on diagnostics anyway, since I don't see how that code could ever cause a compile error. --- crates/backend/src/codegen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index bfedcfbc275..b4fe2550065 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -389,7 +389,7 @@ impl ToTokens for ast::StructField { }; let maybe_assert_copy = respan(maybe_assert_copy, ty); - let mut val = quote_spanned!(self.rust_name.span()=> (*js).borrow().#rust_name); + let mut val = quote! { (*js).borrow().#rust_name }; if let Some(span) = self.getter_with_clone { val = quote_spanned!(span=> <#ty as Clone>::clone(&#val) ); } From 22343b17b1843c85a0eeb2e6d0b4f9671655c6c9 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Tue, 28 Nov 2023 11:12:27 +1100 Subject: [PATCH 2/4] Add test --- tests/wasm/classes.js | 7 +++++++ tests/wasm/classes.rs | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/tests/wasm/classes.js b/tests/wasm/classes.js index 8e503c65803..49bd4472210 100644 --- a/tests/wasm/classes.js +++ b/tests/wasm/classes.js @@ -241,3 +241,10 @@ exports.js_test_inspectable_classes_can_override_generated_methods = () => { assert.strictEqual(overridden_inspectable.toString(), 'string was overwritten'); overridden_inspectable.free(); }; + +exports.js_test_class_defined_in_macro = () => { + const macroClass = new wasm.InsideMacro(); + assert.strictEqual(macroClass.a, 3); + macroClass.a = 5; + assert.strictEqual(macroClass.a, 5); +}; diff --git a/tests/wasm/classes.rs b/tests/wasm/classes.rs index 9321ff92d7f..6058c0a59da 100644 --- a/tests/wasm/classes.rs +++ b/tests/wasm/classes.rs @@ -34,6 +34,7 @@ extern "C" { fn js_test_option_classes(); fn js_test_inspectable_classes(); fn js_test_inspectable_classes_can_override_generated_methods(); + fn js_test_class_defined_in_macro(); } #[wasm_bindgen_test] @@ -608,3 +609,27 @@ impl OverriddenInspectable { String::from("string was overwritten") } } + +macro_rules! make_struct { + ($field:ident) => { + #[wasm_bindgen] + pub struct InsideMacro { + pub $field: u32, + } + }; +} + +make_struct!(a); + +#[wasm_bindgen] +impl InsideMacro { + #[wasm_bindgen(constructor)] + pub fn new() -> Self { + Self { a: 3 } + } +} + +#[wasm_bindgen_test] +fn class_defined_in_macro() { + js_test_class_defined_in_macro(); +} From 3bed9d259f9ccb9d8b2c45062d2ad48740847f7a Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Tue, 28 Nov 2023 11:41:35 +1100 Subject: [PATCH 3/4] Only remove the span from `js` --- crates/backend/src/codegen.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index b4fe2550065..bbcbc9cea14 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -389,7 +389,14 @@ impl ToTokens for ast::StructField { }; let maybe_assert_copy = respan(maybe_assert_copy, ty); - let mut val = quote! { (*js).borrow().#rust_name }; + // Split this out so that it isn't affected by `quote_spanned!`. + // + // If we don't do this, it might end up being unable to reference `js` + // properly because it doesn't have the same span. + // + // See https://github.com/rustwasm/wasm-bindgen/pull/3725. + let js_token = quote! { js }; + let mut val = quote_spanned!(self.rust_name.span()=> (*#js_token).borrow().#rust_name); if let Some(span) = self.getter_with_clone { val = quote_spanned!(span=> <#ty as Clone>::clone(&#val) ); } From e63e36ee170c9d9c6b720dbbfd996208f21884eb Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Wed, 29 Nov 2023 20:08:18 +1100 Subject: [PATCH 4/4] Add changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9bc727c119..2a246ee6f2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ * Add bindings for `UserActivation`. [#3719](https://github.com/rustwasm/wasm-bindgen/pull/3719) +### Fixed + +* Fixed a compiler error when using `#[wasm_bindgen]` inside `macro_rules!`. + [#3725](https://github.com/rustwasm/wasm-bindgen/pull/3725) + ### Removed * Removed Gecko-only `InstallTriggerData` and Gecko-internal `FlexLineGrowthState`, `GridDeclaration`, `GridTrackState`,