summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-07-27 12:35:18 +0000
committerbors <bors@rust-lang.org>2024-07-27 12:35:18 +0000
commit8fe0c753f23e7050b87a444b6622caf4d2272d5d (patch)
tree63d4f54922822700207afe54af5176a3e6b83aa2
parent3942254d00bf95cd5920980f85ebea57a1e6ce2a (diff)
parent8c402f125ccb3d8ccbabef40694482d481fc28b8 (diff)
downloadrust-8fe0c753f23e7050b87a444b6622caf4d2272d5d.tar.gz
rust-8fe0c753f23e7050b87a444b6622caf4d2272d5d.zip
Auto merge of #128006 - tgross35:missing-fragment-specifier-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 <https://github.com/rust-lang/rust/issues/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](https://github.com/rust-lang/rfcs/pull/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 https://github.com/rust-lang/rust/issues/40107#issuecomment-761727885, 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:

- https://github.com/rust-lang/rust/issues/128143
-rw-r--r--compiler/rustc_expand/messages.ftl5
-rw-r--r--compiler/rustc_expand/src/errors.rs17
-rw-r--r--compiler/rustc_expand/src/mbe/macro_check.rs23
-rw-r--r--compiler/rustc_expand/src/mbe/quoted.rs2
-rw-r--r--tests/ui/macros/macro-missing-fragment.e2015.stderr (renamed from tests/ui/macros/macro-missing-fragment.stderr)22
-rw-r--r--tests/ui/macros/macro-missing-fragment.e2024.stderr47
-rw-r--r--tests/ui/macros/macro-missing-fragment.rs23
7 files changed, 114 insertions, 25 deletions
diff --git a/compiler/rustc_expand/messages.ftl b/compiler/rustc_expand/messages.ftl
index cc0b110d2bc..18d95a398fd 100644
--- a/compiler/rustc_expand/messages.ftl
+++ b/compiler/rustc_expand/messages.ftl
@@ -105,6 +105,11 @@ expand_meta_var_dif_seq_matchers = {$msg}
 expand_meta_var_expr_unrecognized_var =
     variable `{$key}` is not recognized in meta-variable expression
 
+expand_missing_fragment_specifier = missing fragment specifier
+    .note = fragment specifiers must be specified in the 2024 edition
+    .suggestion_add_fragspec = try adding a specifier here
+    .valid = {$valid}
+
 expand_module_circular =
     circular modules: {$modules}
 
diff --git a/compiler/rustc_expand/src/errors.rs b/compiler/rustc_expand/src/errors.rs
index 0be7403f25b..6f1a0f16c49 100644
--- a/compiler/rustc_expand/src/errors.rs
+++ b/compiler/rustc_expand/src/errors.rs
@@ -417,6 +417,23 @@ pub struct DuplicateMatcherBinding {
 }
 
 #[derive(Diagnostic)]
