Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only do parser recovery on retried macro matching #104335

Merged
merged 1 commit into from
Nov 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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(_) =
Expand Down Expand Up @@ -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() => {}
Expand All @@ -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, '_> {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/macros/recovery-allowed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
macro_rules! please_recover {
($a:expr) => {};
}

please_recover! { not 1 }
//~^ ERROR unexpected `1` after identifier

fn main() {}
10 changes: 10 additions & 0 deletions src/test/ui/macros/recovery-allowed.stderr
Original file line number Diff line number Diff line change
@@ -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

13 changes: 13 additions & 0 deletions src/test/ui/macros/recovery-forbidden.rs
Original file line number Diff line number Diff line change
@@ -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() {}