about summary refs log tree commit diff
diff options
context:
space:
mode:
authorNilstrieb <48135649+Nilstrieb@users.noreply.github.com>2022-12-11 21:46:30 +0100
committerNilstrieb <48135649+Nilstrieb@users.noreply.github.com>2022-12-12 17:05:27 +0100
commitd72a0c437bd2db922b954af7b0278e1f4bf31edf (patch)
tree64c689e5201e810e2bcc3fa2a5482dc73dd49e15
parent32da2305880765a4c76180086959a2d5da131565 (diff)
downloadrust-d72a0c437bd2db922b954af7b0278e1f4bf31edf.tar.gz
rust-d72a0c437bd2db922b954af7b0278e1f4bf31edf.zip
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.
-rw-r--r--compiler/rustc_expand/src/mbe/diagnostics.rs40
-rw-r--r--compiler/rustc_expand/src/mbe/macro_parser.rs13
-rw-r--r--compiler/rustc_expand/src/mbe/macro_rules.rs6
-rw-r--r--compiler/rustc_parse/src/parser/mod.rs4
-rw-r--r--src/test/ui/macros/best-failure.rs11
-rw-r--r--src/test/ui/macros/best-failure.stderr21
6 files changed, 80 insertions, 15 deletions
diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs
index 197f056917f..40aa64d9d40 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<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();
diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs
index d161868edce..df1c1834c1d 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<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",
                     );
                 }
diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs
index 2dbb90e2190..b4001a497af 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 bebb012660a..008388e3da3 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 00000000000..bbdd465d5ec
--- /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 00000000000..a52fc5e3da6
--- /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
+