Skip to content

Commit

Permalink
Rollup merge of #98633 - c410-f3r:yet-another-let-chain, r=estebank
Browse files Browse the repository at this point in the history 
Fix last `let_chains` blocker

In order to forbid things like `let x = (let y = 1);` or `if let a = 1 && { let x = let y = 1; } {}`, the parser **HAS** to know the context of `let`.

This context thing is not a surprise in the parser because you can see **a lot** of ad hoc fixes mixing parsing logic with validation logic creating code that looks more like spaghetti with tomato sauce.

To make things even greater, a new ad hoc fix was added to only allow `let`s in a valid `let_chains` context by checking the previously processed token. This was the only solution I could think of and believe me, I thought about it for a long time 👍

In the long term, it should be preferable to segregate different responsibilities or create a more robust and cleaner parser framework.

cc #94927
cc #53667
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored Jul 12, 2022
2 parents 7615366 + 1c3bab2 commit 9997c51
Show file tree
Hide file tree
Showing 9 changed files with 827 additions and 301 deletions.
32 changes: 24 additions & 8 deletions compiler/rustc_parse/src/parser/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,9 @@ impl<'a> Parser<'a> {
self.parse_yield_expr(attrs)
} else if self.is_do_yeet() {
self.parse_yeet_expr(attrs)
} else if self.eat_keyword(kw::Let) {
} 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 @@ -2355,16 +2357,30 @@ impl<'a> Parser<'a> {
Ok(cond)
}

