fix: Error reporting for expand_legacy_bang (passes tests/ui)

Author: futile

compiler/rustc_expand/src/base.rs

@@ -15,7 +15,7 @@ use rustc_data_structures::sync::{self, Lrc};
 use rustc_errors::{DiagCtxtHandle, ErrorGuaranteed, PResult};
 use rustc_feature::Features;
 use rustc_lint_defs::{BufferedEarlyLint, RegisteredTools};
-use rustc_middle::expand::{CanRetry, TcxMacroExpander};
+use rustc_middle::expand::TcxMacroExpander;
 use rustc_parse::parser::Parser;
 use rustc_parse::MACRO_ARGUMENTS;
 use rustc_session::config::CollapseMacroDebuginfo;
@@ -1084,7 +1084,7 @@ pub trait ResolverExpand {
         &self,
         invoc_id: LocalExpnId,
         current_expansion: LocalExpnId,
-    ) -> Result<(TokenStream, usize), CanRetry>;
+    ) -> Result<(TokenStream, usize), (Span, ErrorGuaranteed)>;
 }
 
 pub trait LintStoreExpand {

compiler/rustc_expand/src/expand.rs

@@ -19,7 +19,6 @@ use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::PResult;
 use rustc_feature::Features;
-use rustc_middle::expand::CanRetry;
 use rustc_middle::ty::TyCtxt;
 use rustc_parse::parser::{
     AttemptLocalParseRecovery, CommaRecoveryMode, ForceCollect, Parser, RecoverColon, RecoverComma,
@@ -33,7 +32,6 @@ use rustc_span::hygiene::SyntaxContext;
 use rustc_span::symbol::{sym, Ident};
 use rustc_span::{ErrorGuaranteed, FileName, LocalExpnId, Span};
 use smallvec::SmallVec;
-use tracing::debug;
 
 use crate::base::*;
 use crate::config::StripUnconfigured;
@@ -401,7 +399,7 @@ pub struct MacroExpander<'a, 'b> {
 pub fn expand_legacy_bang<'tcx>(
     tcx: TyCtxt<'tcx>,
     key: (LocalExpnId, LocalExpnId),
-) -> Result<(&'tcx TokenStream, usize), CanRetry> {
+) -> Result<(&'tcx TokenStream, usize), (Span, ErrorGuaranteed)> {
     let (invoc_id, current_expansion) = key;
     let map = tcx.macro_map.borrow();
     let (arg, span, expander) = map.get(&invoc_id).as_ref().unwrap();
@@ -739,14 +737,10 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                                 is_local,
                             ))
                         }
