Skip to content

Commit

Permalink
Make missing_fragment_specifier an unconditional error
Browse files Browse the repository at this point in the history
This was attempted in [1] then reverted in [2] because of fallout.
Recently, this was made an edition-dependent error in [3].

Make missing fragment specifiers an unconditional error again.

[1]: #75516
[2]: #80210
[3]: #128006
  • Loading branch information
tgross35 committed Jul 31, 2024
1 parent 249cf71 commit f2d1d82
Show file tree
Hide file tree
Showing 19 changed files with 63 additions and 310 deletions.
25 changes: 6 additions & 19 deletions compiler/rustc_expand/src/mbe/macro_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,8 @@ use rustc_ast::{NodeId, DUMMY_NODE_ID};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::MultiSpan;
use rustc_lint_defs::BuiltinLintDiag;
use rustc_session::lint::builtin::{META_VARIABLE_MISUSE, MISSING_FRAGMENT_SPECIFIER};
use rustc_session::lint::builtin::META_VARIABLE_MISUSE;
use rustc_session::parse::ParseSess;
use rustc_span::edition::Edition;
use rustc_span::symbol::{kw, MacroRulesNormalizedIdent};
use rustc_span::{ErrorGuaranteed, Span};
use smallvec::SmallVec;
Expand Down Expand Up @@ -267,23 +266,11 @@ fn check_binders(
// Similarly, this can only happen when checking a toplevel macro.
TokenTree::MetaVarDecl(span, name, kind) => {
if kind.is_none() && node_id != DUMMY_NODE_ID {
// FIXME: Report this as a hard error eventually and remove equivalent errors from
// `parse_tt_inner` and `nameize`. Until then the error may be reported twice, once
// as a hard error and then once as a buffered lint.
if span.edition() >= Edition::Edition2024 {
psess.dcx().emit_err(errors::MissingFragmentSpecifier {
span,
add_span: span.shrink_to_hi(),
valid: VALID_FRAGMENT_NAMES_MSG_2021,
});
} else {
psess.buffer_lint(
MISSING_FRAGMENT_SPECIFIER,
span,
node_id,
BuiltinLintDiag::MissingFragmentSpecifier,
);
}
psess.dcx().emit_err(errors::MissingFragmentSpecifier {
span,
add_span: span.shrink_to_hi(),
valid: VALID_FRAGMENT_NAMES_MSG_2021,
});
}
if !macros.is_empty() {
psess.dcx().span_bug(span, "unexpected MetaVarDecl in nested lhs");
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,6 @@ lint_metavariable_still_repeating = variable `{$name}` is still repeating at thi
lint_metavariable_wrong_operator = meta-variable repeats with different Kleene operator
lint_missing_fragment_specifier = missing fragment specifier
lint_missing_unsafe_on_extern = extern blocks should be unsafe
.suggestion = needs `unsafe` before the extern keyword
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,6 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
BuiltinLintDiag::CrateNameInCfgAttr => {
lints::CrateNameInCfgAttr.decorate_lint(diag);
}
BuiltinLintDiag::MissingFragmentSpecifier => {
lints::MissingFragmentSpecifier.decorate_lint(diag);
}
BuiltinLintDiag::MetaVariableStillRepeating(name) => {
lints::MetaVariableStillRepeating { name }.decorate_lint(diag);
}
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2459,10 +2459,6 @@ pub struct CrateTypeInCfgAttr;
#[diag(lint_crate_name_in_cfg_attr_deprecated)]
pub struct CrateNameInCfgAttr;

#[derive(LintDiagnostic)]
#[diag(lint_missing_fragment_specifier)]
pub struct MissingFragmentSpecifier;

#[derive(LintDiagnostic)]
#[diag(lint_metavariable_still_repeating)]
pub struct MetaVariableStillRepeating {
Expand Down
45 changes: 0 additions & 45 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ declare_lint_pass! {
MACRO_USE_EXTERN_CRATE,
META_VARIABLE_MISUSE,
MISSING_ABI,
MISSING_FRAGMENT_SPECIFIER,
MISSING_UNSAFE_ON_EXTERN,
MUST_NOT_SUSPEND,
NAMED_ARGUMENTS_USED_POSITIONALLY,
Expand Down Expand Up @@ -1385,50 +1384,6 @@ declare_lint! {
};
}

declare_lint! {
/// The `missing_fragment_specifier` lint 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.
///
/// ### Example
///
/// ```rust,compile_fail
/// macro_rules! foo {
/// () => {};
/// ($name) => { };
/// }
///
/// fn main() {
/// foo!();
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// To fix this, remove the unused pattern from the `macro_rules!` macro definition:
///
/// ```rust
/// macro_rules! foo {
/// () => {};
/// }
/// fn main() {
/// foo!();
/// }
/// ```
pub MISSING_FRAGMENT_SPECIFIER,
Deny,
"detects missing fragment specifiers in unused `macro_rules!` patterns",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #40107 <https://github.com/rust-lang/rust/issues/40107>",
};
}

declare_lint! {
/// The `late_bound_lifetime_arguments` lint detects generic lifetime
/// arguments in path segments with late bound lifetime parameters.
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,6 @@ pub enum BuiltinLintDiag {
CfgAttrNoAttributes,
CrateTypeInCfgAttr,
CrateNameInCfgAttr,
MissingFragmentSpecifier,
MetaVariableStillRepeating(MacroRulesNormalizedIdent),
MetaVariableWrongOperator,
DuplicateMatcherBinding,
Expand Down
4 changes: 0 additions & 4 deletions tests/ui/lint/expansion-time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ macro_rules! foo {
( $($i:ident)* ) => { $($i)+ }; //~ WARN meta-variable repeats with different Kleene operator
}

#[warn(missing_fragment_specifier)]
macro_rules! m { ($i) => {} } //~ WARN missing fragment specifier
//~| WARN this was previously accepted

#[warn(soft_unstable)]
mod benches {
#[bench] //~ WARN use of unstable library feature 'test'
Expand Down
41 changes: 6 additions & 35 deletions tests/ui/lint/expansion-time.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,16 @@ note: the lint level is defined here
LL | #[warn(meta_variable_misuse)]
| ^^^^^^^^^^^^^^^^^^^^

warning: missing fragment specifier
--> $DIR/expansion-time.rs:9:19
|
LL | macro_rules! m { ($i) => {} }
| ^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
note: the lint level is defined here
--> $DIR/expansion-time.rs:8:8
|
LL | #[warn(missing_fragment_specifier)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: use of unstable library feature 'test': `bench` is a part of custom test frameworks which are unstable
--> $DIR/expansion-time.rs:14:7
--> $DIR/expansion-time.rs:10:7
|
LL | #[bench]
| ^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #64266 <https://github.com/rust-lang/rust/issues/64266>
note: the lint level is defined here
--> $DIR/expansion-time.rs:12:8
--> $DIR/expansion-time.rs:8:8
|
LL | #[warn(soft_unstable)]
| ^^^^^^^^^^^^^
Expand All @@ -47,39 +33,24 @@ LL | 2
| ^
|
note: the lint level is defined here
--> $DIR/expansion-time.rs:29:8
--> $DIR/expansion-time.rs:25:8
|
LL | #[warn(incomplete_include)]
| ^^^^^^^^^^^^^^^^^^

warning: 4 warnings emitted
warning: 3 warnings emitted

Future incompatibility report: Future breakage diagnostic:
warning: missing fragment specifier
--> $DIR/expansion-time.rs:9:19
|
LL | macro_rules! m { ($i) => {} }
| ^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
note: the lint level is defined here
--> $DIR/expansion-time.rs:8:8
|
LL | #[warn(missing_fragment_specifier)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

Future breakage diagnostic:
warning: use of unstable library feature 'test': `bench` is a part of custom test frameworks which are unstable
--> $DIR/expansion-time.rs:14:7
--> $DIR/expansion-time.rs:10:7
|
LL | #[bench]
| ^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #64266 <https://github.com/rust-lang/rust/issues/64266>
note: the lint level is defined here
--> $DIR/expansion-time.rs:12:8
--> $DIR/expansion-time.rs:8:8
|
LL | #[warn(soft_unstable)]
| ^^^^^^^^^^^^^
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/macros/issue-39404.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![allow(unused)]

macro_rules! m { ($i) => {} }
//~^ ERROR missing fragment specifier
//~| WARN previously accepted
macro_rules! m {
($i) => {}; //~ ERROR missing fragment specifier
}

fn main() {}
26 changes: 9 additions & 17 deletions tests/ui/macros/issue-39404.stderr
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
error: missing fragment specifier
--> $DIR/issue-39404.rs:3:19
--> $DIR/issue-39404.rs:4:6
|
LL | macro_rules! m { ($i) => {} }
| ^^
LL | ($i) => {};
| ^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
= note: `#[deny(missing_fragment_specifier)]` on by default
= note: fragment specifiers must be specified in the 2024 edition
= help: valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, `expr_2021`, `pat`, `ty`, `lifetime`, `literal`, `path`, `meta`, `tt`, `item` and `vis`
help: try adding a specifier here
|
LL | ($i:spec) => {};
| +++++

error: aborting due to 1 previous error

Future incompatibility report: Future breakage diagnostic:
error: missing fragment specifier
--> $DIR/issue-39404.rs:3:19
|
LL | macro_rules! m { ($i) => {} }
| ^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
= note: `#[deny(missing_fragment_specifier)]` on by default

2 changes: 0 additions & 2 deletions tests/ui/macros/macro-match-nonterminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ macro_rules! test {
//~^ ERROR missing fragment
//~| ERROR missing fragment
//~| ERROR missing fragment
//~| WARN this was previously accepted
//~| WARN this was previously accepted
()
};
}
Expand Down
39 changes: 12 additions & 27 deletions tests/ui/macros/macro-match-nonterminal.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,47 +3,32 @@ error: missing fragment specifier
|
LL | ($a, $b) => {
| ^

error: missing fragment specifier
--> $DIR/macro-match-nonterminal.rs:2:8
|
LL | ($a, $b) => {
| ^
= note: fragment specifiers must be specified in the 2024 edition
= help: valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, `expr_2021`, `pat`, `ty`, `lifetime`, `literal`, `path`, `meta`, `tt`, `item` and `vis`
help: try adding a specifier here
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
= note: `#[deny(missing_fragment_specifier)]` on by default
LL | ($a,:spec $b) => {
| +++++

error: missing fragment specifier
--> $DIR/macro-match-nonterminal.rs:2:10
|
LL | ($a, $b) => {
| ^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>

error: aborting due to 3 previous errors
= note: fragment specifiers must be specified in the 2024 edition
= help: valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, `expr_2021`, `pat`, `ty`, `lifetime`, `literal`, `path`, `meta`, `tt`, `item` and `vis`
help: try adding a specifier here
|
LL | ($a, $b:spec) => {
| +++++

Future incompatibility report: Future breakage diagnostic:
error: missing fragment specifier
--> $DIR/macro-match-nonterminal.rs:2:8
|
LL | ($a, $b) => {
| ^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
= note: `#[deny(missing_fragment_specifier)]` on by default

Future breakage diagnostic:
error: missing fragment specifier
--> $DIR/macro-match-nonterminal.rs:2:10
|
LL | ($a, $b) => {
| ^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
= note: `#[deny(missing_fragment_specifier)]` on by default
error: aborting due to 3 previous errors

6 changes: 2 additions & 4 deletions tests/ui/macros/macro-missing-fragment-deduplication.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
//@ compile-flags: -Zdeduplicate-diagnostics=yes

macro_rules! m {
($name) => {}
//~^ ERROR missing fragment
//~| ERROR missing fragment
//~| WARN this was previously accepted
($name) => {}; //~ ERROR missing fragment
//~| ERROR missing fragment
}

fn main() {
Expand Down
26 changes: 9 additions & 17 deletions tests/ui/macros/macro-missing-fragment-deduplication.stderr
Original file line number Diff line number Diff line change
@@ -1,29 +1,21 @@
error: missing fragment specifier
--> $DIR/macro-missing-fragment-deduplication.rs:4:6
|
LL | ($name) => {}
LL | ($name) => {};
| ^^^^^

error: missing fragment specifier
--> $DIR/macro-missing-fragment-deduplication.rs:4:6
|
LL | ($name) => {}
| ^^^^^
= note: fragment specifiers must be specified in the 2024 edition
= help: valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, `expr_2021`, `pat`, `ty`, `lifetime`, `literal`, `path`, `meta`, `tt`, `item` and `vis`
help: try adding a specifier here
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
= note: `#[deny(missing_fragment_specifier)]` on by default
LL | ($name:spec) => {};
| +++++

error: aborting due to 2 previous errors

Future incompatibility report: Future breakage diagnostic:
error: missing fragment specifier
--> $DIR/macro-missing-fragment-deduplication.rs:4:6
|
LL | ($name) => {}
LL | ($name) => {};
| ^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
= note: `#[deny(missing_fragment_specifier)]` on by default

error: aborting due to 2 previous errors

Loading

0 comments on commit f2d1d82

Please sign in to comment.