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

Improve handling of expressions in patterns #118625

Merged
merged 1 commit into from
Jan 29, 2024
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
14 changes: 14 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,20 @@ parse_unexpected_const_param_declaration = unexpected `const` parameter declarat
parse_unexpected_default_value_for_lifetime_in_generic_parameters = unexpected default lifetime parameter
.label = lifetime parameters cannot have default values

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
}

.label = {$is_method_call ->
[true] method calls
*[false] arbitrary expressions
} are not allowed in patterns

parse_unexpected_if_with_if = unexpected `if` in the condition expression
.suggestion = remove the `if`

Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2415,6 +2415,18 @@ pub(crate) struct ExpectedCommaAfterPatternField {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(parse_unexpected_expr_in_pat)]
pub(crate) struct UnexpectedExpressionInPattern {
#[primary_span]
#[label]
pub span: Span,
/// Was a `RangePatternBound` expected?
pub is_bound: bool,
/// Was the unexpected expression a `MethodCallExpression`?
pub is_method_call: bool,
}

#[derive(Diagnostic)]
#[diag(parse_unexpected_paren_in_range_pat)]
pub(crate) struct UnexpectedParenInRangePat {
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,19 @@ impl<'a> Parser<'a> {
) if self.restrictions.contains(Restrictions::CONST_EXPR) => {
return None;
}
// When recovering patterns as expressions, stop parsing when encountering an assignment `=`, an alternative `|`, or a range `..`.
(
Some(
AssocOp::Assign
| AssocOp::AssignOp(_)
| AssocOp::BitOr
| AssocOp::DotDot
| AssocOp::DotDotEq,
),
_,
) if self.restrictions.contains(Restrictions::IS_PAT) => {
return None;
}
(Some(op), _) => (op, self.token.span),
(None, Some((Ident { name: sym::and, span }, false))) if self.may_recover() => {
self.dcx().emit_err(errors::InvalidLogicalOperator {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ bitflags::bitflags! {
const CONST_EXPR = 1 << 2;
const ALLOW_LET = 1 << 3;
const IN_IF_GUARD = 1 << 4;
const IS_PAT = 1 << 5;
}
}

Expand Down
155 changes: 142 additions & 13 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
use super::{ForceCollect, Parser, PathStyle, TrailingToken};
use super::{ForceCollect, Parser, PathStyle, Restrictions, TrailingToken};
use crate::errors::{
self, AmbiguousRangePattern, DotDotDotForRemainingFields, DotDotDotRangeToPatternNotAllowed,
DotDotDotRestPattern, EnumPatternInsteadOfIdentifier, ExpectedBindingLeftOfAt,
ExpectedCommaAfterPatternField, GenericArgsInPatRequireTurbofishSyntax,
InclusiveRangeExtraEquals, InclusiveRangeMatchArrow, InclusiveRangeNoEnd, InvalidMutInPattern,
PatternOnWrongSideOfAt, RefMutOrderIncorrect, RemoveLet, RepeatedMutInPattern,
SwitchRefBoxOrder, TopLevelOrPatternNotAllowed, TopLevelOrPatternNotAllowedSugg,
TrailingVertNotAllowed, UnexpectedLifetimeInPattern, UnexpectedParenInRangePat,
UnexpectedParenInRangePatSugg, UnexpectedVertVertBeforeFunctionParam,
UnexpectedVertVertInPattern,
TrailingVertNotAllowed, UnexpectedExpressionInPattern, UnexpectedLifetimeInPattern,
UnexpectedParenInRangePat, UnexpectedParenInRangePatSugg,
UnexpectedVertVertBeforeFunctionParam, UnexpectedVertVertInPattern,
};
use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor};
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter};
use rustc_ast::token::{self, BinOpToken, Delimiter, Token};
use rustc_ast::{
self as ast, AttrVec, BindingAnnotation, ByRef, Expr, ExprKind, MacCall, Mutability, Pat,
PatField, PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax,
Expand All @@ -23,7 +23,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder, PResult};
use rustc_session::errors::ExprParenthesesNeeded;
use rustc_span::source_map::{respan, Spanned};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::Span;
use rustc_span::{ErrorGuaranteed, Span};
use thin_vec::{thin_vec, ThinVec};

#[derive(PartialEq, Copy, Clone)]
Expand Down Expand Up @@ -336,6 +336,95 @@ impl<'a> Parser<'a> {
}
}

/// Ensures that the last parsed pattern (or pattern range bound) is not followed by a method call or an operator.
///
/// `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.
#[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> {
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)
&& 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));

// 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`)
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)));

if !has_trailing_method && !has_trailing_operator {
// Nothing to recover here.
return None;
}

// Let's try to parse an expression to emit a better diagnostic.
let mut snapshot = self.create_snapshot_for_diagnostic();
snapshot.restrictions.insert(Restrictions::IS_PAT);

