Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for future-incompatibility lint missing_fragment_specifier #40107

Open
jseyfried opened this issue Feb 26, 2017 · 12 comments · Fixed by #75516 or #128006
Open

Tracking issue for future-incompatibility lint missing_fragment_specifier #40107

jseyfried opened this issue Feb 26, 2017 · 12 comments · Fixed by #75516 or #128006
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jseyfried
Copy link
Contributor

jseyfried commented Feb 26, 2017

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

@steveklabnik steveklabnik added A-compiler T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 1, 2017
danielrh added a commit to dropbox/rust-brotli-decompressor that referenced this issue Apr 25, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-future-incompatibility Category: Future-incompatibility lints label Jun 20, 2017
@petrochenkov petrochenkov changed the title missing_fragment_specifiers future-compatibility warnings missing_fragment_specifier future-compatibility warnings Jul 7, 2017
LinuxMercedes added a commit to LinuxMercedes/hashpipe that referenced this issue Jul 8, 2017
Bump `clap` to fix the build on new versions of `rustc` (see [rust-lang/rust#42894](rust-lang/rust#42894) and [rust-lang/rust#40107](rust-lang/rust#40107) for details)
@stbuehler
Copy link

stbuehler commented Jul 14, 2017

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.

@Mark-Simulacrum Mark-Simulacrum removed the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
bors added a commit that referenced this issue Aug 3, 2019
Transition some C-future-compatibility lints to {ERROR, DENY}

Closes #40107 (ERROR).
Closes #39207 (ERROR).
Closes #37872 (ERROR).
Closes #36887 (ERROR).
Closes #36247 (ERROR.
Closes #42238 (ERROR).
Transitions #59014 (DENY).
Transitions #57571 (DENY).
Closes #60210 (ERROR).
Transitions #35203 (DENY).

r? @petrochenkov
@steveklabnik
Copy link
Member

Triage: this is still deny by default.

@jonas-schievink jonas-schievink added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label Jan 15, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 24, 2020
…_specifier_hard_error, r=Mark-Simulacrum

Revert missing fragment specifier hard error

Closes rust-lang#76605

Reopens rust-lang#40107

r? ``@Mark-Simulacrum``
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 24, 2020
…_specifier_hard_error, r=Mark-Simulacrum

Revert missing fragment specifier hard error

Closes rust-lang#76605

Reopens rust-lang#40107

r? ```@Mark-Simulacrum```
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 25, 2020
…pecifier_hard_error, r=Mark-Simulacrum

Revert missing fragment specifier hard error

Closes rust-lang#76605

Reopens rust-lang#40107

r? `@Mark-Simulacrum`
@wesleywiser wesleywiser reopened this Dec 26, 2020
@estebank
Copy link
Contributor

estebank commented Jan 17, 2021

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
Copy link
Member

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
Copy link
Member

I think this would be a good fit for cargo report-future-compat once rust-lang/cargo#8825 is merged.

@estebank
Copy link
Contributor

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
Copy link

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
Copy link
Member

pnkfelix commented Mar 7, 2022

rust-lang/cargo#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
Copy link
Member

pnkfelix commented Mar 7, 2022

@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
Copy link
Member

@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).

tgross35 added a commit to tgross35/rust that referenced this issue Jul 20, 2024
`missing_fragment_specifier` has been a future compatibility warning
since 2017. Uplifting it to an unconditional hard error was attempted in
2020, but eventually reverted due to fallout.

Make it an error only in edition >= 2024, leaving the lint for older
editions. This change will make it easier to support more macro syntax
that relies on usage of `$`.

Fixes <rust-lang#40107>
tgross35 added a commit to tgross35/rust that referenced this issue Jul 27, 2024
`missing_fragment_specifier` has been a future compatibility warning
since 2017. Uplifting it to an unconditional hard error was attempted in
2020, but eventually reverted due to fallout.

Make it an error only in edition >= 2024, leaving the lint for older
editions. This change will make it easier to support more macro syntax
that relies on usage of `$`.

Fixes <rust-lang#40107>
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 27, 2024
…r-e2024, r=petrochenkov

Make `missing_fragment_specifier` an error in edition 2024

`missing_fragment_specifier` has been a future compatibility warning since 2017. Uplifting it to an unconditional hard error was attempted in 2020, but eventually reverted due to fallout.

Make it an error only in edition >= 2024, leaving the lint for older editions. This change will make it easier to support more macro syntax that relies on usage of `$`.

Fixes <rust-lang#40107>

---

It is rather late for the edition but since this change is relatively small, it seems worth at least bringing up. This follows a brief [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/268952-edition/topic/.60.20DBD.20-.3E.20hard.20error) (cc `@tmandry).`

Making this an edition-dependent lint has come up before but there was not a strong motivation. I am proposing it at this time because this would simplify the [named macro capture groups](rust-lang/rfcs#3649) RFC, which has had mildly positive response, and makes use of new `$` syntax in the matcher. The proposed syntax currently parses as metavariables without a fragment specifier; this warning is raised, but there are no errors.

It is obviously not known that this specific RFC will eventually be accepted, but forbidding `missing_fragment_specifier` should make it easier to support any new syntax in the future that makes use of `$` in different ways. The syntax conflict is also not impossible to overcome, but making it clear that unnamed metavariables are rejected makes things more straightforward and should allow for better diagnostics.

`@Mark-Simulacrum` suggested making this forbid-by-default instead of an error at rust-lang#40107 (comment), but I don't think this would allow the same level of syntax flexibility.

It is also possible to reconsider making this an unconditional error since four years have elapsed since the previous attempt, but this seems likely to hit the same pitfalls. (Possibly worth a crater run?)

Tracking:

- rust-lang#128143
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 27, 2024
…e2024, r=petrochenkov

Make `missing_fragment_specifier` an error in edition 2024

`missing_fragment_specifier` has been a future compatibility warning since 2017. Uplifting it to an unconditional hard error was attempted in 2020, but eventually reverted due to fallout.

Make it an error only in edition >= 2024, leaving the lint for older editions. This change will make it easier to support more macro syntax that relies on usage of `$`.

Fixes <rust-lang#40107>

---

It is rather late for the edition but since this change is relatively small, it seems worth at least bringing up. This follows a brief [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/268952-edition/topic/.60.20DBD.20-.3E.20hard.20error) (cc `@tmandry).`

Making this an edition-dependent lint has come up before but there was not a strong motivation. I am proposing it at this time because this would simplify the [named macro capture groups](rust-lang/rfcs#3649) RFC, which has had mildly positive response, and makes use of new `$` syntax in the matcher. The proposed syntax currently parses as metavariables without a fragment specifier; this warning is raised, but there are no errors.

It is obviously not known that this specific RFC will eventually be accepted, but forbidding `missing_fragment_specifier` should make it easier to support any new syntax in the future that makes use of `$` in different ways. The syntax conflict is also not impossible to overcome, but making it clear that unnamed metavariables are rejected makes things more straightforward and should allow for better diagnostics.

`@Mark-Simulacrum` suggested making this forbid-by-default instead of an error at rust-lang#40107 (comment), but I don't think this would allow the same level of syntax flexibility.

It is also possible to reconsider making this an unconditional error since four years have elapsed since the previous attempt, but this seems likely to hit the same pitfalls. (Possibly worth a crater run?)

Tracking:

- rust-lang#128143
@ehuss
Copy link
Contributor

ehuss commented Jul 29, 2024

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 #128006 (comment), 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).

@ehuss ehuss reopened this Jul 29, 2024
@tgross35
Copy link
Contributor

tgross35 commented Aug 4, 2024

There is a crater run queued for a hard error at #128425.

@fmease fmease changed the title missing_fragment_specifier future-compatibility warnings Tracking issue for future-incompatibility lint missing_fragment_specifier Sep 14, 2024
@fmease fmease added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-parser Area: The parsing of Rust source code to an AST C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Archived in project