Skip to content

Commit

Permalink
Recover more expressions in patterns
Browse files Browse the repository at this point in the history
  • Loading branch information
ShE3py committed Jul 6, 2024
1 parent 5f8987f commit b317f9f
Show file tree
Hide file tree
Showing 20 changed files with 561 additions and 287 deletions.
10 changes: 2 additions & 8 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -774,15 +774,9 @@ parse_unexpected_expr_in_pat =
expected {$is_bound ->
[true] a pattern range bound
*[false] a pattern
}, found {$is_method_call ->
[true] a method call
*[false] an expression
}
}, found an expression
.label = {$is_method_call ->
[true] method calls
*[false] arbitrary expressions
} are not allowed in patterns
.label = arbitrary expressions are not allowed in patterns
parse_unexpected_if_with_if = unexpected `if` in the condition expression
.suggestion = remove the `if`
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2422,8 +2422,6 @@ pub(crate) struct UnexpectedExpressionInPattern {
pub span: Span,
/// Was a `RangePatternBound` expected?
pub is_bound: bool,
/// Was the unexpected expression a `MethodCallExpression`?
pub is_method_call: bool,
}

#[derive(Diagnostic)]
Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ use tracing::instrument;

#[derive(Debug)]
pub(super) enum LhsExpr {
// Already parsed just the outer attributes.
/// Already parsed just the outer attributes.
Unparsed { attrs: AttrWrapper },
// Already parsed the expression.
/// Already parsed the expression.
Parsed { expr: P<Expr>, starts_statement: bool },
}

