regression 1.49: macro_rules unexpected tokens

[Mark-Simulacrum]

Unclear on the extent to which these are the same problem:

• https://crater-reports.s3.amazonaws.com/beta-1.49-1/beta-2020-11-26/gh/ErickHdez96.tiger/log.txt
• https://crater-reports.s3.amazonaws.com/beta-1.49-1/beta-2020-11-26/reg/fourier-0.1.0/log.txt

cc @petrochenkov @Aaron1011

[Aaron1011]

The fourier error looks like an old bug that was fixed by calebzulawski/fourier#17

[Mark-Simulacrum]

https://crates.io/crates/fourier only lists 0.1.0 so we should probably nag @calebzulawski about a point release one more time :)

[calebzulawski]

Whoops, looks like I half-fixed it. One of my dependencies is too strict. It does look like the same error so probably not a regression...

[Mark-Simulacrum]

cc @petrochenkov

[camelid]

@apiraino and I both feel unclear about this: Is this a Rust regression or a regression in a library that was then picked up by Crater?

[Mark-Simulacrum]

This is a regression in the Rust compiler.

[camelid]

The tiger error seems to me like fallout from implementing inline_const. I'm guessing the issue is that it's expecting a block but immediately gets a literal. Not sure how this is supposed to behave though.

---

Reposting the error here:

