Rollup merge of #105570 - Nilstrieb:actual-best-failure, r=compiler-errors

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.

This needs a little cleanup as the `best_failure` field is getting out of hand but it should be mostly good to go. I hope I didn't violate too many abstraction boundaries..

Author: matthiaskrgr

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<BestFailure>,
     root_span: Span,
     result: Option<Box<dyn MacResult + 'cx>>,
 }
 
+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();

compiler/rustc_expand/src/mbe/macro_parser.rs

@@ -310,7 +310,8 @@ pub(crate) enum ParseResult<T> {
     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<NamedParseResult> {
         // 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",
                     );
                 }

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);

compiler/rustc_parse/src/parser/mod.rs

@@ -1499,6 +1499,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(

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() {}

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
+