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

Hygiene break in macros involving string containing single quote #50061

Closed
dtolnay opened this issue Apr 18, 2018 · 7 comments
Closed

Hygiene break in macros involving string containing single quote #50061

dtolnay opened this issue Apr 18, 2018 · 7 comments
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)

Comments

@dtolnay
Copy link
Member

dtolnay commented Apr 18, 2018

This is minimized from the dramatic forum thread The latest nightly compiler has broken macros. The following code is fine in nightly-2018-04-12 but breaks with nightly-2018-04-15. Oddly, lots of minor perturbations make it succeed. For example remove the two @ characters and it works. Or remove the single quote in the string and it works.

#![feature(proc_macro, generators)]

extern crate futures_await as futures;
use futures::prelude::*;

macro_rules! j {
    (@ $v:tt) => {
        let _ = $v;
    };

    (# $v:tt) => {
        j!(@ $v);
    };
}

#[async]
fn test() -> Result<(), ()> {
    let s = "12'3";
    j!(# s);
    Ok(())
}
$ cargo +nightly-2018-04-12 build
   Compiling l v0.1.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.17 secs
$ cargo +nightly-2018-04-15 build
   Compiling l v0.1.0
error[E0425]: cannot find value `s` in this scope
 --> src/lib.rs:8:17
  |
8 |         let _ = $v;
  |                 ^^ not found in this scope

error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.

@alexcrichton

@dtolnay dtolnay added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label Apr 18, 2018
@alexcrichton
Copy link
Member

Alrighty-roo. As everything these days with hygiene, there's a couple of bugs here. With this one though I think only two!

Everything has to do here with #43081 basically. AKA anything that runs through this block of code that falls off this happy path

Why does this function not fit on the happy path?

The usual reasons we think you fall off the "happy path" are due to things like inner attributes, #[cfg], macro expansion, etc. None of that's present here! It should be the case that fn test above can be losslessly converted to a TokenStream based on what we've cached.

Turns out though #49852 had a bug in it (surprise surprise!). There we're using eq_unspanned to compare the resulting two token streams which eventually bottoms out in PartialEq for Token. As it turns out this isn't quite what we want! The PartialEq implementation is structural, but the token stream we get when stringifying is slightly different than what we parsed.

Here in the example you're using the literal string "12'3", but rustc stringifies this token as "12\'3". Similarly if you replaced the string with something like 0xf it'll be stringified as 15. This means that the tokens are not literally byte-for-byte equal, which means we fall off the happy path!

tl;dr the token stream of "12'3" is not equivalent to the token stream "12\'3". This causes us to fall off the happy path and used a stringified token stream.

Recent regression in hygiene information

OK so in the case above we're falling off the happy path when we didn't expect to, but even still there's some weird bad hygiene behavior when we're not on the happy path. There's still something to fix!

It turns out that there's also a regression due to #49154 (found via bisection). Before that PR this example would compile just fine, afterwards though it's hitting this error.

@petrochenkov can you help diagnose this and see what's going on?

The reduced test case I have right now is:

// foo.rs
#![crate_type = "proc-macro"]
#![feature(proc_macro)]

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn foo(_a: TokenStream, b: TokenStream) -> TokenStream {
    b.into_iter().collect()
}
// bar.rs
#![feature(proc_macro)]

extern crate foo;

use foo::*;

macro_rules! j {
    (@ $v:tt) => {
        let _ = $v;
    };

    (# $v:tt) => {
        j!(@ $v);
    };
}

#[foo]
fn main() {
    //! test
    let s = "12'3";
    j!(# s);
}

(note the inner attribute on main to force using a non-cached token stream)

$ rustc +nightly foo.rs
$ rustc +nightly bar.rs -L .
error[E0425]: cannot find value `s` in this scope
 --> bar.rs:9:17
  |
9 |         let _ = $v;
  |                 ^^ not found in this scope

error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 19, 2018
Discovered in rust-lang#50061 we're falling off the "happy path" of using a stringified
token stream more often than we should. This was due to the fact that a
user-written token like `0xf` is equality-different from the stringified token
of `15` (despite being semantically equivalent).

This patch updates the call to `eq_unspanned` with an even more awful solution,
`probably_equal_for_proc_macro`, which ignores the value of each token and
basically only compares the structure of the token stream, assuming that the AST
doesn't change just one token at a time.

While this is a step towards fixing rust-lang#50061 there is still one regression
from rust-lang#49154 which needs to be fixed.
@alexcrichton
Copy link
Member

I've posted a fix for the "happy path" at https://github.com/rust-lang/rust/pull/50069which will actually fix @dtolnay's example above. That PR does not, however, fix the regression from #49154 which is still reproducible with the example I gisted.

@dtolnay
Copy link
Member Author

dtolnay commented Apr 19, 2018

Any idea why my repro case above is fixed if you remove the @ tokens? It seems like fn test should fall off the happy path just as much.

  macro_rules! j {
-     (@ $v:tt) => {
+     ($v:tt) => {
          let _ = $v;
      };

      (# $v:tt) => {
-         j!(@ $v);
+         j!($v);
      };
  }

@alexcrichton
Copy link
Member

If you remove the @ tokens we're still on the stringification path (aka not the happy path). For whatever reason though the hygiene bug goes away. That's likely caused from a subtelty introduced in #49154 (or at least so I'm hoping). I don't know much about #49154, so I'm hoping @petrochenkov will be able to look at this and know what's going on!

@petrochenkov
Copy link
Contributor

I don't know much about #49154, so I'm hoping @petrochenkov will be able to look at this and know what's going on!

I'll investigate.

@petrochenkov petrochenkov self-assigned this Apr 19, 2018
bors added a commit that referenced this issue Apr 20, 2018
proc_macro: Stay on the "use the cache" path more

Discovered in #50061 we're falling off the "happy path" of using a stringified
token stream more often than we should. This was due to the fact that a
user-written token like `0xf` is equality-different from the stringified token
of `15` (despite being semantically equivalent).

This patch updates the call to `eq_unspanned` with an even more awful solution,
`probably_equal_for_proc_macro`, which ignores the value of each token and
basically only compares the structure of the token stream, assuming that the AST
doesn't change just one token at a time.

While this is a step towards fixing #50061 there is still one regression
from #49154 which needs to be fixed.
@petrochenkov
Copy link
Contributor

e2afefd is the commit causing the regression.
Further investigation is in progress.

@SergioBenitez
Copy link
Contributor

SergioBenitez commented May 16, 2018

An issue recently popped up on Rocket (rwf2/Rocket#635) that appears to be identical to this one and is not resolved in any recent nightly. It's possible that the root cause is #50050, however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)
Projects
None yet
Development

No branches or pull requests

4 participants