-                        Err(CanRetry::No(guar)) => {
-                            debug!("Will not retry matching as an error was emitted already");
+                        Err((span, guar)) => {
+                            self.cx.trace_macros_diag();
                             DummyResult::any(span, guar)
                         }
-                        Err(CanRetry::Yes) => {
-                            // Retry and emit a better error.
-                            DummyResult::any_valid(span)
-                        }
                     };
                     let result = if let Some(result) = fragment_kind.make_from(tok_result) {
                         result

compiler/rustc_expand/src/mbe/diagnostics.rs

@@ -3,42 +3,40 @@ use std::borrow::Cow;
 use rustc_ast::token::{self, Token, TokenKind};
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast_pretty::pprust;
-use rustc_errors::{Applicability, Diag, DiagMessage};
+use rustc_errors::{Applicability, Diag, DiagCtxtHandle, DiagMessage};
 use rustc_macros::Subdiagnostic;
 use rustc_parse::parser::{Parser, Recovery};
+use rustc_session::parse::ParseSess;
 use rustc_span::source_map::SourceMap;
 use rustc_span::symbol::Ident;
 use rustc_span::{ErrorGuaranteed, Span};
 use tracing::debug;
 
 use super::macro_rules::{parser_from_cx, NoopTracker};
-use crate::base::{DummyResult, ExtCtxt, MacResult};
 use crate::expand::{parse_ast_fragment, AstFragmentKind};
 use crate::mbe::macro_parser::ParseResult::*;
 use crate::mbe::macro_parser::{MatcherLoc, NamedParseResult, TtParser};
 use crate::mbe::macro_rules::{try_match_macro, Tracker};
 
-pub(crate) fn failed_to_match_macro<'cx>(
-    cx: &'cx mut ExtCtxt<'_>,
+pub(crate) fn failed_to_match_macro(
+    psess: &ParseSess,
     sp: Span,
     def_span: Span,
     name: Ident,
     arg: TokenStream,
     lhses: &[Vec<MatcherLoc>],
-) -> Box<dyn MacResult + 'cx> {
-    let psess = &cx.sess.psess;
-
+) -> (Span, ErrorGuaranteed) {
     // An error occurred, try the expansion again, tracking the expansion closely for better
     // diagnostics.
-    let mut tracker = CollectTrackerAndEmitter::new(cx, sp);
+    let mut tracker = CollectTrackerAndEmitter::new(psess.dcx(), sp);
 
     let try_success_result = try_match_macro(psess, name, &arg, lhses, &mut tracker);
 
     if try_success_result.is_ok() {
         // Nonterminal parser recovery might turn failed matches into successful ones,
         // but for that it must have emitted an error already
         assert!(
-            tracker.cx.dcx().has_errors().is_some(),
+            tracker.dcx.has_errors().is_some(),
             "Macro matching returned a success on the second try"
         );
     }
@@ -50,15 +48,15 @@ pub(crate) fn failed_to_match_macro<'cx>(
 
     let Some(BestFailure { token, msg: label, remaining_matcher, .. }) = tracker.best_failure
     else {
-        return DummyResult::any(sp, cx.dcx().span_delayed_bug(sp, "failed to match a macro"));
+        return (sp, psess.dcx().span_delayed_bug(sp, "failed to match a macro"));
     };
 
     let span = token.span.substitute_dummy(sp);
 
-    let mut err = cx.dcx().struct_span_err(span, parse_failure_msg(&token, None));
+    let mut err = psess.dcx().struct_span_err(span, parse_failure_msg(&token, None));
     err.span_label(span, label);
-    if !def_span.is_dummy() && !cx.source_map().is_imported(def_span) {
-        err.span_label(cx.source_map().guess_head_span(def_span), "when calling this macro");
+    if !def_span.is_dummy() && !psess.source_map().is_imported(def_span) {
+        err.span_label(psess.source_map().guess_head_span(def_span), "when calling this macro");
     }
 
     annotate_doc_comment(&mut err, psess.source_map(), span);
@@ -76,7 +74,7 @@ pub(crate) fn failed_to_match_macro<'cx>(
         err.note("captured metavariables except for `:tt`, `:ident` and `:lifetime` cannot be compared to other tokens");
         err.note("see <https://doc.rust-lang.org/nightly/reference/macros-by-example.html#forwarding-a-matched-fragment> for more information");
 
-        if !def_span.is_dummy() && !cx.source_map().is_imported(def_span) {
+        if !def_span.is_dummy() && !psess.source_map().is_imported(def_span) {
             err.help("try using `:tt` instead in the macro definition");
         }
     }
@@ -104,18 +102,17 @@ pub(crate) fn failed_to_match_macro<'cx>(
         }
     }
     let guar = err.emit();
-    cx.trace_macros_diag();
-    DummyResult::any(sp, guar)
+    (sp, guar)
 }
 
 /// The tracker used for the slow error path that collects useful info for diagnostics.
-struct CollectTrackerAndEmitter<'a, 'cx, 'matcher> {
-    cx: &'a mut ExtCtxt<'cx>,
+struct CollectTrackerAndEmitter<'dcx, 'matcher> {
+    dcx: DiagCtxtHandle<'dcx>,
     remaining_matcher: Option<&'matcher MatcherLoc>,
     /// Which arm's failure should we report? (the one furthest along)
     best_failure: Option<BestFailure>,
     root_span: Span,
-    result: Option<Box<dyn MacResult + 'cx>>,
+    result: Option<(Span, ErrorGuaranteed)>,
 }
 
 struct BestFailure {
@@ -131,7 +128,7 @@ impl BestFailure {
     }
 }
 
-impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx, 'matcher> {
+impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'matcher> {
     type Failure = (Token, u32, &'static str);
 
     fn build_failure(tok: Token, position: u32, msg: &'static str) -> Self::Failure {
@@ -151,7 +148,7 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx,
             Success(_) => {
                 // Nonterminal parser recovery might turn failed matches into successful ones,
                 // but for that it must have emitted an error already
-                self.cx.dcx().span_delayed_bug(
+                self.dcx.span_delayed_bug(
                     self.root_span,
                     "should not collect detailed info for successful macro match",
                 );
@@ -177,10 +174,10 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx,
             }
             Error(err_sp, msg) => {
                 let span = err_sp.substitute_dummy(self.root_span);
-                let guar = self.cx.dcx().span_err(span, msg.clone());
-                self.result = Some(DummyResult::any(span, guar));
+                let guar = self.dcx.span_err(span, msg.clone());
+                self.result = Some((span, guar));
             }
-            ErrorReported(guar) => self.result = Some(DummyResult::any(self.root_span, *guar)),
+            ErrorReported(guar) => self.result = Some((self.root_span, *guar)),
         }
     }
 
@@ -193,9 +190,9 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx,
     }
 }
 
-impl<'a, 'cx> CollectTrackerAndEmitter<'a, 'cx, '_> {
-    fn new(cx: &'a mut ExtCtxt<'cx>, root_span: Span) -> Self {
-        Self { cx, remaining_matcher: None, best_failure: None, root_span, result: None }
+impl<'dcx> CollectTrackerAndEmitter<'dcx, '_> {
+    fn new(dcx: DiagCtxtHandle<'dcx>, root_span: Span) -> Self {
+        Self { dcx, remaining_matcher: None, best_failure: None, root_span, result: None }
     }
 }
 

compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -18,7 +18,7 @@ use rustc_lint_defs::builtin::{
     RUST_2021_INCOMPATIBLE_OR_PATTERNS, SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
 };
 use rustc_lint_defs::BuiltinLintDiag;
-use rustc_middle::expand::{CanRetry, TcxMacroExpander};
+use rustc_middle::expand::TcxMacroExpander;
 use rustc_parse::parser::{ParseNtResult, Parser, Recovery};
 use rustc_session::parse::ParseSess;
 use rustc_session::Session;
@@ -149,7 +149,7 @@ impl TcxMacroExpander for MacroRulesMacroExpander {
         sp: Span,
         input: TokenStream,
         expand_id: LocalExpnId,
-    ) -> Result<(TokenStream, usize), CanRetry> {
+    ) -> Result<(TokenStream, usize), (Span, ErrorGuaranteed)> {
         // Track nothing for the best performance.
         let try_success_result =
             try_match_macro(&sess.psess, self.name, &input, &self.lhses, &mut NoopTracker);
@@ -171,10 +171,24 @@ impl TcxMacroExpander for MacroRulesMacroExpander {
                     expand_id,
                 ) {
                     Ok(tts) => Ok((tts, i)),
-                    Err(err) => Err(CanRetry::No(err.emit())),
+                    Err(err) => {
+                        let arm_span = self.rhses[i].span();
+                        Err((arm_span, err.emit()))
+                    }
                 }
             }
-            Err(e) => Err(e),
+            Err(CanRetry::No(guar)) => Err((sp, guar)),
+            Err(CanRetry::Yes) => {
+                // Retry and emit a better error.
+                Err(crate::mbe::diagnostics::failed_to_match_macro(
+                    &sess.psess,
+                    sp,
+                    self.span,
+                    self.name,
+                    input,
+                    &self.lhses,
+                ))
+            }
         }
     }
 
@@ -333,11 +347,20 @@ fn expand_macro<'cx>(
         }
         Err(CanRetry::Yes) => {
             // Retry and emit a better error.
-            diagnostics::failed_to_match_macro(cx, sp, def_span, name, arg, lhses)
+            let (span, guar) =
+                diagnostics::failed_to_match_macro(cx.psess(), sp, def_span, name, arg, lhses);
+            cx.trace_macros_diag();
+            DummyResult::any(span, guar)
         }
     }
 }
 
+pub(super) enum CanRetry {
+    Yes,
+    /// We are not allowed to retry macro expansion as a fatal error has been emitted already.
+    No(ErrorGuaranteed),
+}
+
 /// 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.

compiler/rustc_middle/src/expand.rs

@@ -1,6 +1,5 @@
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::NodeId;
-use rustc_macros::HashStable_Generic;
 use rustc_session::Session;
 use rustc_span::symbol::Ident;
 use rustc_span::{ErrorGuaranteed, LocalExpnId, Span};
@@ -12,18 +11,11 @@ pub trait TcxMacroExpander {
         _span: Span,
         _input: TokenStream,
         _expand_id: LocalExpnId,
-    ) -> Result<(TokenStream, usize), CanRetry>;
+    ) -> Result<(TokenStream, usize), (Span, ErrorGuaranteed)>;
 
     fn name(&self) -> Ident;
 
     fn arm_span(&self, rhs: usize) -> Span;
 
     fn node_id(&self) -> NodeId;
 }