#[derive(Debug)]
enum DestructuredFloat {
pub(super) enum DestructuredFloat {
/// 1e2
Single(Symbol, Span),
/// 1.
Expand Down Expand Up @@ -1036,15 +1036,17 @@ impl<'a> Parser<'a> {
self.dcx().emit_err(errors::UnexpectedTokenAfterDot { span, actual })
}

// We need an identifier or integer, but the next token is a float.
// Break the float into components to extract the identifier or integer.
/// We need an identifier or integer, but the next token is a float.
/// Break the float into components to extract the identifier or integer.
///
/// See also [`TokenKind::break_two_token_op`] which does similar splitting of `>>` into `>`.
//
// FIXME: With current `TokenCursor` it's hard to break tokens into more than 2
// parts unless those parts are processed immediately. `TokenCursor` should either
// support pushing "future tokens" (would be also helpful to `break_and_eat`), or
// we should break everything including floats into more basic proc-macro style
// tokens in the lexer (probably preferable).
// See also `TokenKind::break_two_token_op` which does similar splitting of `>>` into `>`.
fn break_up_float(&self, float: Symbol, span: Span) -> DestructuredFloat {
pub(super) fn break_up_float(&self, float: Symbol, span: Span) -> DestructuredFloat {
#[derive(Debug)]
enum FloatComponent {
IdentLike(String),
Expand Down
132 changes: 75 additions & 57 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::errors::{
UnexpectedParenInRangePatSugg, UnexpectedVertVertBeforeFunctionParam,
UnexpectedVertVertInPattern,
};
use crate::parser::expr::{could_be_unclosed_char_literal, LhsExpr};
use crate::parser::expr::{could_be_unclosed_char_literal, DestructuredFloat, LhsExpr};
use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor};
use rustc_ast::ptr::P;
Expand Down Expand Up @@ -337,46 +337,61 @@ impl<'a> Parser<'a> {
}
}

/// Ensures that the last parsed pattern (or pattern range bound) is not followed by a method call or an operator.
/// Ensures that the last parsed pattern (or pattern range bound) is not followed by an expression.
///
/// `is_end_bound` indicates whether the last parsed thing was the end bound of a range pattern (see [`parse_pat_range_end`](Self::parse_pat_range_end))
/// in order to say "expected a pattern range bound" instead of "expected a pattern";
/// ```text
/// 0..=1 + 2
/// ^^^^^
/// ```
/// Only the end bound is spanned, and this function have no idea if there were a `..=` before `pat_span`, hence the parameter.
/// Only the end bound is spanned in this case, and this function have no idea if there were a `..=` before `pat_span`, hence the parameter.
///
/// This function returns `Some` if a trailing expression was recovered, and said expression's span.
#[must_use = "the pattern must be discarded as `PatKind::Err` if this function returns Some"]
fn maybe_recover_trailing_expr(
&mut self,
pat_span: Span,
is_end_bound: bool,
) -> Option<ErrorGuaranteed> {
) -> Option<(ErrorGuaranteed, Span)> {
if self.prev_token.is_keyword(kw::Underscore) || !self.may_recover() {
// Don't recover anything after an `_` or if recovery is disabled.
return None;
}

// Check for `.hello()`, but allow `.Hello()` to be recovered as `, Hello()` in `parse_seq_to_before_tokens()`.
let has_trailing_method = self.check_noexpect(&token::Dot)
// Returns `true` iff `token` is a `x.y` float.
let is_two_tuple_indexes = |that: &Self, token: &Token| -> bool {
use token::{Lit, LitKind};

let token::Literal(Lit { kind: LitKind::Float, symbol, suffix: None }) = token.kind
else {
return false;
};

matches!(that.break_up_float(symbol, token.span), DestructuredFloat::MiddleDot(..))
};

// Check for `.hello` or `.0`.
let has_dot_expr = self.check_noexpect(&token::Dot) // `.`
&& self.look_ahead(1, |tok| {
tok.ident()
.and_then(|(ident, _)| ident.name.as_str().chars().next())
.is_some_and(char::is_lowercase)
})
&& self.look_ahead(2, |tok| tok.kind == token::OpenDelim(Delimiter::Parenthesis));
tok.is_ident() // `hello`
|| tok.is_integer_lit() // `0`
|| is_two_tuple_indexes(&self, &tok) // `0.0`
});

// Check for operators.
// `|` is excluded as it is used in pattern alternatives and lambdas,
// `?` is included for error propagation,
// `[` is included for indexing operations,
// `[]` is excluded as `a[]` isn't an expression and should be recovered as `a, []` (cf. `tests/ui/parser/pat-lt-bracket-7.rs`)
// `[]` is excluded as `a[]` isn't an expression and should be recovered as `a, []` (cf. `tests/ui/parser/pat-lt-bracket-7.rs`),
// `as` is included for type casts
let has_trailing_operator = matches!(self.token.kind, token::BinOp(op) if op != BinOpToken::Or)
|| self.token.kind == token::Question
|| (self.token.kind == token::OpenDelim(Delimiter::Bracket)
&& self.look_ahead(1, |tok| tok.kind != token::CloseDelim(Delimiter::Bracket)));
&& self.look_ahead(1, |tok| tok.kind != token::CloseDelim(Delimiter::Bracket))) // excludes `[]`
|| self.token.is_keyword(kw::As);

if !has_trailing_method && !has_trailing_operator {
if !has_dot_expr && !has_trailing_operator {
// Nothing to recover here.
return None;
}
Expand All @@ -386,43 +401,40 @@ impl<'a> Parser<'a> {
snapshot.restrictions.insert(Restrictions::IS_PAT);

// Parse `?`, `.f`, `(arg0, arg1, ...)` or `[expr]` until they've all been eaten.
if let Ok(expr) = snapshot
let Ok(expr) = snapshot
.parse_expr_dot_or_call_with(
self.mk_expr(pat_span, ExprKind::Dummy), // equivalent to transforming the parsed pattern into an `Expr`
pat_span,
AttrVec::new(),
)
.map_err(|err| err.cancel())
{
let non_assoc_span = expr.span;

// Parse an associative expression such as `+ expr`, `% expr`, ...
// Assignements, ranges and `|` are disabled by [`Restrictions::IS_PAT`].
let lhs = LhsExpr::Parsed { expr, starts_statement: false };
if let Ok(expr) = snapshot.parse_expr_assoc_with(0, lhs).map_err(|err| err.cancel()) {
// We got a valid expression.
self.restore_snapshot(snapshot);
self.restrictions.remove(Restrictions::IS_PAT);

let is_bound = is_end_bound
// is_start_bound: either `..` or `)..`
|| self.token.is_range_separator()
|| self.token.kind == token::CloseDelim(Delimiter::Parenthesis)
&& self.look_ahead(1, Token::is_range_separator);

// Check that `parse_expr_assoc_with` didn't eat a rhs.
let is_method_call = has_trailing_method && non_assoc_span == expr.span;

return Some(self.dcx().emit_err(UnexpectedExpressionInPattern {
span: expr.span,
is_bound,
is_method_call,
}));
}
}
else {
// We got a trailing method/operator, but that wasn't an expression.
return None;
};

// Parse an associative expression such as `+ expr`, `% expr`, ...
// Assignements, ranges and `|` are disabled by [`Restrictions::IS_PAT`].
let lhs = LhsExpr::Parsed { expr, starts_statement: false };
let Ok(expr) = snapshot.parse_expr_assoc_with(0, lhs).map_err(|err| err.cancel()) else {
// We got a trailing method/operator, but that wasn't an expression.
return None;
};

// We got a valid expression.
self.restore_snapshot(snapshot);
self.restrictions.remove(Restrictions::IS_PAT);

let is_bound = is_end_bound
// is_start_bound: either `..` or `)..`
|| self.token.is_range_separator()
|| self.token.kind == token::CloseDelim(Delimiter::Parenthesis)
&& self.look_ahead(1, Token::is_range_separator);

// We got a trailing method/operator, but we couldn't parse an expression.
None
Some((
self.dcx().emit_err(UnexpectedExpressionInPattern { span: expr.span, is_bound }),
expr.span,
))
}

/// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
Expand Down Expand Up @@ -534,7 +546,7 @@ impl<'a> Parser<'a> {
self.parse_pat_tuple_struct(qself, path)?
} else {
match self.maybe_recover_trailing_expr(span, false) {
Some(guar) => PatKind::Err(guar),
Some((guar, _)) => PatKind::Err(guar),
None => PatKind::Path(qself, path),
}
}
Expand Down Expand Up @@ -567,10 +579,10 @@ impl<'a> Parser<'a> {
// Try to parse everything else as literal with optional minus
match self.parse_literal_maybe_minus() {
Ok(begin) => {
let begin = match self.maybe_recover_trailing_expr(begin.span, false) {
Some(guar) => self.mk_expr_err(begin.span, guar),
None => begin,
};
let begin = self
.maybe_recover_trailing_expr(begin.span, false)
.map(|(guar, sp)| self.mk_expr_err(sp, guar))
.unwrap_or(begin);

match self.parse_range_end() {
Some(form) => self.parse_pat_range_begin_with(begin, form)?,
Expand Down Expand Up @@ -700,7 +712,8 @@ impl<'a> Parser<'a> {
// For backward compatibility, `(..)` is a tuple pattern as well.
let paren_pattern =
fields.len() == 1 && !(matches!(trailing_comma, Trailing::Yes) || fields[0].is_rest());
if paren_pattern {

let pat = if paren_pattern {
let pat = fields.into_iter().next().unwrap();
let close_paren = self.prev_token.span;

Expand All @@ -718,7 +731,7 @@ impl<'a> Parser<'a> {
},
});

self.parse_pat_range_begin_with(begin.clone(), form)
self.parse_pat_range_begin_with(begin.clone(), form)?
}
// recover ranges with parentheses around the `(start)..`
PatKind::Err(guar)
Expand All @@ -733,15 +746,20 @@ impl<'a> Parser<'a> {
},
});

self.parse_pat_range_begin_with(self.mk_expr_err(pat.span, *guar), form)
self.parse_pat_range_begin_with(self.mk_expr_err(pat.span, *guar), form)?
}

// (pat) with optional parentheses
_ => Ok(PatKind::Paren(pat)),
_ => PatKind::Paren(pat),
}
} else {
Ok(PatKind::Tuple(fields))
}
PatKind::Tuple(fields)
};

