Macro hygiene bug

[jseyfried]

The following should print 1, but it currently prints 0:

fn main() {
    let x = 0;
    macro_rules! foo { () => {
        let x = 1;
        macro_rules! bar { () => {x} }
        println!("{}", bar!());
    }}

    foo! {};
}

For comparison, the following correctly prints 1:

fn main() {
    let x = 0;
    macro_rules! foo { () => {{ //< note the added brace here
        let x = 1;
        macro_rules! bar { () => {x} }
        println!("{}", bar!());
    }}}

    foo! {};
}

[jseyfried]

cc @nrc

[jseyfried]

Another example -- the following should print 1 but currently does not compile:

macro_rules! foo { () => {
    let x = 1;
    macro_rules! bar { () => {x} }
    println!("{}", bar!());
}}

fn main() {
    foo! {};
}

Either adding another pair of braces (as in the first initial comment) or manually expanding the macro allows this example to compile.

[durka]

This is probably a special case of hygiene not really working at all in multi-statement macros, #31856.

[jseyfried]

@durka indeed it is. That issue is also fixed by #32923.

[durka]

Amazing, thanks! (Should we check crater for breakage, though?)

[jseyfried]

I think it's unlikely that there will be breakage, but it would probably be a good idea to check anyway, especially since silent changes in behavior are scary.

[durka]

Well crater doesn't catch those, so we just have to hope that nobody was actually using multi-statement macros...

[jseyfried]

True, but if there's no breakage I believe that's pretty strong evidence that there won't be any behavior changes since I would think breakage (either due to there not being a shadowing binding in the macro expansion or due to the shadowing binding being of the wrong type) would be much more likely than a behavior change (in which there would need to be a shadowing binding of the same type).

[jseyfried]

Also, does crater run tests?

[bluss]

Right, crater does not normally run tests. So macros not used/expanded in a crate somewhere are entirely unexercised by crater.

[durka]

Does it build the tests? If not perhaps it builds bins, and we could "test" macro crates that way?

[durka]

It seems that crater simply runs cargo build. As an experiment I just hacked up this crate so the integration tests are configured as no-op bins, such that cargo build fails if the tests fail to build. It's pretty ugly though and is obviously opt-in.