// 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::CloseDelim(Delimiter::Brace)
| TokenKind::Ident(kw::If, _)
| TokenKind::Ident(kw::While, _)
);
if !self.let_expr_allowed || is_in_a_let_chains_context_but_nested_in_other_expr {
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>> {
if !self.let_expr_allowed {
self.struct_span_err(
self.prev_token.span,
"expected expression, found `let` statement",
)
.emit();
}
let lo = self.prev_token.span;
let pat = self.parse_pat_allow_top_alt(
None,
Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/rfc-2294-if-let-guard/feature-gate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ fn _if_let_guard() {

() if (let 0 = 1) => {}
//~^ ERROR `let` expressions in this position are unstable
//~| ERROR expected expression, found `let` statement

() if (((let 0 = 1))) => {}
//~^ ERROR `let` expressions in this position are unstable
//~| ERROR expected expression, found `let` statement

() if true && let 0 = 1 => {}
//~^ ERROR `if let` guards are experimental
Expand All @@ -23,13 +25,17 @@ fn _if_let_guard() {

() if (let 0 = 1) && true => {}
//~^ ERROR `let` expressions in this position are unstable
//~| ERROR expected expression, found `let` statement

() if true && (let 0 = 1) => {}
//~^ ERROR `let` expressions in this position are unstable
//~| ERROR expected expression, found `let` statement

() if (let 0 = 1) && (let 0 = 1) => {}
//~^ ERROR `let` expressions in this position are unstable
//~| ERROR `let` expressions in this position are unstable
//~| ERROR expected expression, found `let` statement
//~| ERROR expected expression, found `let` statement

() if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
//~^ ERROR `if let` guards are experimental
Expand All @@ -38,6 +44,7 @@ fn _if_let_guard() {
//~| ERROR `let` expressions in this position are unstable
//~| ERROR `let` expressions in this position are unstable
//~| ERROR `let` expressions in this position are unstable
//~| ERROR expected expression, found `let` statement

() if let Range { start: _, end: _ } = (true..true) && false => {}
//~^ ERROR `if let` guards are experimental
Expand Down
90 changes: 66 additions & 24 deletions src/test/ui/rfc-2294-if-let-guard/feature-gate.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,59 @@
error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:59:16
--> $DIR/feature-gate.rs:10:16
|
LL | () if (let 0 = 1) => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:14:18
|
LL | () if (((let 0 = 1))) => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:26:16
|
LL | () if (let 0 = 1) && true => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:30:24
|
LL | () if true && (let 0 = 1) => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:34:16
|
LL | () if (let 0 = 1) && (let 0 = 1) => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:34:31
|
LL | () if (let 0 = 1) && (let 0 = 1) => {}
| ^^^

error: expected expression, found `let` statement
--> $DIR/feature-gate.rs:40:42
|
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:66:16
|
LL | use_expr!((let 0 = 1 && 0 == 0));
| ^^^

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

error: no rules expected the token `let`
--> $DIR/feature-gate.rs:71:15
--> $DIR/feature-gate.rs:78:15
|
LL | macro_rules! use_expr {
| --------------------- when calling this macro
Expand All @@ -30,7 +72,7 @@ LL | () if let 0 = 1 => {}
= 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:16:12
--> $DIR/feature-gate.rs:18:12
|
LL | () if true && let 0 = 1 => {}
| ^^^^^^^^^^^^^^^^^^^^
Expand All @@ -40,7 +82,7 @@ LL | () if true && let 0 = 1 => {}
= 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:20:12
--> $DIR/feature-gate.rs:22:12
|
LL | () if let 0 = 1 && true => {}
| ^^^^^^^^^^^^^^^^^^^^
Expand All @@ -50,7 +92,7 @@ LL | () if let 0 = 1 && true => {}
= 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:34:12
--> $DIR/feature-gate.rs:40:12
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -60,7 +102,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:42:12
--> $DIR/feature-gate.rs:49:12
|
LL | () if let Range { start: _, end: _ } = (true..true) && false => {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -70,7 +112,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:67:12
--> $DIR/feature-gate.rs:74:12
|
LL | () if let 0 = 1 => {}
| ^^^^^^^^^^^^
Expand All @@ -89,7 +131,7 @@ LL | () if (let 0 = 1) => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:13:18
--> $DIR/feature-gate.rs:14:18
|
LL | () if (((let 0 = 1))) => {}
| ^^^^^^^^^
Expand All @@ -98,7 +140,7 @@ LL | () if (((let 0 = 1))) => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:16:23
--> $DIR/feature-gate.rs:18:23
|
LL | () if true && let 0 = 1 => {}
| ^^^^^^^^^
Expand All @@ -107,7 +149,7 @@ LL | () if true && let 0 = 1 => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:20:15
--> $DIR/feature-gate.rs:22:15
|
LL | () if let 0 = 1 && true => {}
| ^^^^^^^^^
Expand All @@ -116,7 +158,7 @@ LL | () if let 0 = 1 && true => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:24:16
--> $DIR/feature-gate.rs:26:16
|
LL | () if (let 0 = 1) && true => {}
| ^^^^^^^^^
Expand All @@ -125,7 +167,7 @@ LL | () if (let 0 = 1) && true => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:27:24
--> $DIR/feature-gate.rs:30:24
|
LL | () if true && (let 0 = 1) => {}
| ^^^^^^^^^
Expand All @@ -134,7 +176,7 @@ LL | () if true && (let 0 = 1) => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:30:16
--> $DIR/feature-gate.rs:34:16
|
LL | () if (let 0 = 1) && (let 0 = 1) => {}
| ^^^^^^^^^
Expand All @@ -143,7 +185,7 @@ LL | () if (let 0 = 1) && (let 0 = 1) => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:30:31
--> $DIR/feature-gate.rs:34:31
|
LL | () if (let 0 = 1) && (let 0 = 1) => {}
| ^^^^^^^^^
Expand All @@ -152,7 +194,7 @@ LL | () if (let 0 = 1) && (let 0 = 1) => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:34:15
--> $DIR/feature-gate.rs:40:15
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^^^^^^^
Expand All @@ -161,7 +203,7 @@ LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 =
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:34:28
--> $DIR/feature-gate.rs:40:28
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^^^^^^^
Expand All @@ -170,7 +212,7 @@ LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 =
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:34:42
--> $DIR/feature-gate.rs:40:42
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^^^^^^^
Expand All @@ -179,7 +221,7 @@ LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 =
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:34:55
--> $DIR/feature-gate.rs:40:55
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^^^^^^^
Expand All @@ -188,7 +230,7 @@ LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 =
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:34:68
--> $DIR/feature-gate.rs:40:68
|
LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) => {}
| ^^^^^^^^^
Expand All @@ -197,7 +239,7 @@ LL | () if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 =
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:42:15
--> $DIR/feature-gate.rs:49:15
|
LL | () if let Range { start: _, end: _ } = (true..true) && false => {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -206,7 +248,7 @@ LL | () if let Range { start: _, end: _ } = (true..true) && false => {}
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:59:16
--> $DIR/feature-gate.rs:66:16
|
LL | use_expr!((let 0 = 1 && 0 == 0));
| ^^^^^^^^^
Expand All @@ -215,14 +257,14 @@ LL | use_expr!((let 0 = 1 && 0 == 0));
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error[E0658]: `let` expressions in this position are unstable
--> $DIR/feature-gate.rs:62:16
--> $DIR/feature-gate.rs:69:16
|
LL | use_expr!((let 0 = 1));
| ^^^^^^^^^
|
= note: see issue #53667 <https://github.com/rust-lang/rust/issues/53667> for more information
= help: add `#![feature(let_chains)]` to the crate attributes to enable

error: aborting due to 25 previous errors
error: aborting due to 32 previous errors

For more information about this error, try `rustc --explain E0658`.
Loading

0 comments on commit 9997c51

Please sign in to comment.