Ok(match self.maybe_recover_trailing_expr(open_paren.to(self.prev_token.span), false) {
None => pat,
Some((guar, _)) => PatKind::Err(guar),
})
}

/// Parse a mutable binding with the `mut` token already eaten.
Expand Down Expand Up @@ -991,7 +1009,7 @@ impl<'a> Parser<'a> {
}

Ok(match recovered {
Some(guar) => self.mk_expr_err(bound.span, guar),
Some((guar, sp)) => self.mk_expr_err(sp, guar),
None => bound,
})
}
Expand Down Expand Up @@ -1060,7 +1078,7 @@ impl<'a> Parser<'a> {
// but not `ident @ subpat` as `subpat` was already checked and `ident` continues with `@`.

let pat = if sub.is_none()
&& let Some(guar) = self.maybe_recover_trailing_expr(ident.span, false)
&& let Some((guar, _)) = self.maybe_recover_trailing_expr(ident.span, false)
{
PatKind::Err(guar)
} else {
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/parser/bad-name.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ error-pattern: expected

fn main() {
let x.y::<isize>.z foo;
//~^ error: field expressions cannot have generic arguments
//~| error: expected a pattern, found an expression
//~| error: expected one of `(`, `.`, `::`, `:`, `;`, `=`, `?`, `|`, or an operator, found `foo`
}
20 changes: 16 additions & 4 deletions tests/ui/parser/bad-name.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
error: expected one of `:`, `;`, `=`, `@`, or `|`, found `.`
--> $DIR/bad-name.rs:4:8
error: field expressions cannot have generic arguments
--> $DIR/bad-name.rs:2:12
|
LL | let x.y::<isize>.z foo;
| ^ expected one of `:`, `;`, `=`, `@`, or `|`
| ^^^^^^^

error: aborting due to 1 previous error
error: expected a pattern, found an expression
--> $DIR/bad-name.rs:2:7
|
LL | let x.y::<isize>.z foo;
| ^^^^^^^^^^^^^^ arbitrary expressions are not allowed in patterns

error: expected one of `(`, `.`, `::`, `:`, `;`, `=`, `?`, `|`, or an operator, found `foo`
--> $DIR/bad-name.rs:2:22
|
LL | let x.y::<isize>.z foo;
| ^^^ expected one of 9 possible tokens

error: aborting due to 3 previous errors

28 changes: 0 additions & 28 deletions tests/ui/parser/pat-recover-exprs.rs

This file was deleted.

Loading

0 comments on commit b317f9f

Please sign in to comment.