Skip to content

Use Parser's restrictions instead of let_expr_allowed #100011

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

Merged
merged 1 commit into from
Aug 2, 2022
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
56 changes: 17 additions & 39 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,8 +1391,6 @@ impl<'a> Parser<'a> {
} else if self.is_do_yeet() {
self.parse_yeet_expr(attrs)
} else if self.check_keyword(kw::Let) {
self.manage_let_chains_context();
self.bump();
self.parse_let_expr(attrs)
} else if self.eat_keyword(kw::Underscore) {
Ok(self.mk_expr(self.prev_token.span, ExprKind::Underscore, attrs))
Expand Down Expand Up @@ -2342,32 +2340,24 @@ impl<'a> Parser<'a> {

/// Parses the condition of a `if` or `while` expression.
fn parse_cond_expr(&mut self) -> PResult<'a, P<Expr>> {
self.with_let_management(true, |local_self| {
local_self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)
})
self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL | Restrictions::ALLOW_LET, None)
}

// Checks if `let` is in an invalid position like `let x = let y = 1;` or
// if the current `let` is in a let_chains context but nested in another
// expression like `if let Some(_) = _opt && [1, 2, 3][let _ = ()] = 1`.
//
// This method expects that the current token is `let`.
fn manage_let_chains_context(&mut self) {
debug_assert!(matches!(self.token.kind, TokenKind::Ident(kw::Let, _)));
let is_in_a_let_chains_context_but_nested_in_other_expr = self.let_expr_allowed
&& !matches!(
self.prev_token.kind,
TokenKind::AndAnd | TokenKind::Ident(kw::If, _) | TokenKind::Ident(kw::While, _)
);
if !self.let_expr_allowed || is_in_a_let_chains_context_but_nested_in_other_expr {
/// Parses a `let $pat = $expr` pseudo-expression.
fn parse_let_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
// This is a *approximate* heuristic that detects if `let` chains are
// being parsed in the right position. It's approximate because it
// doesn't deny all invalid `let` expressions, just completely wrong usages.
let not_in_chain = !matches!(
self.prev_token.kind,
TokenKind::AndAnd | TokenKind::Ident(kw::If, _) | TokenKind::Ident(kw::While, _)
);
if !self.restrictions.contains(Restrictions::ALLOW_LET) || not_in_chain {
self.struct_span_err(self.token.span, "expected expression, found `let` statement")
.emit();
}
}

/// Parses a `let $pat = $expr` pseudo-expression.
/// The `let` token has already been eaten.
fn parse_let_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
self.bump(); // Eat `let` token
let lo = self.prev_token.span;
let pat = self.parse_pat_allow_top_alt(
None,
Expand Down Expand Up @@ -2687,7 +2677,9 @@ impl<'a> Parser<'a> {
// `&&` tokens.
fn check_let_expr(expr: &Expr) -> bool {
match expr.kind {
ExprKind::Binary(_, ref lhs, ref rhs) => check_let_expr(lhs) || check_let_expr(rhs),
ExprKind::Binary(BinOp { node: BinOpKind::And, .. }, ref lhs, ref rhs) => {
check_let_expr(lhs) || check_let_expr(rhs)
}
ExprKind::Let(..) => true,
_ => false,
}
Expand All @@ -2703,9 +2695,8 @@ impl<'a> Parser<'a> {
)?;
let guard = if this.eat_keyword(kw::If) {
let if_span = this.prev_token.span;
let cond = this.with_let_management(true, |local_this| local_this.parse_expr())?;
let has_let_expr = check_let_expr(&cond);
if has_let_expr {
let cond = this.parse_expr_res(Restrictions::ALLOW_LET, None)?;
if check_let_expr(&cond) {
let span = if_span.to(cond.span);
this.sess.gated_spans.gate(sym::if_let_guard, span);
}
Expand Down Expand Up @@ -3279,17 +3270,4 @@ impl<'a> Parser<'a> {
Ok((res, trailing))
})
}

// Calls `f` with the internal `let_expr_allowed` set to `let_expr_allowed` and then
// sets the internal `let_expr_allowed` back to its original value.
fn with_let_management<T>(
&mut self,
let_expr_allowed: bool,
f: impl FnOnce(&mut Self) -> T,
) -> T {
let last_let_expr_allowed = mem::replace(&mut self.let_expr_allowed, let_expr_allowed);
let rslt = f(self);
self.let_expr_allowed = last_let_expr_allowed;
rslt
}
}
7 changes: 2 additions & 5 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ bitflags::bitflags! {
const STMT_EXPR = 1 << 0;
const NO_STRUCT_LITERAL = 1 << 1;
const CONST_EXPR = 1 << 2;
const ALLOW_LET = 1 << 3;
}
}

Expand Down Expand Up @@ -147,15 +148,12 @@ 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<ClosureSpans>,
/// Used to track where `let`s are allowed. For example, `if true && let 1 = 1` is valid
/// but `[1, 2, 3][let _ = ()]` is not.
let_expr_allowed: bool,
}

// This type is used a lot, e.g. it's cloned when matching many declarative macro rules. 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<'_>, 336);
rustc_data_structures::static_assert_size!(Parser<'_>, 328);

/// Stores span information about a closure.
#[derive(Clone)]
Expand Down Expand Up @@ -458,7 +456,6 @@ impl<'a> Parser<'a> {
inner_attr_ranges: Default::default(),
},
current_closure: None,
let_expr_allowed: false,
};

// Make parser point to the first token.
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/rfc-2294-if-let-guard/feature-gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ fn _if_let_guard() {
() if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
//~^ ERROR `if let` guards are experimental
//~| ERROR expected expression, found `let` statement
//~| ERROR expected expression, found `let` statement
//~| ERROR expected expression, found `let` statement

() if let Range { start: _, end: _ } = (true..true) && false => {}
//~^ ERROR `if let` guards are experimental
Expand Down
24 changes: 18 additions & 6 deletions src/test/ui/rfc-2294-if-let-guard/feature-gate.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,31 @@ LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 =
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:52:16
--> $DIR/feature-gate.rs:32:55
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:32:68
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:54:16
|
LL | use_expr!((let 0 = 1 && 0 == 0));
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:54:16
--> $DIR/feature-gate.rs:56:16
|
LL | use_expr!((let 0 = 1));
| ^^^

error: no rules expected the token `let`
--> $DIR/feature-gate.rs:62:15
--> $DIR/feature-gate.rs:64:15
|
LL | macro_rules! use_expr {
| --------------------- when calling this macro
Expand Down Expand Up @@ -102,7 +114,7 @@ LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 =
= help: you can write `if matches!(<expr>, <pattern>)` instead of `if let <pattern> = <expr>`

error[E0658]: `if let` guards are experimental
--> $DIR/feature-gate.rs:36:12
--> $DIR/feature-gate.rs:38:12
|
LL | () if let Range { start: _, end: _ } = (true..true) && false => {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -112,7 +124,7 @@ LL | () if let Range { start: _, end: _ } = (true..true) && false => {}
= help: you can write `if matches!(<expr>, <pattern>)` instead of `if let <pattern> = <expr>`

error[E0658]: `if let` guards are experimental
--> $DIR/feature-gate.rs:58:12
--> $DIR/feature-gate.rs:60:12
|
LL | () if let 0 = 1 => {}
| ^^^^^^^^^^^^
Expand All @@ -121,6 +133,6 @@ LL | () if let 0 = 1 => {}
= help: add `#![feature(if_let_guard)]` to the crate attributes to enable
= help: you can write `if matches!(<expr>, <pattern>)` instead of `if let <pattern> = <expr>`

error: aborting due to 16 previous errors
error: aborting due to 18 previous errors

For more information about this error, try `rustc --explain E0658`.
12 changes: 11 additions & 1 deletion src/test/ui/rfc-2497-if-let-chains/disallowed-positions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ fn _if() {
//~| ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
//~| ERROR expected expression, found `let` statement
//~| ERROR expected expression, found `let` statement
//~| ERROR expected expression, found `let` statement
}

fn _while() {
Expand Down Expand Up @@ -81,6 +83,8 @@ fn _while() {
//~| ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
//~| ERROR expected expression, found `let` statement
//~| ERROR expected expression, found `let` statement
//~| ERROR expected expression, found `let` statement
}

fn _macros() {
Expand Down Expand Up @@ -146,6 +150,7 @@ fn nested_within_if_expr() {
//~| ERROR expected expression, found `let` statement
if true || (true && let 0 = 0) {}
//~^ ERROR `let` expressions are not supported here
//~| ERROR expected expression, found `let` statement

let mut x = true;
if x = let 0 = 0 {}
Expand Down Expand Up @@ -237,6 +242,7 @@ fn nested_within_while_expr() {
//~| ERROR expected expression, found `let` statement
while true || (true && let 0 = 0) {}
//~^ ERROR `let` expressions are not supported here
//~| ERROR expected expression, found `let` statement

let mut x = true;
while x = let 0 = 0 {}
Expand Down Expand Up @@ -388,16 +394,19 @@ fn inside_const_generic_arguments() {
if let A::<{
true && let 1 = 1
//~^ ERROR `let` expressions are not supported here
//~| ERROR expected expression, found `let` statement
}>::O = 5 {}

while let A::<{
true && let 1 = 1
//~^ ERROR `let` expressions are not supported here
//~| ERROR expected expression, found `let` statement
}>::O = 5 {}

if A::<{
true && let 1 = 1
//~^ ERROR `let` expressions are not supported here
//~| ERROR expected expression, found `let` statement
}>::O == 5 {}

// In the cases above we have `ExprKind::Block` to help us out.
Expand All @@ -409,7 +418,8 @@ fn inside_const_generic_arguments() {
if A::<
true && let 1 = 1
//~^ ERROR `let` expressions are not supported here
//~| ERROR expressions must be enclosed in braces
//~| ERROR expressions must be enclosed in braces
//~| ERROR expected expression, found `let` statement
>::O == 5 {}
}

Expand Down
Loading