Skip to content

Commit

Permalink
Auto merge of #117565 - estebank:issue-100825, r=Nilstrieb
Browse files Browse the repository at this point in the history 
Tweak parsing recovery of enums, for exprs and match arm patterns

Tweak recovery of `for (pat in expr) {}` for more accurate spans.

When encountering `match` arm `(pat if expr) => {}`, recover and suggest removing parentheses. Fix #100825.

When encountering malformed enums, try more localized per-variant parse recovery.

Move parser recovery tests to subdirectory.
  • Loading branch information
@bors
bors committed Nov 30, 2023
2 parents c9c760f + 88453aa commit 74fccd0
Show file tree
Hide file tree
Showing 24 changed files with 348 additions and 218 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_parse/messages.ftl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,9 @@ parse_unexpected_lifetime_in_pattern = unexpected lifetime `{$symbol}` in patter
parse_unexpected_parentheses_in_for_head = unexpected parentheses surrounding `for` loop head
.suggestion = remove parentheses in `for` loop
parse_unexpected_parentheses_in_match_arm_pattern = unexpected parentheses surrounding `match` arm pattern
.suggestion = remove parentheses surrounding the pattern
parse_unexpected_self_in_generic_parameters = unexpected keyword `Self` in generic parameters
.note = you cannot use `Self` as a generic parameter because it is reserved for associated items
Expand Down
24 changes: 20 additions & 4 deletions compiler/rustc_parse/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1275,12 +1275,28 @@ pub(crate) struct ParenthesesInForHead {
#[derive(Subdiagnostic)]
#[multipart_suggestion(parse_suggestion, applicability = "machine-applicable")]
pub(crate) struct ParenthesesInForHeadSugg {
#[suggestion_part(code = "{left_snippet}")]
#[suggestion_part(code = " ")]
pub left: Span,
#[suggestion_part(code = " ")]
pub right: Span,
}

#[derive(Diagnostic)]
#[diag(parse_unexpected_parentheses_in_match_arm_pattern)]
pub(crate) struct ParenthesesInMatchPat {
#[primary_span]
pub span: Vec<Span>,
#[subdiagnostic]
pub sugg: ParenthesesInMatchPatSugg,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(parse_suggestion, applicability = "machine-applicable")]
pub(crate) struct ParenthesesInMatchPatSugg {
#[suggestion_part(code = "")]
pub left: Span,
pub left_snippet: String,
#[suggestion_part(code = "{right_snippet}")]
#[suggestion_part(code = "")]
pub right: Span,
pub right_snippet: String,
}

#[derive(Diagnostic)]
Expand Down
62 changes: 6 additions & 56 deletions compiler/rustc_parse/src/parser/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ use crate::errors::{
DoubleColonInBound, ExpectedIdentifier, ExpectedSemi, ExpectedSemiSugg,
GenericParamsWithoutAngleBrackets, GenericParamsWithoutAngleBracketsSugg,
HelpIdentifierStartsWithNumber, InInTypo, IncorrectAwait, IncorrectSemicolon,
IncorrectUseOfAwait, ParenthesesInForHead, ParenthesesInForHeadSugg,
PatternMethodParamWithoutBody, QuestionMarkInType, QuestionMarkInTypeSugg, SelfParamNotFirst,
StructLiteralBodyWithoutPath, StructLiteralBodyWithoutPathSugg, StructLiteralNeedingParens,
StructLiteralNeedingParensSugg, SuggAddMissingLetStmt, SuggEscapeIdentifier, SuggRemoveComma,
TernaryOperator, UnexpectedConstInGenericParam, UnexpectedConstParamDeclaration,
UnexpectedConstParamDeclarationSugg, UnmatchedAngleBrackets, UseEqInstead, WrapType,
IncorrectUseOfAwait, PatternMethodParamWithoutBody, QuestionMarkInType, QuestionMarkInTypeSugg,
SelfParamNotFirst, StructLiteralBodyWithoutPath, StructLiteralBodyWithoutPathSugg,
StructLiteralNeedingParens, StructLiteralNeedingParensSugg, SuggAddMissingLetStmt,
SuggEscapeIdentifier, SuggRemoveComma, TernaryOperator, UnexpectedConstInGenericParam,
UnexpectedConstParamDeclaration, UnexpectedConstParamDeclarationSugg, UnmatchedAngleBrackets,
UseEqInstead, WrapType,
};

use crate::fluent_generated as fluent;
Expand Down Expand Up @@ -1994,56 +1994,6 @@ impl<'a> Parser<'a> {
}
}

