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

Pass value_changed to macro_rules! macros #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Aaron1011
Copy link

@Aaron1011 Aaron1011 commented May 31, 2020

In Rust, variable identifiers in a macro_rules! body are resolved
in the scope of tht body - they cannot see through to the caller's body.
For example, the following code errors:

macro_rules! weird_ident {
    () => { my_ident; }
}

fn main() {
    let my_ident = 1;
    weird_ident!();
}

However, due to a compiler bug (rust-lang/rust#43081),
this kind of code current compiles when a procedural macro is invoked by
a macro_rules! macro. Eventually, this will cause a compilation error.

In the color! and slider! macro, you're currently writing an expression
involving value_changed, and passing it to a procedural macro (html!).
However, value_changed comes from the body of the caller of color!/slider!,
so this code will stop compiling once the compiler bug is fixed.

Fortunately, the identifier of value_changed can be passed into
color! and slider!. This modified code will with the current version of
Rust, as well as future version that causes the old code into an error.

I also ran cargo update to bump the dependencies in your Cargo.lock.
This will allow your crate to continue to compile when the compiler bug is fixed,
as you are depending on some crates (e.g. syn) that have released updates
to address the issue.

Feel free to ask me about any questions you may have. For more details,
see rust-lang/rust#72622

In Rust, variable identifiers in a `macro_rules!` body are resolved
in the scope of tht body - they cannot see through to the caller's body.
For example, the following code errors:

`rust
macro_rules! weird_ident {
    () => { my_ident; }
}

fn main() {
    let my_ident = 1;
    weird_ident!();
}
`

However, due to a compiler bug (rust-lang/rust#43081),
this kind of code current compiles when a procedural macro is invoked by
a `macro_rules!` macro. Eventually, this will cause a compilation error.

In the `color!` and `slider!` macro, you're currently writing an expression
involving `value_changed`, and passing it to a procedural macro (`html!`).
However, `value_changed` comes from the body of the caller of `color!`/`slider!`,
so this code will stop compiling once the compiler bug is fixed.

Fortunately, the identifier of `value_changed` can be passed into
`color!` and `slider!`. This modified code will with the current version of
Rust, as well as future version that causes the old code into an error.

I also ran `cargo update` to bump the dependencies in your Cargo.lock.
This will allow your crate to continue to compile when the compiler bug is fixed,
as you are depending on some crates (e.g. `syn`) that have released updates
to address the issue.

Feel free to ask me about any questions you may have. For more details,
see rust-lang/rust#72622
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 23, 2020
…d, r=petrochenkov

Re-land PR rust-lang#72388:  Recursively expand `TokenKind::Interpolated` in `probably_equal_for_proc_macro`

PR rust-lang#72388 allowed us to preserve the original `TokenStream` in more cases during proc-macro expansion, but had to be reverted due to a large number of regressions (See rust-lang#72545 and rust-lang#72622). These regressions fell into two categories

1. Missing handling for `Group`s with `Delimiter::None`, which are inserted during `macro_rules!` expansion (but are lost during stringification and re-parsing). A large number of these regressions were due to `syn` and `proc-macro-hack`, but several crates needed changes to their own proc-macro code.
2. Legitimate hygiene issues that were previously being masked by stringification. Some of these were relatively benign (e.g. [a compiliation error](paritytech/parity-scale-codec#210) caused by misusing `quote_spanned!`). However, two crates had intentionally written unhygenic `macro_rules!` macros, which were able to access identifiers that were not passed as arguments (see rust-lang#72622 (comment)).

All but one of the Crater regressions have now been fixed upstream (see https://hackmd.io/ItrXWRaSSquVwoJATPx3PQ?both). The remaining crate (which has a PR pending at sammhicks/face-generator#1) is not on `crates.io`, and is a Yew application that seems unlikely to have any reverse dependencies.

As @petrochenkov mentioned in rust-lang#72545 (comment), not re-landing PR rust-lang#72388 allows more crates to write unhygenic `macro_rules!` macros, which will eventually stop compiling. Since there is only one Crater regression remaining, since additional crates could write unhygenic `macro_rules!` macros in the time it takes that PR to be merged.
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.

1 participant