-
-#[derive(Copy, Clone, HashStable_Generic, Debug)]
-pub enum CanRetry {
-    Yes,
-    /// We are not allowed to retry macro expansion as a fatal error has been emitted already.
-    No(ErrorGuaranteed),
-}

compiler/rustc_middle/src/lib.rs

@@ -92,8 +92,6 @@ pub mod query;
 #[macro_use]
 pub mod dep_graph;
 
-use rustc_span::HashStableContext;
-
 // Allows macros to refer to this crate as `::rustc_middle`
 extern crate self as rustc_middle;
 

compiler/rustc_middle/src/query/erase.rs

@@ -2,7 +2,7 @@ use std::intrinsics::transmute_unchecked;
 use std::mem::MaybeUninit;
 
 use rustc_ast::tokenstream::TokenStream;
-use rustc_middle::expand::CanRetry;
+use rustc_span::{ErrorGuaranteed, Span};
 
 use crate::query::CyclePlaceholder;
 use crate::ty::adjustment::CoerceUnsizedInfo;
@@ -175,8 +175,8 @@ impl EraseType for Result<ty::EarlyBinder<'_, Ty<'_>>, CyclePlaceholder> {
     type Result = [u8; size_of::<Result<ty::EarlyBinder<'static, Ty<'_>>, CyclePlaceholder>>()];
 }
 
-impl EraseType for Result<(&'_ TokenStream, usize), CanRetry> {
-    type Result = [u8; size_of::<Result<(&'_ TokenStream, usize), CanRetry>>()];
+impl EraseType for Result<(&'_ TokenStream, usize), (Span, ErrorGuaranteed)> {
+    type Result = [u8; size_of::<Result<(&'_ TokenStream, usize), (Span, ErrorGuaranteed)>>()];
 }
 
 impl<T> EraseType for Option<&'_ T> {

compiler/rustc_middle/src/query/mod.rs

@@ -45,7 +45,6 @@ use rustc_target::abi;
 use rustc_target::spec::PanicStrategy;
 use {rustc_ast as ast, rustc_attr as attr, rustc_hir as hir};
 
-use crate::expand::CanRetry;
 use crate::infer::canonical::{self, Canonical};
 use crate::lint::LintExpectation;
 use crate::metadata::ModChild;
@@ -111,7 +110,7 @@ rustc_queries! {
         desc { "triggering a delayed bug for testing incremental" }
     }
 
-    query expand_legacy_bang(key: (LocalExpnId, LocalExpnId)) -> Result<(&'tcx TokenStream, usize), CanRetry> {
+    query expand_legacy_bang(key: (LocalExpnId, LocalExpnId)) -> Result<(&'tcx TokenStream, usize), (Span, ErrorGuaranteed)> {
         eval_always
         no_hash
         desc { "expand legacy bang" }

compiler/rustc_resolve/src/macros.rs

@@ -22,7 +22,6 @@ use rustc_expand::expand::{
 };
 use rustc_hir::def::{self, DefKind, Namespace, NonMacroAttrKind};
 use rustc_hir::def_id::{CrateNum, DefId, LocalDefId};
-use rustc_middle::expand::CanRetry;
 use rustc_middle::middle::stability;
 use rustc_middle::ty::{RegisteredTools, TyCtxt, Visibility};
 use rustc_session::lint::builtin::{
@@ -35,7 +34,7 @@ use rustc_span::edit_distance::edit_distance;
 use rustc_span::edition::Edition;
 use rustc_span::hygiene::{self, AstPass, ExpnData, ExpnKind, LocalExpnId, MacroKind};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
-use rustc_span::{Span, DUMMY_SP};
+use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP};
 
 use crate::errors::{
     self, AddAsNonDerive, CannotDetermineMacroResolution, CannotFindIdentInThisScope,
@@ -539,7 +538,7 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
         &self,
         invoc_id: LocalExpnId,
         current_expansion: LocalExpnId,
-    ) -> Result<(TokenStream, usize), CanRetry> {
+    ) -> Result<(TokenStream, usize), (Span, ErrorGuaranteed)> {
         self.tcx()
             .expand_legacy_bang((invoc_id, current_expansion))
             .map(|(tts, i)| (tts.clone(), i))