/// Recovers a situation like `for ( $pat in $expr )`
/// and suggest writing `for $pat in $expr` instead.
///
/// This should be called before parsing the `$block`.
pub(super) fn recover_parens_around_for_head(
&mut self,
pat: P<Pat>,
begin_paren: Option<Span>,
) -> P<Pat> {
match (&self.token.kind, begin_paren) {
(token::CloseDelim(Delimiter::Parenthesis), Some(begin_par_sp)) => {
self.bump();

let sm = self.sess.source_map();
let left = begin_par_sp;
let right = self.prev_token.span;
let left_snippet = if let Ok(snip) = sm.span_to_prev_source(left)
&& !snip.ends_with(' ')
{
" ".to_string()
} else {
"".to_string()
};

let right_snippet = if let Ok(snip) = sm.span_to_next_source(right)
&& !snip.starts_with(' ')
{
" ".to_string()
} else {
"".to_string()
};

self.sess.emit_err(ParenthesesInForHead {
span: vec![left, right],
// With e.g. `for (x) in y)` this would replace `(x) in y)`
// with `x) in y)` which is syntactically invalid.
// However, this is prevented before we get here.
sugg: ParenthesesInForHeadSugg { left, right, left_snippet, right_snippet },
});

// Unwrap `(pat)` into `pat` to avoid the `unused_parens` lint.
pat.and_then(|pat| match pat.kind {
PatKind::Paren(pat) => pat,
_ => P(pat),
})
}
_ => pat,
}
}

pub(super) fn recover_seq_parse_error(
&mut self,
delim: Delimiter,
Expand Down
201 changes: 145 additions & 56 deletions compiler/rustc_parse/src/parser/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use super::{
use crate::errors;
use crate::maybe_recover_from_interpolated_ty_qpath;
use ast::mut_visit::{noop_visit_expr, MutVisitor};
use ast::{GenBlockKind, Path, PathSegment};
use ast::{GenBlockKind, Pat, Path, PathSegment};
use core::mem;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, Token, TokenKind};
Expand Down Expand Up @@ -2609,30 +2609,72 @@ impl<'a> Parser<'a> {
}
}

/// Parses `for <src_pat> in <src_expr> <src_loop_block>` (`for` token already eaten).
fn parse_expr_for(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'a, P<Expr>> {
// Record whether we are about to parse `for (`.
// This is used below for recovery in case of `for ( $stuff ) $block`
// in which case we will suggest `for $stuff $block`.
let begin_paren = match self.token.kind {
token::OpenDelim(Delimiter::Parenthesis) => Some(self.token.span),
_ => None,
fn parse_for_head(&mut self) -> PResult<'a, (P<Pat>, P<Expr>)> {
let begin_paren = if self.token.kind == token::OpenDelim(Delimiter::Parenthesis) {
// Record whether we are about to parse `for (`.
// This is used below for recovery in case of `for ( $stuff ) $block`
// in which case we will suggest `for $stuff $block`.
let start_span = self.token.span;
let left = self.prev_token.span.between(self.look_ahead(1, |t| t.span));
Some((start_span, left))
} else {
None
};
// Try to parse the pattern `for ($PAT) in $EXPR`.
let pat = match (
self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
),
begin_paren,
) {
(Ok(pat), _) => pat, // Happy path.
(Err(err), Some((start_span, left))) if self.eat_keyword(kw::In) => {
// We know for sure we have seen `for ($SOMETHING in`. In the happy path this would
// happen right before the return of this method.
let expr = match self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None) {
Ok(expr) => expr,
Err(expr_err) => {
// We don't know what followed the `in`, so cancel and bubble up the
// original error.
expr_err.cancel();
return Err(err);
}
};
return if self.token.kind == token::CloseDelim(Delimiter::Parenthesis) {
// We know for sure we have seen `for ($SOMETHING in $EXPR)`, so we recover the
// parser state and emit a targetted suggestion.
let span = vec![start_span, self.token.span];
let right = self.prev_token.span.between(self.look_ahead(1, |t| t.span));
self.bump(); // )
err.cancel();
self.sess.emit_err(errors::ParenthesesInForHead {
span,
// With e.g. `for (x) in y)` this would replace `(x) in y)`
// with `x) in y)` which is syntactically invalid.
// However, this is prevented before we get here.
sugg: errors::ParenthesesInForHeadSugg { left, right },
});
Ok((self.mk_pat(start_span.to(right), ast::PatKind::Wild), expr))
} else {
Err(err) // Some other error, bubble up.
};
}
(Err(err), _) => return Err(err), // Some other error, bubble up.
};

let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
)?;
if !self.eat_keyword(kw::In) {
self.error_missing_in_for_loop();
}
self.check_for_for_in_in_typo(self.prev_token.span);
let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
Ok((pat, expr))
}

let pat = self.recover_parens_around_for_head(pat, begin_paren);

