rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.88k stars 12.67k forks source link

Tracking issue for future-incompatibility lint `missing_fragment_specifier` #40107

Open jseyfried opened 7 years ago

jseyfried commented 7 years ago

This is the summary issue for the missing_fragment_specifier future-compatibility warning and other related errors. The goal of this page is describe why this change was made and how you can fix code that is affected by it. It also provides a place to ask questions or register a complaint if you feel the change should not be made.

What is the warning for?

The missing_fragment_specifier warning is issued when an unused pattern in a macro_rules! macro definition has a meta-variable (e.g. $e) that is not followed by a fragment specifier (e.g. :expr).

This warning can always be fixed by removing the unused pattern in the macro_rules! macro definition.

When will this warning become a hard error?

At the beginning of each 6-week release cycle, the Rust compiler team will review the set of outstanding future compatibility warnings and nominate some of them for Final Comment Period. Toward the end of the cycle, we will review any comments and make a final determination whether to convert the warning into a hard error or remove it entirely.

Current status

stbuehler commented 7 years ago

As far as I can tell a macro rule which triggers this warning is not actually usable; is there any reason not to make this an error right now?

See ab91c70cc6:src/libsyntax/ext/tt/macro_parser.rs:384.

Edit: it seems in nightly the lint default was changed to Deny.

steveklabnik commented 4 years ago

Triage: this is still deny by default.

estebank commented 3 years ago

Given the story behind this I think that a possible course of action would be to promote this to a hard error only on 2021 edition. This would neatly sidestep the breakage observed in the ecosystem.

cc @rust-lang/compiler @rust-lang/lang

Mark-Simulacrum commented 3 years ago

I don't think an edition-specific hard error buys us much over the current situation. I think we should make the lint an error on all editions, but affected by cap-lints - essentially a forbid-by-default lint. That should remove the global state that @matklad wanted to remove and avoid too much complexity, while essentially avoiding any of the known cases where there's breakage.

I suspect that we could also just make it a hard error in the new edition (no cap-lints) but it's not really necessary IMO, just complicates things.

Aaron1011 commented 3 years ago

I think this would be a good fit for cargo report-future-compat once https://github.com/rust-lang/cargo/pull/8825 is merged.

estebank commented 3 years ago

I suspect that we could also just make it a hard error in the new edition (no cap-lints) but it's not really necessary IMO, just complicates things.

I am a fan of promoting lints to hard errors on edition boundaries, despite the implementation burden. It gives us a remarkably powerful tool for language evolution (once we have determined that is the right decision).

koivunej commented 3 years ago

I'm doing a lot of typos today thanks to rust-analyzer not working; found this issue through me typing:

macro_rules! foo { ($x:$ident) => { pub struct $x; } }

or

macro_rules! foo { ($x$ident) => { pub struct $x; } }

playground

I can't remember ever seeing these kinds of macros this warning issue is for live but it does sound like the parser begins reading another parameter on $[ident] without requiring there to be a comma in between. Should this be reported as a bug or is this good here, maybe getting fixed when hard-error on 2021 happens?

pnkfelix commented 2 years ago

https://github.com/rust-lang/cargo/pull/8825 has been merged, so this (among many other issues tagged C-future-incompatibility) could be a candidate for becoming a hard error in a future edition.

pnkfelix commented 2 years ago

@koivunej wrote:

I can't remember ever seeing these kinds of macros this warning issue is for live but it does sound like the parser begins reading another parameter on $[ident] without requiring there to be a comma in between. Should this be reported as a bug or is this good here, maybe getting fixed when hard-error on 2021 happens?

I'm sorry, I'm not clear on what your comment is asking.

The two examples you wrote look like buggy code to me, and so I think the lint is correctly firing on them. Can you explain what you think should be happening instead?

Aaron1011 commented 2 years ago

@pnkfelix Note that this lint (along with all of the other future-incompat lints) is not included in the future-incompat report by default. That is, the lint will continue to be completely hidden when the crate is compiled as a dependency.

The only lints that show up in the cargo report are ones marked FutureReleaseErrorReportNow (currently, only PROC_MACRO_BACK_COMPAT and PROC_MACRO_DERIVE_RESOLUTION_FALLBACK).

ehuss commented 2 months ago

I'm going to reopen since I don't think this should have been closed. It is still a future-incompatible lint generating reports in dependencies. #128006 just changed it in the 2024 edition to be a hard error. Per https://github.com/rust-lang/rust/pull/128006#issuecomment-2251957820, it is still being considered to make this a hard-error in all editions. I think this issue would be as good as any for tracking that final step (and making a decision on it).

tgross35 commented 2 months ago

There is a crater run queued for a hard error at https://github.com/rust-lang/rust/pull/128425.