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

Remove span in generated field getter #3725

Merged
merged 4 commits into from
Nov 29, 2023
Merged

Conversation

Liamolucko
Copy link
Collaborator

@Liamolucko Liamolucko commented Nov 28, 2023

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. Turns out it was used, so I just got rid of the span on js instead of the whole thing.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Very funny business.

@daxpedda
Copy link
Collaborator

Ah, could you add a note to the changelog as well?

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 Liamolucko merged commit def9147 into rustwasm:main Nov 29, 2023
25 checks passed
@Liamolucko Liamolucko deleted the fix-hygiene branch November 29, 2023 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[wasm_bindgen] attribute not work in macro_rules
2 participants