/// Parses `for <src_pat> in <src_expr> <src_loop_block>` (`for` token already eaten).
fn parse_expr_for(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'a, P<Expr>> {
let (pat, expr) = self.parse_for_head()?;
// Recover from missing expression in `for` loop
if matches!(expr.kind, ExprKind::Block(..))
&& !matches!(self.token.kind, token::OpenDelim(Delimiter::Brace))
Expand Down Expand Up @@ -2853,47 +2895,10 @@ impl<'a> Parser<'a> {
}

pub(super) fn parse_arm(&mut self) -> PResult<'a, Arm> {
// Used to check the `let_chains` and `if_let_guard` features mostly by scanning
// `&&` tokens.
fn check_let_expr(expr: &Expr) -> (bool, bool) {
match &expr.kind {
ExprKind::Binary(BinOp { node: BinOpKind::And, .. }, lhs, rhs) => {
let lhs_rslt = check_let_expr(lhs);
let rhs_rslt = check_let_expr(rhs);
(lhs_rslt.0 || rhs_rslt.0, false)
}
ExprKind::Let(..) => (true, true),
_ => (false, true),
}
}
let attrs = self.parse_outer_attributes()?;
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let lo = this.token.span;
let pat = this.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
let guard = if this.eat_keyword(kw::If) {
let if_span = this.prev_token.span;
let mut cond = this.parse_match_guard_condition()?;

CondChecker::new(this).visit_expr(&mut cond);

let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
if has_let_expr {
if does_not_have_bin_op {
// Remove the last feature gating of a `let` expression since it's stable.
this.sess.gated_spans.ungate_last(sym::let_chains, cond.span);
}
let span = if_span.to(cond.span);
this.sess.gated_spans.gate(sym::if_let_guard, span);
}
Some(cond)
} else {
None
};
let (pat, guard) = this.parse_match_arm_pat_and_guard()?;
let arrow_span = this.token.span;
if let Err(mut err) = this.expect(&token::FatArrow) {
// We might have a `=>` -> `=` or `->` typo (issue #89396).
Expand Down Expand Up @@ -3023,6 +3028,90 @@ impl<'a> Parser<'a> {
})
}

fn parse_match_arm_guard(&mut self) -> PResult<'a, Option<P<Expr>>> {
// Used to check the `let_chains` and `if_let_guard` features mostly by scanning
// `&&` tokens.
fn check_let_expr(expr: &Expr) -> (bool, bool) {
match &expr.kind {
ExprKind::Binary(BinOp { node: BinOpKind::And, .. }, lhs, rhs) => {
let lhs_rslt = check_let_expr(lhs);
let rhs_rslt = check_let_expr(rhs);
(lhs_rslt.0 || rhs_rslt.0, false)
}
ExprKind::Let(..) => (true, true),
_ => (false, true),
}
}
if !self.eat_keyword(kw::If) {
// No match arm guard present.
return Ok(None);
}

let if_span = self.prev_token.span;
let mut cond = self.parse_match_guard_condition()?;

CondChecker::new(self).visit_expr(&mut cond);

let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
if has_let_expr {
if does_not_have_bin_op {
// Remove the last feature gating of a `let` expression since it's stable.
self.sess.gated_spans.ungate_last(sym::let_chains, cond.span);
}
let span = if_span.to(cond.span);
self.sess.gated_spans.gate(sym::if_let_guard, span);
}
Ok(Some(cond))
}

fn parse_match_arm_pat_and_guard(&mut self) -> PResult<'a, (P<Pat>, Option<P<Expr>>)> {
if self.token.kind == token::OpenDelim(Delimiter::Parenthesis) {
// Detect and recover from `($pat if $cond) => $arm`.
let left = self.token.span;
match self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
) {
Ok(pat) => Ok((pat, self.parse_match_arm_guard()?)),
Err(err)
if let prev_sp = self.prev_token.span
&& let true = self.eat_keyword(kw::If) =>
{
// We know for certain we've found `($pat if` so far.
let mut cond = match self.parse_match_guard_condition() {
Ok(cond) => cond,
Err(cond_err) => {
cond_err.cancel();
return Err(err);
}
};
err.cancel();
CondChecker::new(self).visit_expr(&mut cond);
self.eat_to_tokens(&[&token::CloseDelim(Delimiter::Parenthesis)]);
self.expect(&token::CloseDelim(Delimiter::Parenthesis))?;
let right = self.prev_token.span;
self.sess.emit_err(errors::ParenthesesInMatchPat {
span: vec![left, right],
sugg: errors::ParenthesesInMatchPatSugg { left, right },
});
Ok((self.mk_pat(left.to(prev_sp), ast::PatKind::Wild), Some(cond)))
}
Err(err) => Err(err),
}
} else {
// Regular parser flow:
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
Ok((pat, self.parse_match_arm_guard()?))
}
}

fn parse_match_guard_condition(&mut self) -> PResult<'a, P<Expr>> {
self.parse_expr_res(Restrictions::ALLOW_LET | Restrictions::IN_IF_GUARD, None).map_err(
|mut err| {
Expand Down
Loading

0 comments on commit 74fccd0

Please sign in to comment.