mbe: Defer checks for compile_error! until reporting an unused macro rule

The MBE parser checks rules at initial parse time to see if their RHS
has `compile_error!` in it, and returns a list of rule indexes and LHS
spans that don't map to `compile_error!`, for use in unused macro rule
checking.

Instead, have the unused macro rule reporting ask the macro for the rule
to report, and let the macro check at that time. That avoids checking
rules unless they're unused.

In the process, refactor the data structure used to store macro rules,
to group the LHS and RHS (and LHS span) of each rule together, and
refactor the unused rule tracking to only track rule indexes.

This ends up being a net simplification, and reduction in code size.

Author: joshtriplett

compiler/rustc_expand/src/base.rs

@@ -348,6 +348,10 @@ pub trait TTMacroExpander {
         span: Span,
         input: TokenStream,
     ) -> MacroExpanderResult<'cx>;
+
+    fn get_unused_rule(&self, _rule_i: usize) -> Option<(&Ident, Span)> {
+        None
+    }
 }
 
 pub type MacroExpanderResult<'cx> = ExpandResult<Box<dyn MacResult + 'cx>, ()>;

compiler/rustc_expand/src/mbe/diagnostics.rs

@@ -10,7 +10,7 @@ use rustc_span::source_map::SourceMap;
 use rustc_span::{ErrorGuaranteed, Ident, Span};
 use tracing::debug;
 
-use super::macro_rules::{NoopTracker, parser_from_cx};
+use super::macro_rules::{MacroRule, NoopTracker, parser_from_cx};
 use crate::expand::{AstFragmentKind, parse_ast_fragment};
 use crate::mbe::macro_parser::ParseResult::*;
 use crate::mbe::macro_parser::{MatcherLoc, NamedParseResult, TtParser};
