Rollup merge of rust-lang#95808 - petrochenkov:fragspec, r=nnethercote

expand: Remove `ParseSess::missing_fragment_specifiers`

It was used for deduplicating some errors for legacy code which are mostly deduplicated even without that, but at cost of global mutable state, which is not a good tradeoff.

cc rust-lang#95747 (comment)
r? `@nnethercote`

Author: Dylan-DPC

compiler/rustc_expand/src/mbe/macro_check.rs

@@ -110,7 +110,7 @@ use rustc_ast::token::{DelimToken, Token, TokenKind};
 use rustc_ast::{NodeId, DUMMY_NODE_ID};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::MultiSpan;
-use rustc_session::lint::builtin::META_VARIABLE_MISUSE;
+use rustc_session::lint::builtin::{META_VARIABLE_MISUSE, MISSING_FRAGMENT_SPECIFIER};
 use rustc_session::parse::ParseSess;
 use rustc_span::symbol::kw;
 use rustc_span::{symbol::MacroRulesNormalizedIdent, Span};
@@ -261,7 +261,18 @@ fn check_binders(
             }
         }
         // Similarly, this can only happen when checking a toplevel macro.
-        TokenTree::MetaVarDecl(span, name, _kind) => {
+        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.
+                sess.buffer_lint(
+                    MISSING_FRAGMENT_SPECIFIER,
+                    span,
+                    node_id,
+                    &format!("missing fragment specifier"),
+                );
+            }
             if !macros.is_empty() {
                 sess.span_diagnostic.span_bug(span, "unexpected MetaVarDecl in nested lhs");
             }

compiler/rustc_expand/src/mbe/macro_parser.rs

@@ -411,7 +411,6 @@ impl TtParser {
     /// track of through the mps generated.
     fn parse_tt_inner(
         &mut self,
-        sess: &ParseSess,
         matcher: &[MatcherLoc],
         token: &Token,
     ) -> Option<NamedParseResult> {
@@ -519,11 +518,9 @@ impl TtParser {
                             self.bb_mps.push(mp);
                         }
                     } else {
+                        // E.g. `$e` instead of `$e:expr`, reported as a hard error if actually used.
                         // Both this check and the one in `nameize` are necessary, surprisingly.
-                        if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
-                            // E.g. `$e` instead of `$e:expr`.
-                            return Some(Error(span, "missing fragment specifier".to_string()));
-                        }
+                        return Some(Error(span, "missing fragment specifier".to_string()));
                     }
                 }
                 MatcherLoc::Eof => {
@@ -549,7 +546,7 @@ impl TtParser {
                     // Need to take ownership of the matches from within the `Lrc`.
                     Lrc::make_mut(&mut eof_mp.matches);
                     let matches = Lrc::try_unwrap(eof_mp.matches).unwrap().into_iter();
-                    self.nameize(sess, matcher, matches)
+                    self.nameize(matcher, matches)
                 }
                 EofMatcherPositions::Multiple => {
                     Error(token.span, "ambiguity: multiple successful parses".to_string())
@@ -587,7 +584,7 @@ impl TtParser {
 
             // Process `cur_mps` until either we have finished the input or we need to get some
             // parsing from the black-box parser done.
-            if let Some(res) = self.parse_tt_inner(&parser.sess, matcher, &parser.token) {
+            if let Some(res) = self.parse_tt_inner(matcher, &parser.token) {
                 return res;
             }
 
@@ -694,7 +691,6 @@ impl TtParser {
 
     fn nameize<I: Iterator<Item = NamedMatch>>(
         &self,
-        sess: &ParseSess,
         matcher: &[MatcherLoc],
         mut res: I,
     ) -> NamedParseResult {
@@ -711,11 +707,9 @@ impl TtParser {
                         }
                     };
                 } else {
+                    // E.g. `$e` instead of `$e:expr`, reported as a hard error if actually used.
                     // Both this check and the one in `parse_tt_inner` are necessary, surprisingly.
-                    if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
-                        // E.g. `$e` instead of `$e:expr`.
-                        return Error(span, "missing fragment specifier".to_string());
-                    }
+                    return Error(span, "missing fragment specifier".to_string());
                 }
             }
         }

compiler/rustc_expand/src/mbe/quoted.rs

@@ -2,8 +2,7 @@ use crate::mbe::macro_parser::count_metavar_decls;
 use crate::mbe::{Delimited, KleeneOp, KleeneToken, MetaVarExpr, SequenceRepetition, TokenTree};
 
 use rustc_ast::token::{self, Token};
-use rustc_ast::tokenstream;
-use rustc_ast::{NodeId, DUMMY_NODE_ID};
+use rustc_ast::{tokenstream, NodeId};
 use rustc_ast_pretty::pprust;
 use rustc_feature::Features;
 use rustc_session::parse::{feature_err, ParseSess};
@@ -104,10 +103,7 @@ pub(super) fn parse(
                     }
                     tree => tree.as_ref().map_or(start_sp, tokenstream::TokenTree::span),
                 };
-                if node_id != DUMMY_NODE_ID {
-                    // Macros loaded from other crates have dummy node ids.
-                    sess.missing_fragment_specifiers.borrow_mut().insert(span, node_id);
-                }
+
                 result.push(TokenTree::MetaVarDecl(span, ident, None));
             }
 

compiler/rustc_interface/src/passes.rs

