Rollup merge of rust-lang#104335 - Nilstrieb:macrowo, r=compiler-errors

Only do parser recovery on retried macro matching

Eager parser recovery can break macros, so we don't do it at first. But when we already know that the macro failed, we can retry it with recovery enabled to still emit useful diagnostics.

Helps with rust-lang#103534

Author: matthiaskrgr

compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -22,7 +22,7 @@ use rustc_lint_defs::builtin::{
     RUST_2021_INCOMPATIBLE_OR_PATTERNS, SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
 };
 use rustc_lint_defs::BuiltinLintDiagnostics;
-use rustc_parse::parser::Parser;
+use rustc_parse::parser::{Parser, Recovery};
 use rustc_session::parse::ParseSess;
 use rustc_session::Session;
 use rustc_span::edition::Edition;
@@ -219,6 +219,8 @@ pub(super) trait Tracker<'matcher> {
 
     /// For tracing.
     fn description() -> &'static str;
+
+    fn recovery() -> Recovery;
 }
 
 /// A noop tracker that is used in the hot path of the expansion, has zero overhead thanks to monomorphization.
@@ -230,6 +232,9 @@ impl<'matcher> Tracker<'matcher> for NoopTracker {
     fn description() -> &'static str {
         "none"
     }
+    fn recovery() -> Recovery {
+        Recovery::Forbidden
+    }
 }
 
 /// Expands the rules based macro defined by `lhses` and `rhses` for a given
@@ -330,15 +335,20 @@ fn expand_macro<'cx>(
     let mut tracker = CollectTrackerAndEmitter::new(cx, sp);
 
     let try_success_result = try_match_macro(sess, name, &arg, lhses, &mut tracker);
-    assert!(try_success_result.is_err(), "Macro matching returned a success on the second try");
+
+    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
+        tracker.cx.sess.delay_span_bug(sp, "Macro matching returned a success on the second try");
+    }
 
     if let Some(result) = tracker.result {
         // An irrecoverable error occurred and has been emitted.
         return result;
     }
 
     let Some((token, label, remaining_matcher)) = tracker.best_failure else {
-        return tracker.result.expect("must have encountered Error or ErrorReported");
+        return DummyResult::any(sp);
     };
 
     let span = token.span.substitute_dummy(sp);
@@ -360,7 +370,7 @@ fn expand_macro<'cx>(
     // Check whether there's a missing comma in this macro call, like `println!("{}" a);`
     if let Some((arg, comma_span)) = arg.add_comma() {
         for lhs in lhses {
-            let parser = parser_from_cx(sess, arg.clone());
+            let parser = parser_from_cx(sess, arg.clone(), Recovery::Allowed);
             let mut tt_parser = TtParser::new(name);
 
             if let Success(_) =
@@ -406,7 +416,12 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx,
     fn after_arm(&mut self, result: &NamedParseResult) {
         match result {
             Success(_) => {
-                unreachable!("should not collect detailed info for successful macro match");
+                // Nonterminal parser recovery might turn failed matches into successful ones,
+                // but for that it must have emitted an error already
+                self.cx.sess.delay_span_bug(
+                    self.root_span,
+                    "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() => {}
@@ -432,6 +447,10 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx,
     fn description() -> &'static str {
         "detailed"
     }
+
+    fn recovery() -> Recovery {
+        Recovery::Allowed
+    }
 }
 
 impl<'a, 'cx> CollectTrackerAndEmitter<'a, 'cx, '_> {
@@ -477,7 +496,7 @@ fn try_match_macro<'matcher, T: Tracker<'matcher>>(
     // 68836 suggests a more comprehensive but more complex change to deal with
     // this situation.)
     // FIXME(Nilstrieb): Stop recovery from happening on this parser and retry later with recovery if the macro failed to match.
-    let parser = parser_from_cx(sess, arg.clone());
+    let parser = parser_from_cx(sess, arg.clone(), T::recovery());
     // Try each arm's matchers.
     let mut tt_parser = TtParser::new(name);
     for (i, lhs) in lhses.iter().enumerate() {
@@ -1559,8 +1578,8 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String {
     }
 }
 
-fn parser_from_cx(sess: &ParseSess, tts: TokenStream) -> Parser<'_> {
-    Parser::new(sess, tts, true, rustc_parse::MACRO_ARGUMENTS)
+fn parser_from_cx(sess: &ParseSess, tts: TokenStream, recovery: Recovery) -> Parser<'_> {
+    Parser::new(sess, tts, true, rustc_parse::MACRO_ARGUMENTS).recovery(recovery)
 }
 
 /// Generates an appropriate parsing failure message. For EOF, this is "unexpected end...". For

compiler/rustc_parse/src/parser/mod.rs

@@ -503,8 +503,8 @@ impl<'a> Parser<'a> {
         parser
     }
 
-    pub fn forbid_recovery(mut self) -> Self {
-        self.recovery = Recovery::Forbidden;
+    pub fn recovery(mut self, recovery: Recovery) -> Self {
+        self.recovery = recovery;
         self
     }
 

src/test/ui/macros/recovery-allowed.rs

@@ -0,0 +1,8 @@
+macro_rules! please_recover {
+    ($a:expr) => {};
+}
+
+please_recover! { not 1 }
+//~^ ERROR unexpected `1` after identifier
+
+fn main() {}

src/test/ui/macros/recovery-allowed.stderr

@@ -0,0 +1,10 @@
+error: unexpected `1` after identifier
+  --> $DIR/recovery-allowed.rs:5:23
+   |
+LL | please_recover! { not 1 }
+   |                   ----^
+   |                   |
+   |                   help: use `!` to perform bitwise not
+
+error: aborting due to previous error
+

src/test/ui/macros/recovery-forbidden.rs

@@ -0,0 +1,13 @@
+// check-pass
+
+macro_rules! dont_recover_here {
+    ($e:expr) => {
+        compile_error!("Must not recover to single !1 expr");
+    };
+
+    (not $a:literal) => {};
+}
+
+dont_recover_here! { not 1 }
+
+fn main() {}