@@ -22,14 +22,14 @@ pub(super) fn failed_to_match_macro(
     def_span: Span,
     name: Ident,
     arg: TokenStream,
-    lhses: &[Vec<MatcherLoc>],
+    rules: &[MacroRule],
 ) -> (Span, ErrorGuaranteed) {
     debug!("failed to match macro");
     // An error occurred, try the expansion again, tracking the expansion closely for better
     // diagnostics.
     let mut tracker = CollectTrackerAndEmitter::new(psess.dcx(), sp);
 
-    let try_success_result = try_match_macro(psess, name, &arg, lhses, &mut tracker);
+    let try_success_result = try_match_macro(psess, name, &arg, rules, &mut tracker);
 
     if try_success_result.is_ok() {
         // Nonterminal parser recovery might turn failed matches into successful ones,
@@ -80,12 +80,12 @@ pub(super) fn failed_to_match_macro(
 
     // Check whether there's a missing comma in this macro call, like `println!("{}" a);`
     if let Some((arg, comma_span)) = arg.add_comma() {
-        for lhs in lhses {
+        for rule in rules {
             let parser = parser_from_cx(psess, arg.clone(), Recovery::Allowed);
             let mut tt_parser = TtParser::new(name);
 
             if let Success(_) =
-                tt_parser.parse_tt(&mut Cow::Borrowed(&parser), lhs, &mut NoopTracker)
+                tt_parser.parse_tt(&mut Cow::Borrowed(&parser), &rule.lhs, &mut NoopTracker)
             {
                 if comma_span.is_dummy() {
                     err.note("you might be missing a comma");

compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -98,13 +98,18 @@ impl<'a> ParserAnyMacro<'a> {
     }
 }
 
+pub(super) struct MacroRule {
+    pub(super) lhs: Vec<MatcherLoc>,
+    lhs_span: Span,
+    rhs: mbe::TokenTree,
+}
+
 struct MacroRulesMacroExpander {
     node_id: NodeId,
     name: Ident,
     span: Span,
     transparency: Transparency,
-    lhses: Vec<Vec<MatcherLoc>>,
-    rhses: Vec<mbe::TokenTree>,
+    rules: Vec<MacroRule>,
 }
 
 impl TTMacroExpander for MacroRulesMacroExpander {
@@ -122,10 +127,15 @@ impl TTMacroExpander for MacroRulesMacroExpander {
             self.name,
             self.transparency,
             input,
-            &self.lhses,
-            &self.rhses,
+            &self.rules,
         ))
     }
+
+    fn get_unused_rule(&self, rule_i: usize) -> Option<(&Ident, Span)> {
+        // If the rhs contains an invocation like `compile_error!`, don't report it as unused.
+        let rule = &self.rules[rule_i];
+        if has_compile_error_macro(&rule.rhs) { None } else { Some((&self.name, rule.lhs_span)) }
+    }
 }
 
 struct DummyExpander(ErrorGuaranteed);
@@ -184,9 +194,8 @@ impl<'matcher> Tracker<'matcher> for NoopTracker {
     }
 }
 
-/// Expands the rules based macro defined by `lhses` and `rhses` for a given
-/// input `arg`.
-#[instrument(skip(cx, transparency, arg, lhses, rhses))]
+/// Expands the rules based macro defined by `rules` for a given input `arg`.
+#[instrument(skip(cx, transparency, arg, rules))]
 fn expand_macro<'cx>(
     cx: &'cx mut ExtCtxt<'_>,
     sp: Span,
@@ -195,8 +204,7 @@ fn expand_macro<'cx>(
     name: Ident,
     transparency: Transparency,
     arg: TokenStream,
-    lhses: &[Vec<MatcherLoc>],
-    rhses: &[mbe::TokenTree],
+    rules: &[MacroRule],
 ) -> Box<dyn MacResult + 'cx> {
     let psess = &cx.sess.psess;
     // Macros defined in the current crate have a real node id,
@@ -209,14 +217,14 @@ fn expand_macro<'cx>(
     }
 
     // Track nothing for the best performance.
-    let try_success_result = try_match_macro(psess, name, &arg, lhses, &mut NoopTracker);
+    let try_success_result = try_match_macro(psess, name, &arg, rules, &mut NoopTracker);
 
     match try_success_result {
-        Ok((i, named_matches)) => {
-            let mbe::TokenTree::Delimited(rhs_span, _, ref rhs) = rhses[i] else {
+        Ok((i, rule, named_matches)) => {
+            let mbe::TokenTree::Delimited(rhs_span, _, ref rhs) = rule.rhs else {
                 cx.dcx().span_bug(sp, "malformed macro rhs");
             };
-            let arm_span = rhses[i].span();
+            let arm_span = rule.rhs.span();
 
             // rhs has holes ( `$id` and `$(...)` that need filled)
             let id = cx.current_expansion.id;
@@ -262,7 +270,7 @@ fn expand_macro<'cx>(
         Err(CanRetry::Yes) => {
             // Retry and emit a better error.
             let (span, guar) =
-                diagnostics::failed_to_match_macro(cx.psess(), sp, def_span, name, arg, lhses);
+                diagnostics::failed_to_match_macro(cx.psess(), sp, def_span, name, arg, rules);
             cx.trace_macros_diag();
             DummyResult::any(span, guar)
         }
@@ -278,14 +286,14 @@ pub(super) enum CanRetry {
 /// Try expanding the macro. Returns the index of the successful arm and its named_matches if it was successful,
 /// and nothing if it failed. On failure, it's the callers job to use `track` accordingly to record all errors
 /// correctly.
-#[instrument(level = "debug", skip(psess, arg, lhses, track), fields(tracking = %T::description()))]
+#[instrument(level = "debug", skip(psess, arg, rules, track), fields(tracking = %T::description()))]
 pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>(
     psess: &ParseSess,
     name: Ident,
     arg: &TokenStream,
-    lhses: &'matcher [Vec<MatcherLoc>],
+    rules: &'matcher [MacroRule],
     track: &mut T,
-) -> Result<(usize, NamedMatches), CanRetry> {
+) -> Result<(usize, &'matcher MacroRule, NamedMatches), CanRetry> {
     // We create a base parser that can be used for the "black box" parts.
     // Every iteration needs a fresh copy of that parser. However, the parser
     // is not mutated on many of the iterations, particularly when dealing with
@@ -308,7 +316,7 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>(
     let parser = parser_from_cx(psess, arg.clone(), T::recovery());
     // Try each arm's matchers.
     let mut tt_parser = TtParser::new(name);
-    for (i, lhs) in lhses.iter().enumerate() {
+    for (i, rule) in rules.iter().enumerate() {
         let _tracing_span = trace_span!("Matching arm", %i);
 
         // Take a snapshot of the state of pre-expansion gating at this point.
@@ -317,7 +325,7 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>(
         // are not recorded. On the first `Success(..)`ful matcher, the spans are merged.
         let mut gated_spans_snapshot = mem::take(&mut *psess.gated_spans.spans.borrow_mut());
 
-        let result = tt_parser.parse_tt(&mut Cow::Borrowed(&parser), lhs, track);
+        let result = tt_parser.parse_tt(&mut Cow::Borrowed(&parser), &rule.lhs, track);
 
         track.after_arm(&result);
 
@@ -328,7 +336,7 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>(
                 // Merge the gated spans from parsing the matcher with the preexisting ones.
                 psess.gated_spans.merge(gated_spans_snapshot);
 
-                return Ok((i, named_matches));
+                return Ok((i, rule, named_matches));
             }
             Failure(_) => {
                 trace!("Failed to match arm, trying the next one");
@@ -364,7 +372,7 @@ pub fn compile_declarative_macro(
     span: Span,
     node_id: NodeId,
     edition: Edition,
-) -> (SyntaxExtension, Vec<(usize, Span)>) {
+) -> (SyntaxExtension, usize) {
     let mk_syn_ext = |expander| {
         SyntaxExtension::new(
             sess,
@@ -377,7 +385,7 @@ pub fn compile_declarative_macro(
             node_id != DUMMY_NODE_ID,
         )
     };
-    let dummy_syn_ext = |guar| (mk_syn_ext(Arc::new(DummyExpander(guar))), Vec::new());
+    let dummy_syn_ext = |guar| (mk_syn_ext(Arc::new(DummyExpander(guar))), 0);
 
     let macro_rules = macro_def.macro_rules;
     let exp_sep = if macro_rules { exp!(Semi) } else { exp!(Comma) };
@@ -389,8 +397,7 @@ pub fn compile_declarative_macro(
     let mut guar = None;
     let mut check_emission = |ret: Result<(), ErrorGuaranteed>| guar = guar.or(ret.err());
 
-    let mut lhses = Vec::new();
-    let mut rhses = Vec::new();
+    let mut rules = Vec::new();
 
     while p.token != token::Eof {
         let lhs_tt = p.parse_token_tree();
@@ -415,8 +422,15 @@ pub fn compile_declarative_macro(
         let rhs_tt = parse_one_tt(rhs_tt, RulePart::Body, sess, node_id, features, edition);
         check_emission(check_rhs(sess, &rhs_tt));
         check_emission(macro_check::check_meta_variables(&sess.psess, node_id, &lhs_tt, &rhs_tt));
-        lhses.push(lhs_tt);
-        rhses.push(rhs_tt);
+        let lhs_span = lhs_tt.span();
+        // Convert the lhs into `MatcherLoc` form, which is better for doing the
+        // actual matching.
+        let lhs = if let mbe::TokenTree::Delimited(.., delimited) = lhs_tt {
+            mbe::macro_parser::compute_locs(&delimited.tts)
+        } else {
+            return dummy_syn_ext(guar.unwrap());
+        };
+        rules.push(MacroRule { lhs, lhs_span, rhs: rhs_tt });
         if p.token == token::Eof {
             break;
         }
@@ -425,7 +439,7 @@ pub fn compile_declarative_macro(
         }
     }
 
-    if lhses.is_empty() {
+    if rules.is_empty() {
         let guar = sess.dcx().span_err(span, "macros must contain at least one rule");
         return dummy_syn_ext(guar);
     }
@@ -439,48 +453,12 @@ pub fn compile_declarative_macro(
         return dummy_syn_ext(guar);
     }
 
-    // Compute the spans of the macro rules for unused rule linting.
-    // Also, we are only interested in non-foreign macros.
-    let rule_spans = if node_id != DUMMY_NODE_ID {
-        lhses
-            .iter()
-            .zip(rhses.iter())
-            .enumerate()
-            // If the rhs contains an invocation like compile_error!,
-            // don't consider the rule for the unused rule lint.
-            .filter(|(_idx, (_lhs, rhs))| !has_compile_error_macro(rhs))
-            // We only take the span of the lhs here,
-            // so that the spans of created warnings are smaller.
-            .map(|(idx, (lhs, _rhs))| (idx, lhs.span()))
-            .collect::<Vec<_>>()
-    } else {
-        Vec::new()
-    };
+    // Return the number of rules for unused rule linting, if this is a local macro.
+    let nrules = if node_id != DUMMY_NODE_ID { rules.len() } else { 0 };
 
-    // Convert the lhses into `MatcherLoc` form, which is better for doing the
-    // actual matching.
-    let lhses = lhses
-        .iter()
-        .map(|lhs| {
-            // Ignore the delimiters around the matcher.
-            match lhs {
-                mbe::TokenTree::Delimited(.., delimited) => {
-                    mbe::macro_parser::compute_locs(&delimited.tts)
-                }
-                _ => sess.dcx().span_bug(span, "malformed macro lhs"),
-            }
-        })
-        .collect();
-
-    let expander = Arc::new(MacroRulesMacroExpander {
-        name: ident,
-        span,
-        node_id,
-        transparency,
-        lhses,
-        rhses,
-    });
-    (mk_syn_ext(expander), rule_spans)
+    let expander =
+        Arc::new(MacroRulesMacroExpander { name: ident, span, node_id, transparency, rules });
+    (mk_syn_ext(expander), nrules)
 }
 
 fn check_lhs_nt_follows(

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -1202,12 +1202,8 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
     fn insert_unused_macro(&mut self, ident: Ident, def_id: LocalDefId, node_id: NodeId) {
         if !ident.as_str().starts_with('_') {
             self.r.unused_macros.insert(def_id, (node_id, ident));
-            for (rule_i, rule_span) in &self.r.macro_map[&def_id.to_def_id()].rule_spans {
-                self.r
-                    .unused_macro_rules
-                    .entry(node_id)
-                    .or_default()
-                    .insert(*rule_i, (ident, *rule_span));
+            for rule_i in 0..self.r.macro_map[&def_id.to_def_id()].nrules {
+                self.r.unused_macro_rules.entry(node_id).or_default().insert(rule_i);
             }
         }
     }

compiler/rustc_resolve/src/lib.rs

@@ -1014,13 +1014,13 @@ struct DeriveData {
 
 struct MacroData {
     ext: Arc<SyntaxExtension>,
-    rule_spans: Vec<(usize, Span)>,
+    nrules: usize,
     macro_rules: bool,
 }
 
 impl MacroData {
     fn new(ext: Arc<SyntaxExtension>) -> MacroData {
-        MacroData { ext, rule_spans: Vec::new(), macro_rules: false }
+        MacroData { ext, nrules: 0, macro_rules: false }
     }
 }
 
@@ -1135,7 +1135,7 @@ pub struct Resolver<'ra, 'tcx> {
     ast_transform_scopes: FxHashMap<LocalExpnId, Module<'ra>>,
     unused_macros: FxIndexMap<LocalDefId, (NodeId, Ident)>,
     /// A map from the macro to all its potentially unused arms.
-    unused_macro_rules: FxIndexMap<NodeId, UnordMap<usize, (Ident, Span)>>,
+    unused_macro_rules: FxIndexMap<NodeId, UnordSet<usize>>,
     proc_macro_stubs: FxHashSet<LocalDefId>,
     /// Traces collected during macro resolution and validated when it's complete.
     single_segment_macro_resolutions:

compiler/rustc_resolve/src/macros.rs

@@ -351,13 +351,23 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
         }
 
         for (&node_id, unused_arms) in self.unused_macro_rules.iter() {
-            for (&arm_i, &(ident, rule_span)) in unused_arms.to_sorted_stable_ord() {
-                self.lint_buffer.buffer_lint(
-                    UNUSED_MACRO_RULES,
-                    node_id,
-                    rule_span,
-                    BuiltinLintDiag::MacroRuleNeverUsed(arm_i, ident.name),
-                );
+            if unused_arms.is_empty() {
+                continue;
+            }
+            let def_id = self.local_def_id(node_id).to_def_id();
+            let m = &self.macro_map[&def_id];
+            let SyntaxExtensionKind::LegacyBang(ref ext) = m.ext.kind else {
+                continue;
+            };
+            for &arm_i in unused_arms.to_sorted_stable_ord() {
+                if let Some((ident, rule_span)) = ext.get_unused_rule(arm_i) {
+                    self.lint_buffer.buffer_lint(
+                        UNUSED_MACRO_RULES,
+                        node_id,
+                        rule_span,
+                        BuiltinLintDiag::MacroRuleNeverUsed(arm_i, ident.name),
+                    );
+                }
             }
         }
     }
@@ -1146,7 +1156,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         node_id: NodeId,
         edition: Edition,
     ) -> MacroData {
-        let (mut ext, mut rule_spans) = compile_declarative_macro(
+        let (mut ext, mut nrules) = compile_declarative_macro(
             self.tcx.sess,
             self.tcx.features(),
             macro_def,
@@ -1163,13 +1173,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 // The macro is a built-in, replace its expander function
                 // while still taking everything else from the source code.
                 ext.kind = builtin_ext_kind.clone();
-                rule_spans = Vec::new();
+                nrules = 0;
             } else {
                 self.dcx().emit_err(errors::CannotFindBuiltinMacroWithName { span, ident });
             }
         }
 
-        MacroData { ext: Arc::new(ext), rule_spans, macro_rules: macro_def.macro_rules }
+        MacroData { ext: Arc::new(ext), nrules, macro_rules: macro_def.macro_rules }
     }
 
     fn path_accessible(