From ed142028641079918b95b539f0570e92469687fe Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Tue, 25 Oct 2022 21:24:01 +0200 Subject: [PATCH 1/3] Add flag to forbid recovery in the parser --- compiler/rustc_expand/src/mbe/macro_rules.rs | 1 + compiler/rustc_parse/src/parser/expr.rs | 2 ++ compiler/rustc_parse/src/parser/mod.rs | 23 ++++++++++++++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 1e268542bcd91..f6fe38174f7c5 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -250,6 +250,7 @@ fn expand_macro<'cx>( // hacky, but speeds up the `html5ever` benchmark significantly. (Issue // 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()); // Try each arm's matchers. diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index ca216b1cd1008..a781748efc52a 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -2112,6 +2112,8 @@ impl<'a> Parser<'a> { // HACK: This is needed so we can detect whether we're inside a macro, // where regular assumptions about what tokens can follow other tokens // don't necessarily apply. + && self.may_recover() + // FIXME(Nilstrieb): Remove this check once `may_recover` actually stops recovery && self.subparser_name.is_none() { // It is likely that the closure body is a block but where the diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 89c24920f857d..89f7ab930b1a4 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -115,6 +115,12 @@ macro_rules! maybe_recover_from_interpolated_ty_qpath { }; } +#[derive(Clone, Copy)] +pub enum Recovery { + Allowed, + Forbidden, +} + #[derive(Clone)] pub struct Parser<'a> { pub sess: &'a ParseSess, @@ -152,12 +158,15 @@ pub struct Parser<'a> { /// This allows us to recover when the user forget to add braces around /// multiple statements in the closure body. pub current_closure: Option, + /// Whether the parser is allowed to recover and parse invalid code successfully (and emit a diagnostic as a side effect). + /// This is disabled when parsing macro arguments, see #103534 + pub recovery: Recovery, } -// This type is used a lot, e.g. it's cloned when matching many declarative macro rules. Make sure +// This type is used a lot, e.g. it's cloned when matching many declarative macro rules with nonterminals. Make sure // it doesn't unintentionally get bigger. #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(Parser<'_>, 328); +rustc_data_structures::static_assert_size!(Parser<'_>, 336); /// Stores span information about a closure. #[derive(Clone)] @@ -483,6 +492,7 @@ impl<'a> Parser<'a> { inner_attr_ranges: Default::default(), }, current_closure: None, + recovery: Recovery::Allowed, }; // Make parser point to the first token. @@ -491,6 +501,15 @@ impl<'a> Parser<'a> { parser } + pub fn forbid_recovery(mut self) -> Self { + self.recovery = Recovery::Forbidden; + self + } + + fn may_recover(&self) -> bool { + matches!(self.recovery, Recovery::Allowed) + } + pub fn unexpected(&mut self) -> PResult<'a, T> { match self.expect_one_of(&[], &[]) { Err(e) => Err(e), From 796114a5b0c66abbb2527257b8a38c4cda964a66 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Wed, 26 Oct 2022 21:09:28 +0200 Subject: [PATCH 2/3] Add documentation --- compiler/rustc_parse/src/parser/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 89f7ab930b1a4..4376e5832efbc 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -158,7 +158,7 @@ pub struct Parser<'a> { /// This allows us to recover when the user forget to add braces around /// multiple statements in the closure body. pub current_closure: Option, - /// Whether the parser is allowed to recover and parse invalid code successfully (and emit a diagnostic as a side effect). + /// Whether the parser is allowed to do recovery. /// This is disabled when parsing macro arguments, see #103534 pub recovery: Recovery, } @@ -506,6 +506,13 @@ impl<'a> Parser<'a> { self } + /// Whether the parser is allowed to recover from broken code. + /// + /// If this returns false, recovering broken code into valid code (especially if this recovery does lookahead) + /// is not allowed. All recovery done by the parser must be gated behind this check. + /// + /// Technically, this only needs to restruct eager recovery by doing lookahead at more tokens. + /// But making the distinction is very subtle, and simply forbidding all recovery is a lot simpler to uphold. fn may_recover(&self) -> bool { matches!(self.recovery, Recovery::Allowed) } From da407ed38f6bcb79683379d59d18e615d2b8dfaa Mon Sep 17 00:00:00 2001 From: nils <48135649+Nilstrieb@users.noreply.github.com> Date: Wed, 26 Oct 2022 22:06:35 +0200 Subject: [PATCH 3/3] Fix typo Co-authored-by: Esteban Kuber --- compiler/rustc_parse/src/parser/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 4376e5832efbc..5fe29062b85b9 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -511,7 +511,7 @@ impl<'a> Parser<'a> { /// If this returns false, recovering broken code into valid code (especially if this recovery does lookahead) /// is not allowed. All recovery done by the parser must be gated behind this check. /// - /// Technically, this only needs to restruct eager recovery by doing lookahead at more tokens. + /// Technically, this only needs to restrict eager recovery by doing lookahead at more tokens. /// But making the distinction is very subtle, and simply forbidding all recovery is a lot simpler to uphold. fn may_recover(&self) -> bool { matches!(self.recovery, Recovery::Allowed)