missing_fragment_specifier hard error

[pietroalbini]

In the beta run for Rust 1.47 the release team discovered that #75516 is breaking more than 475 crates or git repositories currently compiling, across at least the following crates:

• js-sys
• clap
• time-macros-impl

cc @petrochenkov @matklad @rust-lang/release

[pietroalbini]

Personally I think that's too much breakage to land, even though it was a deprecation warning since 2017.

[petrochenkov]

I agree that it's not an acceptable breakage, although I suspect that most of it happens due to committed Cargo.locks.
I think we can add a hack accepting a missing fragment specifier for the 2-3 specific crates responsible for majority of the breakage, like it was done in #73345.
(In a more local way than before #75516, without Option<NonterminalKind> in MetaVarDecl, and without missing_fragment_specifiers in session.)

[Mark-Simulacrum]

We could also make it a hard warning rather than error for all crates, but I'm not sure if that's better. It also doesn't need session state.

[tmandry]

To expand a bit: cargo hides warnings generated by your dependencies, so crate owners don't feel the pain of depending on these old versions. I believe it passes -Awarnings to rustc, but otherwise passes through the output unchanged. If we had a warning that printed in spite of -Awarnings it would encourage people to update their deps.

I assume that is what is meant by "hard warning"?

[Mark-Simulacrum]

I've posted a revert to current beta (#77456) but we still need to fix this on master. Moving milestone to 1.48, though.

[Aaron1011]

I think this is another case where #71249 would help (see #75534)

[petrochenkov]

I'll try to address this on nightly until the next release (the revert has landed on beta only #77456).

[pietroalbini]

Changed this to a nightly regression (so 1.48 regression) since we reverted the change on beta.

[jhpratt]

What needs to change in time-macros-impl? There's no warnings currently emitted by any of my code, as verified in CI. I certainly would have run across it at some point while developing otherwise.

In the future though, a friendly ping would be appreciated 🙂

[Mark-Simulacrum]

Assigning to @estebank to change the lint on nightly to a hard warning, which should both prevent hard errors (and as such regressions) and still avoid global state.

[petrochenkov]

So, I assumed that $i without a "fragment specifier" would behave like some actual fragment specifier, e.g. tt, but apparently it doesn't and it always produces an error on attempt to match, so the Option in MetaVarDecl is necessary and the logic cannot be simplified.

[estebank]

@petrochenkov does that mean we need to revert #75516 in master too (and potentially on the upcoming beta)?

[petrochenkov]

I think we need to revert on master and then do any other work starting from that state.

[Mark-Simulacrum]

#75516 does not seem to revert cleanly on current beta (1.49). @estebank @petrochenkov could one of you prepare a revert PR targeting beta? (I agree we should also revert on master)