Parse bang macro as a statement when used in trailing expr position

[Aaron1011]

cc #33953

Currently, the following code produces an error

fn main() {
    macro_rules! a {
        ($e:expr) => { $e; }
    }
    a!(true)
}

With this change, it now compiles, since we parse a!(true) as a
statement.

[rust-highfive]

r? @ecstatic-morse

(rust_highfive has picked a reviewer for you, use r? to override)

[Aaron1011]

@bors try

[bors]

⌛ Trying commit 6a5caac with merge 8b72f65b805cbe4c261de45030d663f524f27376...

[bors]

☀️ Try build successful - checks-actions
Build commit: 8b72f65b805cbe4c261de45030d663f524f27376 (8b72f65b805cbe4c261de45030d663f524f27376)

[Aaron1011]

@craterbot check

[craterbot]

👌 Experiment pr-78991 created and queued.
🤖 Automatically detected try build 8b72f65b805cbe4c261de45030d663f524f27376
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-78991 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-78991 is completed!
📊 6590 regressed and 4 fixed (130032 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[Aaron1011]

There's a large number of legitimate regressions, due to some trailing macro expressions now evaluating to ().

It should be possible to make this a warning - we can continue to parse trailing macro calls as expressions, but add a flag that makes us emit a warning during typecheck if the type of the expression is not ()

However, it might be worthwhile to check with the lang team to make sure this is something that we want to do.

[petrochenkov]

r? @petrochenkov

[Aaron1011]

On further investigation, many (if not all) of the regressions appear to be due to an interaction with semicolons in macro bodies (see #78685)

If we have a macro like:

macro_rules! a {
    ($e:expr) => { $e; }
}

we ignore the trailing semicolon if the macro is used in expression position:

rust/compiler/rustc_expand/src/mbe/macro_rules.rs

Lines 151 to 156 in c919f49

	// We allow semicolons at the end of expressions -- e.g., the semicolon in
	// `macro_rules! m { () => { panic!(); } }` isn't parsed by `.parse_expr()`,
	// but `m!()` is allowed in expression positions (cf. issue #34706).
	if kind == AstFragmentKind::Expr && parser.token == token::Semi {
	parser.bump();
	}

With this change, we now parse the macro body in statement position, so the above code is never run.

To make this into a warning, we would want to extend the above workaround to apply to statements, but only when the original macro call occurred as the trailing statement in a block.

[petrochenkov]

To make this into a warning, we would want to extend the above workaround to apply to statements, but only when the original macro call occurred as the trailing statement in a block.

Yes, I think it would be a good strategy for the last statement of expanded code to inherit "expr-ness" from the macro call for the purpose of this hack.

I don't have any other specific ideas right now, but I think the general guiding principle here is that expansion should be built and work using a holistic token-based model as if we didn't have any compatibility restrictions. Then minimal compatibility hack would be applied on top of that model in such way that they could report warnings or be turned off at edition boundary.

[crlf0710]

@Aaron1011 Ping from triage: Any updates on this?

[Aaron1011]

I'll revisit this after #79819 is merged

[Dylan-DPC]

Closing this as inactive