Forbid $crate in macro patterns

[CAD97]

Fixes#99037. This currently implements a hard error because it was convenient to reuse the existing error for other disallowed metavars in the macro pattern, but should maybe be a forward-compat lint instead. Or maybe a clean crater run would justify just landing this directly as a hard error.

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[jackh726]

Not sure who the right reviewer is for this. Likely not me.

r? rust-lang/compiler

[fee1-dead]

r? rust-lang/compiler

[estebank]

The changes look reasonable to me, but 1) we should run crater and 2) @petrochenkov should take a look (it is a 3 line diff).

@bors try
@craterbot check

[craterbot]

🚨 Error: missing start toolchain

🆘 If you have any trouble with Crater please ping @rust-lang/infra!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[bors]

⌛ Trying commit 8bb7a85 with merge ff47dd48ed0d02534055e0033cf766ce528f8ebc...

[petrochenkov]

I think this is the right thing to do.

• $ crate certainly shouldn't turn into glued kw::DollarCrate in macro LHS, it only supposed to do that in macro output, i.e. in the results of the macro work after the macro is expanded.
• If $ any_arbitrary_ident was accepted in macro LHS and was interpreted as two tokens, then it would be fine to accept $ crate as two tokens too, but it's not accepted because it's considered "reserved" as a part of the $ name : kind syntax even if there's no : after it, so it's reasonable to reserve $ crate too.

[CAD97]

If the crater run comes back clean, I'd like to at least consider landing this as-is with the hard error. If it's not clean or that's too aggressive, I can switch this to a future-compatility lint.

Ultimately, I'd like if $crate went through the binder handling path as just a reserved binder name rather than eager gluing; this would also open up the door to other default binder names (e.g. I've previously proposed $mod or similar to get path-fragment-to-macro-definition-scope). Also, if there's a specific reason for expanding/gluing to kw::DollarCrate rather than a def-site crate (diagnostics? avoiding hygiene 2.0 reliance?) I didn't find it the last time I looked.

(Doing so has knock-on benefits to #99035/#99445 as well.)

That's only really a potential follow-up after we catch lhs $crate in the first place, though. (And perhaps the gluing/resolution behavior needs to stay on current editions and only change in future ones for compatibility.)

[petrochenkov]

Also, if there's a specific reason for expanding/gluing to kw::DollarCrate rather than a def-site crate

I think it was just impossible before #51952, and after than nobody tried, I guess.

diagnostics? avoiding hygiene 2.0 reliance?

Probably the former.
Also $crate is special-cased in pretty-printing because people try to compile pretty-printed code in practice.
Someone need to try it and see what happens.

[bors]

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

[estebank]

@craterbot check

[craterbot]

👌 Experiment pr-99447 created and queued.
🤖 Automatically detected try build ff47dd48ed0d02534055e0033cf766ce528f8ebc
🔍 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-99447 is now running

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

[craterbot]

🎉 Experiment pr-99447 is completed!
📊 987 regressed and 5 fixed (240453 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

[Mark-Simulacrum]

Cc rust-lang/crater#663, seems like whatever is causing that happened here too (not entirely unexpectedly)

[CAD97]

Of the crater-reported root regressions

Legitimate

crate	downloads	results	patch
combu	8202	build failure
ndarray	3668328	build failure	rust-ndarray/ndarray#1195

So this needs to be future incompatibility at worst.

Spurious

crate	opinion
gclark916.vulkano_examples	looks spurious (shader validation)
gclark916.vulkano_tri1	looks spurious (shader validation)
bitrust-0.1.0	Access Denied
ledger-transport-zemu-0.10.0	Access Denied
Ayrx.binja-rs-hello-world	binaryninjacore-sys; looks spurious
Ninja3047.load-symbols	binaryninjacore-sys; looks spurious
etke.bff	binaryninjacore-sys; looks spurious
knarkzel.katsu	looks spurious (unclear)
stuartZhang.scaffold-wizard	looks spurious (unclear)
catboost2-sys	spurious (missing rustfmt)
catboost2	catboost2-sys; spurious (missing rustfmt)
openexr-sys	spurious (C++ dependencies)

Those unlisted here are all rust-lang/crater#663.

[CAD97]

@rustbot author

I need to change this to a future-compat lint to not break ndarray

[Dylan-DPC]

@CAD97 any updates on this?

[jyn514]

I'm going to close this for now - feel free to re-open later if you have time to work on it.