From b7b67228f9cd4fab4462ebe4d4d05ad10bf5a7d7 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sat, 12 Nov 2022 22:12:33 +0100 Subject: [PATCH] Only do parser recovery on retried macro matching This prevents issues with eager parser recovery during macro matching. --- compiler/rustc_expand/src/mbe/macro_rules.rs | 35 +++++++++++++++----- compiler/rustc_parse/src/parser/mod.rs | 4 +-- src/test/ui/macros/recovery-allowed.rs | 8 +++++ src/test/ui/macros/recovery-allowed.stderr | 10 ++++++ src/test/ui/macros/recovery-forbidden.rs | 13 ++++++++ 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/macros/recovery-allowed.rs create mode 100644 src/test/ui/macros/recovery-allowed.stderr create mode 100644 src/test/ui/macros/recovery-forbidden.rs diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 182fbe36919..c02680b77fb 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -22,7 +22,7 @@ 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 @@ fn after_arm(&mut self, _: &NamedParseResult) {} fn description() -> &'static str { "none" } + fn recovery() -> Recovery { + Recovery::Forbidden + } } /// Expands the rules based macro defined by `lhses` and `rhses` for a given @@ -330,7 +335,12 @@ 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. @@ -338,7 +348,7 @@ fn expand_macro<'cx>( } 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 @@ fn before_match_loc(&mut self, parser: &TtParser, matcher: &'matcher MatcherLoc) 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 @@ fn after_arm(&mut self, result: &NamedParseResult) { 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 diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 14dc490fb02..13a38a17735 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -503,8 +503,8 @@ pub fn new( parser } - pub fn forbid_recovery(mut self) -> Self { - self.recovery = Recovery::Forbidden; + pub fn recovery(mut self, recovery: Recovery) -> Self { + self.recovery = recovery; self } diff --git a/src/test/ui/macros/recovery-allowed.rs b/src/test/ui/macros/recovery-allowed.rs new file mode 100644 index 00000000000..ebf65f1cc01 --- /dev/null +++ b/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() {} diff --git a/src/test/ui/macros/recovery-allowed.stderr b/src/test/ui/macros/recovery-allowed.stderr new file mode 100644 index 00000000000..ec036e8b1e2 --- /dev/null +++ b/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 + diff --git a/src/test/ui/macros/recovery-forbidden.rs b/src/test/ui/macros/recovery-forbidden.rs new file mode 100644 index 00000000000..5dd2619330c --- /dev/null +++ b/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() {} -- 2.44.0