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

#[wasm_bindgen] attribute not work in macro_rules #3707

Closed
sshwy opened this issue Nov 15, 2023 · 6 comments · Fixed by #3725
Closed

#[wasm_bindgen] attribute not work in macro_rules #3707

sshwy opened this issue Nov 15, 2023 · 6 comments · Fixed by #3725
Labels

Comments

@sshwy
Copy link

sshwy commented Nov 15, 2023

Describe the Bug

When putting struct attributed #[wasm_bindgen] in a custom macro_rules, it throws error cannot find value 'js' in this scope.

Steps to Reproduce

wasm_bindgen version: 0.2.88.

code snippet:

// a macro that output exactly the same input struct
macro_rules! make_struct {
    {
        $( #[$attr:meta] )*
        $svis:vis struct $record_name:ident {
            $(
                $fvis:vis $fname:ident : $ftype:ty
            ),* $(,)?
        }
    } => {
        $( #[$attr] )*
        $svis struct $record_name {
            $(
                $fvis $fname : $ftype
            ),*
        }
    };
}

// testing
make_struct!{
    #[wasm_bindgen]
    #[derive(Debug, Clone, Copy)]
    pub struct Test {
        pub a: i32 // error: cannot find value `js` in this scope
    }
}

Expected Behavior

In my imagination, macro rules should be expanded before the proc macros, thus the above code should work as normal.

Correct me if I'm wrong.

@sshwy sshwy added the bug label Nov 15, 2023
@daxpedda
Copy link
Collaborator

This is very strange.
When I used cargo expand it seemed to output code that worked just fine, So I'm not sure what the problem here is.

@ChiaMineJP
Copy link

ChiaMineJP commented Nov 26, 2023

Hello, I'm also experiencing this issue.
I've created a simple reproducible repository here: https://github.com/ChiaMineJP/error-report/tree/main/wasm-bindgen-with-macro

Summary

I defined a simple macro rule as below, which just adds a line #[wasm-bindgen] to
a struct definition.

macro_rules! with_wasm {
    ($name:ident {$field:ident: $t:ty}) => {
        #[wasm_bindgen::prelude::wasm_bindgen]
        pub struct $name {
            pub $field: $t
        }
    }
}

This macro simply converts

with_wasm!(AStruct {
    a_field: u8
});

to

#[wasm_bindgen::prelude::wasm_bindgen]
pub struct AStruct {
    pub a_field: u8,
}

While the expanded code (the below) passes wasm-pack build without error,
the original code with macro (the above) fails with error:

error[E0425]: cannot find value `js` in this scope

Possible hint to solve this issue

When I replace pub $field: $t in the macro with pub a_field: $t like below, then wasm-pack build runs without error.

macro_rules! with_wasm {
    ($name:ident {$field:ident: $t:ty}) => {
        #[wasm_bindgen::prelude::wasm_bindgen]
        pub struct $name {
            // Previously `pub $field: $t`
            pub a_field: $t
        }
    }
}

This suggests something weird happenning on macro expansion. especially field name evaluation.

Possible hint to solve this issue 2

When I remove pub in the macro, wasm-pack build succeeded.

macro_rules! with_wasm {
    ($name:ident {$field:ident: $t:ty}) => {
        #[wasm_bindgen::prelude::wasm_bindgen]
        pub struct $name {
            // Previously `pub $field: $t`
            $field: $t
        }
    }
}

@sshwy
Copy link
Author

sshwy commented Nov 26, 2023

@ChiaMineJP thank you for your solutions, however they won't work for me :). I expect the field of the struct to be customizable, and exported to the js side, thus $field and pub are both required.

@ChiaMineJP
Copy link

Hi @sshwy
Sorry my comment was a bit confusing.
Possible hint to solve this issue was not a direct fix suggestion but just a bunch of helpful findings to locate the root cause of this issue.

For example, the fact that replacing $field with a fixed field name does not emit error suggests that #[wasm_bindgen] custom attribute does not recognize the field name which is output from the macro.

@ChiaMineJP
Copy link

ChiaMineJP commented Nov 26, 2023

Update:

I tried to compare token output of #[wasm_bindgen] by outputing it to a file as below.
https://github.com/rustwasm/wasm-bindgen/blob/main/crates/macro/src/lib.rs
Screenshot 2023-11-27 at 0 48 40

(I cloned this wasm_bindgen repository and use it as a path dependency like this)
Screenshot 2023-11-27 at 0 58 04

As I wrote in my comment above, replacing $field with a_field (fixed field name) does not raise an error.
I wanted to know whether token output by #[wasm_bindgen] is different with $field or a_field.
The result was the same and even when the formar complained error(cannot find value js in this scope), wasm_bindgen custom attribute itself returns Ok(tokens).

This suggests that there might be a bug in rust compiler and not in wasm_bindgen.

@ChiaMineJP
Copy link

@sshwy
I've come up with a violent solution.

The idea is to let some custom attribute add #[wasm_bindgen] to target struct.
In my environment this strangely works. Hope it works well for you.

Please clone my repository to try my solution.
https://github.com/ChiaMineJP/error-report/tree/main/wasm-bindgen-with-macro#temporal-solution

Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this issue Nov 27, 2023
Fixes rustwasm#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.
Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this issue Nov 29, 2023
Fixes rustwasm#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants