From d72a0c437bd2db922b954af7b0278e1f4bf31edf Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 11 Dec 2022 21:46:30 +0100 Subject: [PATCH] Properly calculate best failure in macro matching Previously, we used spans. This was not good. Sometimes, the span of the token that failed to match may come from a position later in the file which has been transcribed into a token stream way earlier in the file. If precisely this token fails to match, we think that it was the best match because its span is so high, even though other arms might have gotten further in the token stream. We now try to properly use the location in the token stream. --- compiler/rustc_expand/src/mbe/diagnostics.rs | 40 ++++++++++++++----- compiler/rustc_expand/src/mbe/macro_parser.rs | 13 +++++- compiler/rustc_expand/src/mbe/macro_rules.rs | 6 +-- compiler/rustc_parse/src/parser/mod.rs | 4 ++ src/test/ui/macros/best-failure.rs | 11 +++++ src/test/ui/macros/best-failure.stderr | 21 ++++++++++ 6 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/macros/best-failure.rs create mode 100644 src/test/ui/macros/best-failure.stderr diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index 197f056917f5d..40aa64d9d4040 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -43,7 +43,7 @@ pub(super) fn failed_to_match_macro<'cx>( return result; } - let Some((token, label, remaining_matcher)) = tracker.best_failure else { + let Some(BestFailure { token, msg: label, remaining_matcher, .. }) = tracker.best_failure else { return DummyResult::any(sp); }; @@ -95,11 +95,24 @@ struct CollectTrackerAndEmitter<'a, 'cx, 'matcher> { cx: &'a mut ExtCtxt<'cx>, remaining_matcher: Option<&'matcher MatcherLoc>, /// Which arm's failure should we report? (the one furthest along) - best_failure: Option<(Token, &'static str, MatcherLoc)>, + best_failure: Option, root_span: Span, result: Option>, } +struct BestFailure { + token: Token, + position_in_tokenstream: usize, + msg: &'static str, + remaining_matcher: MatcherLoc, +} + +impl BestFailure { + fn is_better_position(&self, position: usize) -> bool { + position > self.position_in_tokenstream + } +} + impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx, 'matcher> { fn before_match_loc(&mut self, parser: &TtParser, matcher: &'matcher MatcherLoc) { if self.remaining_matcher.is_none() @@ -119,18 +132,25 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx, "should not collect detailed info for successful macro match", ); } - Failure(token, msg) => match self.best_failure { - Some((ref best_token, _, _)) if best_token.span.lo() >= token.span.lo() => {} - _ => { - self.best_failure = Some(( - token.clone(), + Failure(token, approx_position, msg) => { + debug!(?token, ?msg, "a new failure of an arm"); + + if self + .best_failure + .as_ref() + .map_or(true, |failure| failure.is_better_position(*approx_position)) + { + self.best_failure = Some(BestFailure { + token: token.clone(), + position_in_tokenstream: *approx_position, msg, - self.remaining_matcher + remaining_matcher: self + .remaining_matcher .expect("must have collected matcher already") .clone(), - )) + }) } - }, + } Error(err_sp, msg) => { let span = err_sp.substitute_dummy(self.root_span); self.cx.struct_span_err(span, msg).emit(); diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index d161868edce77..df1c1834c1dc0 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -310,7 +310,8 @@ pub(crate) enum ParseResult { Success(T), /// Arm failed to match. If the second parameter is `token::Eof`, it indicates an unexpected /// end of macro invocation. Otherwise, it indicates that no rules expected the given token. - Failure(Token, &'static str), + /// The usize is the approximate position of the token in the input token stream. + Failure(Token, usize, &'static str), /// Fatal error (malformed macro?). Abort compilation. Error(rustc_span::Span, String), ErrorReported(ErrorGuaranteed), @@ -455,6 +456,7 @@ impl TtParser { &mut self, matcher: &'matcher [MatcherLoc], token: &Token, + approx_position: usize, track: &mut T, ) -> Option { // Matcher positions that would be valid if the macro invocation was over now. Only @@ -598,6 +600,7 @@ impl TtParser { token::Eof, if token.span.is_dummy() { token.span } else { token.span.shrink_to_hi() }, ), + approx_position, "missing tokens in macro arguments", ), }) @@ -627,7 +630,12 @@ 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. - let res = self.parse_tt_inner(matcher, &parser.token, track); + let res = self.parse_tt_inner( + matcher, + &parser.token, + parser.approx_token_stream_pos(), + track, + ); if let Some(res) = res { return res; } @@ -642,6 +650,7 @@ impl TtParser { // parser: syntax error. return Failure( parser.token.clone(), + parser.approx_token_stream_pos(), "no rules expected this token in macro call", ); } diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 2dbb90e2190f0..b4001a497afa8 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -326,8 +326,8 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>( return Ok((i, named_matches)); } - Failure(_, _) => { - trace!("Failed to match arm, trying the next one"); + Failure(_, reached_position, _) => { + trace!(%reached_position, "Failed to match arm, trying the next one"); // Try the next arm. } Error(_, _) => { @@ -432,7 +432,7 @@ pub fn compile_declarative_macro( let argument_map = match tt_parser.parse_tt(&mut Cow::Owned(parser), &argument_gram, &mut NoopTracker) { Success(m) => m, - Failure(token, msg) => { + Failure(token, _, msg) => { let s = parse_failure_msg(&token); let sp = token.span.substitute_dummy(def.span); let mut err = sess.parse_sess.span_diagnostic.struct_span_err(sp, &s); diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index bebb012660a16..008388e3da340 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1503,6 +1503,10 @@ impl<'a> Parser<'a> { pub fn clear_expected_tokens(&mut self) { self.expected_tokens.clear(); } + + pub fn approx_token_stream_pos(&self) -> usize { + self.token_cursor.num_next_calls + } } pub(crate) fn make_unclosed_delims_error( diff --git a/src/test/ui/macros/best-failure.rs b/src/test/ui/macros/best-failure.rs new file mode 100644 index 0000000000000..bbdd465d5ec96 --- /dev/null +++ b/src/test/ui/macros/best-failure.rs @@ -0,0 +1,11 @@ +macro_rules! number { + (neg false, $self:ident) => { $self }; + ($signed:tt => $ty:ty;) => { + number!(neg $signed, $self); + //~^ ERROR no rules expected the token `$` + }; +} + +number! { false => u8; } + +fn main() {} diff --git a/src/test/ui/macros/best-failure.stderr b/src/test/ui/macros/best-failure.stderr new file mode 100644 index 0000000000000..a52fc5e3da6a5 --- /dev/null +++ b/src/test/ui/macros/best-failure.stderr @@ -0,0 +1,21 @@ +error: no rules expected the token `$` + --> $DIR/best-failure.rs:4:30 + | +LL | macro_rules! number { + | ------------------- when calling this macro +... +LL | number!(neg $signed, $self); + | ^^^^^ no rules expected this token in macro call +... +LL | number! { false => u8; } + | ------------------------ in this macro invocation + | +note: while trying to match meta-variable `$self:ident` + --> $DIR/best-failure.rs:2:17 + | +LL | (neg false, $self:ident) => { $self }; + | ^^^^^^^^^^^ + = note: this error originates in the macro `number` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to previous error +