@@ -30,7 +30,6 @@ use rustc_resolve::{Resolver, ResolverArenas};
 use rustc_serialize::json;
 use rustc_session::config::{CrateType, Input, OutputFilenames, OutputType};
 use rustc_session::cstore::{MetadataLoader, MetadataLoaderDyn};
-use rustc_session::lint;
 use rustc_session::output::{filename_for_input, filename_for_metadata};
 use rustc_session::search_paths::PathKind;
 use rustc_session::{Limit, Session};
@@ -349,23 +348,8 @@ pub fn configure_and_expand(
             ecx.check_unused_macros();
         });
 
-        let mut missing_fragment_specifiers: Vec<_> = ecx
-            .sess
-            .parse_sess
-            .missing_fragment_specifiers
-            .borrow()
-            .iter()
-            .map(|(span, node_id)| (*span, *node_id))
-            .collect();
-        missing_fragment_specifiers.sort_unstable_by_key(|(span, _)| *span);
-
         let recursion_limit_hit = ecx.reduced_recursion_limit.is_some();
 
-        for (span, node_id) in missing_fragment_specifiers {
-            let lint = lint::builtin::MISSING_FRAGMENT_SPECIFIER;
-            let msg = "missing fragment specifier";
-            resolver.lint_buffer().buffer_lint(lint, node_id, span, msg);
-        }
         if cfg!(windows) {
             env::set_var("PATH", &old_path);
         }

compiler/rustc_session/src/parse.rs

@@ -140,7 +140,6 @@ pub struct ParseSess {
     pub config: CrateConfig,
     pub check_config: CrateCheckConfig,
     pub edition: Edition,
-    pub missing_fragment_specifiers: Lock<FxHashMap<Span, NodeId>>,
     /// Places where raw identifiers were used. This is used to avoid complaining about idents
     /// clashing with keywords in new editions.
     pub raw_identifier_spans: Lock<Vec<Span>>,
@@ -195,7 +194,6 @@ impl ParseSess {
             config: FxHashSet::default(),
             check_config: CrateCheckConfig::default(),
             edition: ExpnId::root().expn_data().edition,
-            missing_fragment_specifiers: Default::default(),
             raw_identifier_spans: Lock::new(Vec::new()),
             bad_unicode_identifiers: Lock::new(Default::default()),
             source_map,

src/test/ui/macros/macro-match-nonterminal.rs

@@ -2,6 +2,8 @@ macro_rules! test {
     ($a, $b) => {
         //~^ ERROR missing fragment
         //~| ERROR missing fragment
+        //~| ERROR missing fragment
+        //~| WARN this was previously accepted
         //~| WARN this was previously accepted
         ()
     };

src/test/ui/macros/macro-match-nonterminal.stderr

@@ -4,15 +4,24 @@ error: missing fragment specifier
 LL |     ($a, $b) => {
    |        ^
 
+error: missing fragment specifier
+  --> $DIR/macro-match-nonterminal.rs:2:8
+   |
+LL |     ($a, $b) => {
+   |        ^
+   |
+   = note: `#[deny(missing_fragment_specifier)]` on by default
+   = 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: missing fragment specifier
   --> $DIR/macro-match-nonterminal.rs:2:10
    |
 LL |     ($a, $b) => {
    |          ^^
    |
-   = note: `#[deny(missing_fragment_specifier)]` on by default
    = 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 2 previous errors
+error: aborting due to 3 previous errors
 

src/test/ui/macros/macro-missing-fragment-deduplication.rs

@@ -0,0 +1,15 @@
+// compile-flags: -Zdeduplicate-diagnostics=yes
+
+macro_rules! m {
+    ($name) => {}
+    //~^ ERROR missing fragment
+    //~| ERROR missing fragment
+    //~| WARN this was previously accepted
+}
+
+fn main() {
+    m!();
+    m!();
+    m!();
+    m!();
+}

src/test/ui/macros/macro-missing-fragment-deduplication.stderr

@@ -0,0 +1,18 @@
+error: missing fragment specifier
+  --> $DIR/macro-missing-fragment-deduplication.rs:4:6
+   |
+LL |     ($name) => {}
+   |      ^^^^^
+
+error: missing fragment specifier
+  --> $DIR/macro-missing-fragment-deduplication.rs:4:6
+   |
+LL |     ($name) => {}
+   |      ^^^^^
+   |
+   = note: `#[deny(missing_fragment_specifier)]` on by default
+   = 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 2 previous errors
+

src/test/ui/macros/macro-missing-fragment.rs

@@ -1,7 +1,26 @@
-macro_rules! m {
-    ( $( any_token $field_rust_type )* ) => {}; //~ ERROR missing fragment
+#![warn(missing_fragment_specifier)]
+
+macro_rules! used_arm {
+    ( $( any_token $field_rust_type )* ) => {};
+    //~^ ERROR missing fragment
+    //~| WARN missing fragment
+    //~| WARN this was previously accepted
+}
+
+macro_rules! used_macro_unused_arm {
+    () => {};
+    ( $name ) => {};
+    //~^ WARN missing fragment
+    //~| WARN this was previously accepted
+}
+
+macro_rules! unused_macro {
+    ( $name ) => {};
+    //~^ WARN missing fragment
+    //~| WARN this was previously accepted
 }
 
 fn main() {
-    m!();
+    used_arm!();
+    used_macro_unused_arm!();
 }