+#[diag(expand_missing_fragment_specifier)]
+#[note]
+#[help(expand_valid)]
+pub struct MissingFragmentSpecifier {
+    #[primary_span]
+    pub span: Span,
+    #[suggestion(
+        expand_suggestion_add_fragspec,
+        style = "verbose",
+        code = ":spec",
+        applicability = "maybe-incorrect"
+    )]
+    pub add_span: Span,
+    pub valid: &'static str,
+}
+
+#[derive(Diagnostic)]
 #[diag(expand_invalid_fragment_specifier)]
 #[help]
 pub struct InvalidFragmentSpecifier {
diff --git a/compiler/rustc_expand/src/mbe/macro_check.rs b/compiler/rustc_expand/src/mbe/macro_check.rs
index 4b730d307fd..161e27fe02c 100644
--- a/compiler/rustc_expand/src/mbe/macro_check.rs
+++ b/compiler/rustc_expand/src/mbe/macro_check.rs
@@ -115,6 +115,7 @@ use rustc_errors::MultiSpan;
 use rustc_lint_defs::BuiltinLintDiag;
 use rustc_session::lint::builtin::{META_VARIABLE_MISUSE, MISSING_FRAGMENT_SPECIFIER};
 use rustc_session::parse::ParseSess;
+use rustc_span::edition::Edition;
 use rustc_span::symbol::kw;
 use rustc_span::{symbol::MacroRulesNormalizedIdent, ErrorGuaranteed, Span};
 
@@ -122,6 +123,8 @@ use smallvec::SmallVec;
 
 use std::iter;
 
+use super::quoted::VALID_FRAGMENT_NAMES_MSG_2021;
+
 /// Stack represented as linked list.
 ///
 /// Those are used for environments because they grow incrementally and are not mutable.
@@ -269,12 +272,20 @@ fn check_binders(
                 // 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.
-                psess.buffer_lint(
-                    MISSING_FRAGMENT_SPECIFIER,
-                    span,
-                    node_id,
-                    BuiltinLintDiag::MissingFragmentSpecifier,
-                );
+                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,
+                    );
+                }
             }
             if !macros.is_empty() {
                 psess.dcx().span_bug(span, "unexpected MetaVarDecl in nested lhs");
diff --git a/compiler/rustc_expand/src/mbe/quoted.rs b/compiler/rustc_expand/src/mbe/quoted.rs
index 9c480f17b42..57b6947316d 100644
--- a/compiler/rustc_expand/src/mbe/quoted.rs
+++ b/compiler/rustc_expand/src/mbe/quoted.rs
@@ -16,7 +16,7 @@ use rustc_span::Span;
 const VALID_FRAGMENT_NAMES_MSG: &str = "valid fragment specifiers are \
                                         `ident`, `block`, `stmt`, `expr`, `pat`, `ty`, `lifetime`, \
                                         `literal`, `path`, `meta`, `tt`, `item` and `vis`";
-const VALID_FRAGMENT_NAMES_MSG_2021: &str = "valid fragment specifiers are \
+pub const VALID_FRAGMENT_NAMES_MSG_2021: &str = "valid fragment specifiers are \
                                              `ident`, `block`, `stmt`, `expr`, `expr_2021`, `pat`, \
                                              `ty`, `lifetime`, `literal`, `path`, `meta`, `tt`, \
                                              `item` and `vis`";
diff --git a/tests/ui/macros/macro-missing-fragment.stderr b/tests/ui/macros/macro-missing-fragment.e2015.stderr
index abe4d4cd68a..a068dc5c054 100644
--- a/tests/ui/macros/macro-missing-fragment.stderr
+++ b/tests/ui/macros/macro-missing-fragment.e2015.stderr
@@ -1,11 +1,11 @@
 error: missing fragment specifier
-  --> $DIR/macro-missing-fragment.rs:4:20
+  --> $DIR/macro-missing-fragment.rs:9:20
    |
 LL |     ( $( any_token $field_rust_type )* ) => {};
    |                    ^^^^^^^^^^^^^^^^
 
 warning: missing fragment specifier
-  --> $DIR/macro-missing-fragment.rs:4:20
+  --> $DIR/macro-missing-fragment.rs:9:20
    |
 LL |     ( $( any_token $field_rust_type )* ) => {};
    |                    ^^^^^^^^^^^^^^^^
@@ -13,13 +13,13 @@ LL |     ( $( any_token $field_rust_type )* ) => {};
    = 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/macro-missing-fragment.rs:1:9
+  --> $DIR/macro-missing-fragment.rs:6:9
    |
 LL | #![warn(missing_fragment_specifier)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: missing fragment specifier
-  --> $DIR/macro-missing-fragment.rs:12:7
+  --> $DIR/macro-missing-fragment.rs:19:7
    |
 LL |     ( $name ) => {};
    |       ^^^^^
@@ -28,7 +28,7 @@ LL |     ( $name ) => {};
    = note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
 
 warning: missing fragment specifier
-  --> $DIR/macro-missing-fragment.rs:18:7
+  --> $DIR/macro-missing-fragment.rs:26:7
    |
 LL |     ( $name ) => {};
    |       ^^^^^
@@ -40,7 +40,7 @@ error: aborting due to 1 previous error; 3 warnings emitted
 
 Future incompatibility report: Future breakage diagnostic:
 warning: missing fragment specifier
-  --> $DIR/macro-missing-fragment.rs:4:20
+  --> $DIR/macro-missing-fragment.rs:9:20
    |
 LL |     ( $( any_token $field_rust_type )* ) => {};
    |                    ^^^^^^^^^^^^^^^^
@@ -48,14 +48,14 @@ LL |     ( $( any_token $field_rust_type )* ) => {};
    = 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/macro-missing-fragment.rs:1:9
+  --> $DIR/macro-missing-fragment.rs:6:9
    |
 LL | #![warn(missing_fragment_specifier)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Future breakage diagnostic:
 warning: missing fragment specifier
-  --> $DIR/macro-missing-fragment.rs:12:7
+  --> $DIR/macro-missing-fragment.rs:19:7
    |
 LL |     ( $name ) => {};
    |       ^^^^^
@@ -63,14 +63,14 @@ 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: the lint level is defined here
-  --> $DIR/macro-missing-fragment.rs:1:9
+  --> $DIR/macro-missing-fragment.rs:6:9
    |
 LL | #![warn(missing_fragment_specifier)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Future breakage diagnostic:
 warning: missing fragment specifier
-  --> $DIR/macro-missing-fragment.rs:18:7
+  --> $DIR/macro-missing-fragment.rs:26:7
    |
 LL |     ( $name ) => {};
    |       ^^^^^
@@ -78,7 +78,7 @@ 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: the lint level is defined here
-  --> $DIR/macro-missing-fragment.rs:1:9
+  --> $DIR/macro-missing-fragment.rs:6:9
    |
 LL | #![warn(missing_fragment_specifier)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/ui/macros/macro-missing-fragment.e2024.stderr b/tests/ui/macros/macro-missing-fragment.e2024.stderr
new file mode 100644
index 00000000000..3afa069c170
--- /dev/null
+++ b/tests/ui/macros/macro-missing-fragment.e2024.stderr
@@ -0,0 +1,47 @@
+error: missing fragment specifier
+  --> $DIR/macro-missing-fragment.rs:9:20
+   |
+LL |     ( $( any_token $field_rust_type )* ) => {};
+   |                    ^^^^^^^^^^^^^^^^
+   |
+   = 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 |     ( $( any_token $field_rust_type:spec )* ) => {};
+   |                                    +++++
+
+error: missing fragment specifier
+  --> $DIR/macro-missing-fragment.rs:19:7
+   |
+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
+   |
+LL |     ( $name:spec ) => {};
+   |            +++++
+
+error: missing fragment specifier
+  --> $DIR/macro-missing-fragment.rs:26:7
+   |
+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
+   |
+LL |     ( $name:spec ) => {};
+   |            +++++
+
+error: missing fragment specifier
+  --> $DIR/macro-missing-fragment.rs:9:20
+   |
+LL |     ( $( any_token $field_rust_type )* ) => {};
+   |                    ^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+
diff --git a/tests/ui/macros/macro-missing-fragment.rs b/tests/ui/macros/macro-missing-fragment.rs
index 210c85ebbf2..b7da87ae610 100644
--- a/tests/ui/macros/macro-missing-fragment.rs
+++ b/tests/ui/macros/macro-missing-fragment.rs
@@ -1,23 +1,32 @@
+//@ revisions: e2015 e2024
+//@[e2015] edition:2015
+//@[e2024] edition:2024
+//@[e2024] compile-flags: -Zunstable-options
+
 #![warn(missing_fragment_specifier)]
 
 macro_rules! used_arm {
     ( $( any_token $field_rust_type )* ) => {};
-    //~^ ERROR missing fragment
-    //~| WARN missing fragment
-    //~| WARN this was previously accepted
+    //[e2015]~^ ERROR missing fragment
+    //[e2015]~| WARN missing fragment
+    //[e2015]~| WARN this was previously accepted
+    //[e2024]~^^^^ ERROR missing fragment
+    //[e2024]~| ERROR missing fragment
 }
 
 macro_rules! used_macro_unused_arm {
     () => {};
     ( $name ) => {};
-    //~^ WARN missing fragment
-    //~| WARN this was previously accepted
+    //[e2015]~^ WARN missing fragment
+    //[e2015]~| WARN this was previously accepted
+    //[e2024]~^^^ ERROR missing fragment
 }
 
 macro_rules! unused_macro {
     ( $name ) => {};
-    //~^ WARN missing fragment
-    //~| WARN this was previously accepted
+    //[e2015]~^ WARN missing fragment
+    //[e2015]~| WARN this was previously accepted
+    //[e2024]~^^^ ERROR missing fragment
 }
 
 fn main() {