[INFO] [stdout] error: expected expression, found keyword `const`
[INFO] [stdout]    --> src/ir.rs:481:23
[INFO] [stdout]     |
[INFO] [stdout] 347 |     (exp $e:expr) => {
[INFO] [stdout]     |          ------- while parsing argument for this `expr` macro fragment
[INFO] [stdout] ...
[INFO] [stdout] 481 |             stmt!(exp const 1);
[INFO] [stdout]     |                       ^^^^^ expected expression
[INFO] [stdout] 
[INFO] [stdout] 
[INFO] [stdout] error: expected expression, found keyword `const`
[INFO] [stdout]    --> src/ir.rs:482:23
[INFO] [stdout]     |
[INFO] [stdout] 347 |     (exp $e:expr) => {
[INFO] [stdout]     |          ------- while parsing argument for this `expr` macro fragment
[INFO] [stdout] ...
[INFO] [stdout] 482 |             stmt!(exp const 2);
[INFO] [stdout]     |                       ^^^^^ expected expression
[INFO] [stdout] 
[INFO] [stdout] 
[INFO] [stdout] error: expected expression, found keyword `const`
[INFO] [stdout]    --> src/ir.rs:483:23
[INFO] [stdout]     |
[INFO] [stdout] 347 |     (exp $e:expr) => {
[INFO] [stdout]     |          ------- while parsing argument for this `expr` macro fragment
[INFO] [stdout] ...
[INFO] [stdout] 483 |             stmt!(exp const 3);
[INFO] [stdout]     |                       ^^^^^ expected expression
[INFO] [stdout] 
[INFO] [stdout] 
[INFO] [stdout] error: aborting due to 3 previous errors

[camelid]

But I'm not quite sure; the span seems potentially wrong.

[spastorino]

@rustbot ping cleanup

would be nice to get an MCVE

[rustbot]

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @JamesPatrickGill @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @shekohex @sinato @smmalis37 @steffahn @Stupremee @tamuhey @turboladen @woshilapin @yerke

[camelid]

Somewhat minimized (playground):

macro_rules! exp {
    (const $n:expr) => {
        Box::new(Exp::Const($n))
    };
}

macro_rules! stmt {
    (exp $e:expr) => {
        Box::new(Stmt::Exp($e))
    };
    (exp $($t:tt)+) => {
        Box::new(Stmt::Exp(exp!($($t)+)))
    };
}

macro_rules! seq {
    ($stmt1:expr; $stmt2:expr $(;)?) => {
        Box::new(Stmt::Seq($stmt1, $stmt2))
    };
    ($stmt1:expr; $stmt2:expr; $($stmt:expr);+ $(;)?) => {
        seq!(seq!($stmt1; $stmt2); $($stmt);+)
    };
}

fn main() {
    let seq = seq!(
        stmt!(exp const 1);
        stmt!(exp const 2);
        stmt!(exp const 3);
    );
}

On stable, it produces a bunch of semantic errors (which make sense since I removed a lot of type definitions), but on beta it produces:

error: expected expression, found keyword `const`
  --> src/main.rs:99:19
   |
47 |     (exp $e:expr) => {
   |          ------- while parsing argument for this `expr` macro fragment
...
99 |         stmt!(exp const 1);
   |                   ^^^^^ expected expression

error: expected expression, found keyword `const`
   --> src/main.rs:100:19
    |
47  |     (exp $e:expr) => {
    |          ------- while parsing argument for this `expr` macro fragment
...
100 |         stmt!(exp const 2);
    |                   ^^^^^ expected expression

error: expected expression, found keyword `const`
   --> src/main.rs:101:19
    |
47  |     (exp $e:expr) => {
    |          ------- while parsing argument for this `expr` macro fragment
...
101 |         stmt!(exp const 3);
    |                   ^^^^^ expected expression

[spastorino]

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

[camelid]

Much more minimized:

macro_rules! exp {
    (const $n:expr) => {
        $n
    };
}

macro_rules! stmt {
    (exp $e:expr) => {
        $e
    };
    (exp $($t:tt)+) => {
        exp!($($t)+)
    };
}

fn main() {
    stmt!(exp const 1);
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error: expected expression, found keyword `const`
  --> src/main.rs:17:15
   |
8  |     (exp $e:expr) => {
   |          ------- while parsing argument for this `expr` macro fragment
...
17 |     stmt!(exp const 1);
   |               ^^^^^ expected expression

warning: unused macro definition
 --> src/main.rs:1:1
  |
1 | / macro_rules! exp {
2 | |     (const $n:expr) => {
3 | |         $n
4 | |     };
5 | | }
  | |_^
  |
  = note: `#[warn(unused_macros)]` on by default

error: aborting due to previous error; 1 warning emitted

error: could not compile `playground`

To learn more, run the command again with --verbose.

[camelid]

Here's the code again:

macro_rules! exp {
    (const $n:expr) => {
        $n
    };
}

macro_rules! stmt {
    (exp $e:expr) => {
        $e
    };
    (exp $($t:tt)+) => {
        exp!($($t)+)
    };
}

fn main() {
    stmt!(exp const 1);
}

I think the issue is that on beta the compiler thinks the const in the macro invocation is starting an expression because of the new inline_const feature – thus it thinks it's using the first rule in the macro. But then when doing macro expansion, it realizes that it's actually not a valid expression because it's missing braces.

On stable this is not an issue because the compiler never thinks that const is starting an expression.

I think the fix might be to not treat const as the start of an expression if it is not immediately followed by braces.

---

See the original inline_const PR: #77124.

cc @spastorino who has been implementing inline_const

[camelid]

My bisection is in line with my diagnosis:

searched nightlies: from nightly-2020-10-15 to nightly-2020-10-19
regressed nightly: nightly-2020-10-18
searched commits: from e3051d8 to 043eca7
regressed commit: 6af9846

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-apple-darwin
Reproduce with:

cargo bisect-rustc --preserve --regress=error --start=2020-10-15 --end=2020-10-19 

[camelid]

I'm going to try working on this. If I get stuck I'll release my assignment since this is P-high!

[camelid]

So I'm trying to figure out which code is the culprit here. The inline_const PR is the one that introduced the regression, but looking at its changes I don't know if it's actual the underlying cause. I think the issue might be in the macro expansion code, but I'm not familiar with it at all.

Since this issue is P-critical and potentially a release blocker, I'm going to release my assignment so that someone more experienced can take it over. Though I'm happy to help if there's some way to do that!

[camelid]

Actually, I just noticed that petrochenkov responded to my message asking for help finding the code 😄

[camelid]

This should be fixed on the next nightly.

[Mark-Simulacrum]

Reopening to track beta fix.

[Mark-Simulacrum]

Should be backported to beta as part of #80417.

[camelid]

I think this can be closed as #80417 has been merged?