Skip to content

Commit

Permalink
fix: Error reporting for expand_legacy_bang (passes tests/ui)
Browse files Browse the repository at this point in the history
  • Loading branch information
futile committed Aug 3, 2024
1 parent 1763388 commit 3ffaf37
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 62 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
51 changes: 24 additions & 27 deletions compiler/rustc_expand/src/mbe/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
Expand All @@ -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);
Expand All @@ -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");
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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",
);
Expand All @@ -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)),
}
}

Expand All @@ -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 }
}
}

Expand Down
33 changes: 28 additions & 5 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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,
))
}
}
}

Expand Down Expand Up @@ -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.
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_middle/src/expand.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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),
}
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/query/erase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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> {
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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" }
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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,
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit 3ffaf37

Please sign in to comment.