// Parse `?`, `.f`, `(arg0, arg1, ...)` or `[expr]` until they've all been eaten.
if let Ok(expr) = snapshot
.parse_expr_dot_or_call_with(
self.mk_expr_err(pat_span), // 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`].
if let Ok(expr) =
snapshot.parse_expr_assoc_with(0, expr.into()).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,
}));
}
}

// We got a trailing method/operator, but we couldn't parse an expression.
None
}

/// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
/// allowed).
fn parse_pat_with_range_pat(
Expand Down Expand Up @@ -441,7 +530,10 @@ impl<'a> Parser<'a> {
} else if self.check(&token::OpenDelim(Delimiter::Parenthesis)) {
self.parse_pat_tuple_struct(qself, path)?
} else {
PatKind::Path(qself, path)
match self.maybe_recover_trailing_expr(span, false) {
Some(guar) => PatKind::Err(guar),
None => PatKind::Path(qself, path),
}
}
} else if matches!(self.token.kind, token::Lifetime(_))
// In pattern position, we're totally fine with using "next token isn't colon"
Expand Down Expand Up @@ -470,10 +562,17 @@ impl<'a> Parser<'a> {
} else {
// Try to parse everything else as literal with optional minus
match self.parse_literal_maybe_minus() {
Ok(begin) => match self.parse_range_end() {
Some(form) => self.parse_pat_range_begin_with(begin, form)?,
None => PatKind::Lit(begin),
},
Ok(begin) => {
let begin = match self.maybe_recover_trailing_expr(begin.span, false) {
Some(_) => self.mk_expr_err(begin.span),
None => begin,
};

match self.parse_range_end() {
Some(form) => self.parse_pat_range_begin_with(begin, form)?,
None => PatKind::Lit(begin),
}
}
Err(err) => return self.fatal_unexpected_non_pat(err, expected),
}
};
Expand Down Expand Up @@ -615,6 +714,21 @@ impl<'a> Parser<'a> {

self.parse_pat_range_begin_with(begin.clone(), form)?
}
// recover ranges with parentheses around the `(start)..`
PatKind::Err(_)
if self.may_recover()
&& let Some(form) = self.parse_range_end() =>
{
self.dcx().emit_err(UnexpectedParenInRangePat {
span: vec![open_paren, close_paren],
sugg: UnexpectedParenInRangePatSugg {
start_span: open_paren,
end_span: close_paren,
},
});

self.parse_pat_range_begin_with(self.mk_expr(pat.span, ExprKind::Err), form)?
}

// (pat) with optional parentheses
_ => PatKind::Paren(pat),
Expand Down Expand Up @@ -853,6 +967,8 @@ impl<'a> Parser<'a> {
self.parse_literal_maybe_minus()
}?;

let recovered = self.maybe_recover_trailing_expr(bound.span, true);

// recover trailing `)`
if let Some(open_paren) = open_paren {
self.expect(&token::CloseDelim(Delimiter::Parenthesis))?;
Expand All @@ -866,7 +982,10 @@ impl<'a> Parser<'a> {
});
}

Ok(bound)
Ok(match recovered {
Some(_) => self.mk_expr_err(bound.span),
None => bound,
})
}

/// Is this the start of a pattern beginning with a path?
Expand Down Expand Up @@ -929,7 +1048,17 @@ impl<'a> Parser<'a> {
.create_err(EnumPatternInsteadOfIdentifier { span: self.prev_token.span }));
}

Ok(PatKind::Ident(binding_annotation, ident, sub))
// Check for method calls after the `ident`,
// 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)
{
PatKind::Err(guar)
} else {
PatKind::Ident(binding_annotation, ident, sub)
};
Ok(pat)
}

/// Parse a struct ("record") pattern (e.g. `Foo { ... }` or `Foo::Bar { ... }`).
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/half-open-range-patterns/range_pat_interactions1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@ fn main() {
}
match x as i32 {
0..5+1 => errors_only.push(x),
//~^ error: expected one of `=>`, `if`, or `|`, found `+`
//~^ error: expected a pattern range bound, found an expression
//~| error: exclusive range pattern syntax is experimental
1 | -3..0 => first_or.push(x),
//~^ error: exclusive range pattern syntax is experimental
y @ (0..5 | 6) => or_two.push(y),
//~^ error: exclusive range pattern syntax is experimental
y @ 0..const { 5 + 1 } => assert_eq!(y, 5),
//~^ error: exclusive range pattern syntax is experimental
//~| error: inline-const in pattern position is experimental
y @ -5.. => range_from.push(y),
y @ ..-7 => assert_eq!(y, -8),
//~^ error: exclusive range pattern syntax is experimental
y => bottom.push(y),
}
}
Expand Down
Loading
Loading