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

Tweak parsing recovery of enums, for exprs and match arm patterns #117565

Merged
merged 13 commits into from
Nov 30, 2023
Merged
3 changes: 3 additions & 0 deletions compiler/rustc_parse/messages.ftl
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
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
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
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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it obvious that the only semantic difference is in the error path, I'd prefer to move the call to parse_pat_allow_top_alt out of the if and move the if down to the error case.
This doesn't have to block the PR and can be done as a follow-up (it wouldn't be too bad if it never happens either, but it would be nice to have it).`

// 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
Loading