Skip to content

Commit

Permalink
Rollup merge of #108427 - y21:for-else-diagnostic, r=compiler-errors
Browse files Browse the repository at this point in the history
Recover from for-else and while-else

This recovers from attempts at writing for-else or while-else loops, which might help users coming from e.g. Python.
```rs
for _ in 0..0 {
  // ...
} else {
  // ...
}
```
Combined with trying to store it in a let binding, the current diagnostic can be a bit confusing. It mentions let-else and suggests wrapping the loop in parentheses, which the user probably doesn't want. let-else doesn't make sense for `for` and `while` loops, as they are of type `()` (which already is an irrefutable pattern and doesn't need let-else).
<details>
<summary>Current diagnostic</summary>

```rs
error: right curly brace `}` before `else` in a `let...else` statement not allowed
 --> src/main.rs:4:5
  |
4 |     } else {
  |     ^
  |
help: wrap the expression in parentheses
  |
2 ~     let _x = (for _ in 0..0 {
3 |
4 ~     }) else {
  |
```
</details>

Some questions:
- Can the wording for the error message be improved? Would "for...else loops are not allowed" fit better?
- Should we be more "conservative" in case we want to support this in the future (i.e. say "for...else loops are **currently** not allowed/supported")?
- Is there a better way than storing a `&'static str` for the loop type? It is used for substituting the placeholder in the locale file (since it can emit either `for...else` or `while...else`). Maybe there is an enum I could use that I couldn't find
  • Loading branch information
matthiaskrgr authored Mar 1, 2023
2 parents 4ebf78a + 13b8497 commit 75bd7cc
Show file tree
Hide file tree
Showing 20 changed files with 218 additions and 22 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_parse/locales/en-US.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ parse_missing_in_in_for_loop = missing `in` in `for` loop
parse_missing_expression_in_for_loop = missing expression to iterate on in `for` loop
.suggestion = try adding an expression to the `for` loop
parse_loop_else = `{$loop_kind}...else` loops are not supported
.note = consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
.loop_keyword = `else` is attached to this loop
parse_missing_comma_after_match_arm = expected `,` following `match` arm
.suggestion = missing a comma here to end this `match` arm
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,17 @@ pub(crate) struct MissingExpressionInForLoop {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(parse_loop_else)]
#[note]
pub(crate) struct LoopElseNotSupported {
#[primary_span]
pub span: Span,
pub loop_kind: &'static str,
#[label(parse_loop_keyword)]
pub loop_kw: Span,
}

#[derive(Diagnostic)]
#[diag(parse_missing_comma_after_match_arm)]
pub(crate) struct MissingCommaAfterMatchArm {
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2503,9 +2503,27 @@ impl<'a> Parser<'a> {
let (attrs, loop_block) = self.parse_inner_attrs_and_block()?;

let kind = ExprKind::ForLoop(pat, expr, loop_block, opt_label);

self.recover_loop_else("for", lo)?;

Ok(self.mk_expr_with_attrs(lo.to(self.prev_token.span), kind, attrs))
}

/// Recovers from an `else` clause after a loop (`for...else`, `while...else`)
fn recover_loop_else(&mut self, loop_kind: &'static str, loop_kw: Span) -> PResult<'a, ()> {
if self.token.is_keyword(kw::Else) && self.may_recover() {
let else_span = self.token.span;
self.bump();
let else_clause = self.parse_else_expr()?;
self.sess.emit_err(errors::LoopElseNotSupported {
span: else_span.to(else_clause.span),
loop_kind,
loop_kw,
});
}
Ok(())
}

fn error_missing_in_for_loop(&mut self) {
let (span, sub): (_, fn(_) -> _) = if self.token.is_ident_named(sym::of) {
// Possibly using JS syntax (#75311).
Expand All @@ -2530,6 +2548,9 @@ impl<'a> Parser<'a> {
err.span_label(cond.span, "this `while` condition successfully parsed");
err
})?;

self.recover_loop_else("while", lo)?;

Ok(self.mk_expr_with_attrs(
lo.to(self.prev_token.span),
ExprKind::While(cond, body, opt_label),
Expand All @@ -2541,6 +2562,7 @@ impl<'a> Parser<'a> {
fn parse_expr_loop(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'a, P<Expr>> {
let loop_span = self.prev_token.span;
let (attrs, body) = self.parse_inner_attrs_and_block()?;
self.recover_loop_else("loop", lo)?;
Ok(self.mk_expr_with_attrs(
lo.to(self.prev_token.span),
ExprKind::Loop(body, opt_label, loop_span),
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/for/for-else-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
for _ in 0..1 {
//~^ NOTE `else` is attached to this loop
} else {
//~^ ERROR `for...else` loops are not supported
//~| NOTE consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
}
}
17 changes: 17 additions & 0 deletions tests/ui/for/for-else-err.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: `for...else` loops are not supported
--> $DIR/for-else-err.rs:4:7
|
LL | for _ in 0..1 {
| --- `else` is attached to this loop
LL |
LL | } else {
| _______^
LL | |
LL | |
LL | | }
| |_____^
|
= note: consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run

error: aborting due to previous error

8 changes: 8 additions & 0 deletions tests/ui/for/for-else-let-else-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
let _ = for _ in 0..1 {
//~^ NOTE `else` is attached to this loop
} else {
//~^ ERROR `for...else` loops are not supported
//~| NOTE consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
};
}
17 changes: 17 additions & 0 deletions tests/ui/for/for-else-let-else-err.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: `for...else` loops are not supported
--> $DIR/for-else-let-else-err.rs:4:7
|
LL | let _ = for _ in 0..1 {
| --- `else` is attached to this loop
LL |
LL | } else {
| _______^
LL | |
LL | |
LL | | };
| |_____^
|
= note: consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run

error: aborting due to previous error

4 changes: 0 additions & 4 deletions tests/ui/let-else/let-else-brace-before-else.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ fn main() {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
return;
};
let Some(1) = (loop { break Some(1) }) else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
return;
};
let 2 = 1 + (match 1 { n => n }) else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
return;
Expand Down
4 changes: 0 additions & 4 deletions tests/ui/let-else/let-else-brace-before-else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ fn main() {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
return;
};
let Some(1) = loop { break Some(1) } else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
return;
};
let 2 = 1 + match 1 { n => n } else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
return;
Expand Down
17 changes: 3 additions & 14 deletions tests/ui/let-else/let-else-brace-before-else.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,7 @@ LL | let Some(1) = ({ Some(1) }) else {
| + +

error: right curly brace `}` before `else` in a `let...else` statement not allowed
--> $DIR/let-else-brace-before-else.rs:10:40
|
LL | let Some(1) = loop { break Some(1) } else {
| ^
|
help: wrap the expression in parentheses
|
LL | let Some(1) = (loop { break Some(1) }) else {
| + +

error: right curly brace `}` before `else` in a `let...else` statement not allowed
--> $DIR/let-else-brace-before-else.rs:14:34
--> $DIR/let-else-brace-before-else.rs:10:34
|
LL | let 2 = 1 + match 1 { n => n } else {
| ^
Expand All @@ -32,7 +21,7 @@ LL | let 2 = 1 + (match 1 { n => n }) else {
| + +

error: right curly brace `}` before `else` in a `let...else` statement not allowed
--> $DIR/let-else-brace-before-else.rs:18:40
--> $DIR/let-else-brace-before-else.rs:14:40
|
LL | let Some(1) = unsafe { unsafe_fn() } else {
| ^
Expand All @@ -42,5 +31,5 @@ help: wrap the expression in parentheses
LL | let Some(1) = (unsafe { unsafe_fn() }) else {
| + +

error: aborting due to 4 previous errors
error: aborting due to 3 previous errors

10 changes: 10 additions & 0 deletions tests/ui/loops/loop-else-break-with-value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn main() {
let Some(1) = loop {
//~^ NOTE `else` is attached to this loop
break Some(1)
} else {
//~^ ERROR `loop...else` loops are not supported
//~| NOTE consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
return;
};
}
18 changes: 18 additions & 0 deletions tests/ui/loops/loop-else-break-with-value.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: `loop...else` loops are not supported
--> $DIR/loop-else-break-with-value.rs:5:7
|
LL | let Some(1) = loop {
| ---- `else` is attached to this loop
...
LL | } else {
| _______^
LL | |
LL | |
LL | | return;
LL | | };
| |_____^
|
= note: consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run

error: aborting due to previous error

8 changes: 8 additions & 0 deletions tests/ui/loops/loop-else-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
loop {
//~^ NOTE `else` is attached to this loop
} else {
//~^ ERROR `loop...else` loops are not supported
//~| NOTE consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
}
}
17 changes: 17 additions & 0 deletions tests/ui/loops/loop-else-err.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: `loop...else` loops are not supported
--> $DIR/loop-else-err.rs:4:7
|
LL | loop {
| ---- `else` is attached to this loop
LL |
LL | } else {
| _______^
LL | |
LL | |
LL | | }
| |_____^
|
= note: consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run

error: aborting due to previous error

8 changes: 8 additions & 0 deletions tests/ui/loops/loop-else-let-else-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
let _ = loop {
//~^ NOTE `else` is attached to this loop
} else {
//~^ ERROR `loop...else` loops are not supported
//~| NOTE consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
};
}
17 changes: 17 additions & 0 deletions tests/ui/loops/loop-else-let-else-err.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: `loop...else` loops are not supported
--> $DIR/loop-else-let-else-err.rs:4:7
|
LL | let _ = loop {
| ---- `else` is attached to this loop
LL |
LL | } else {
| _______^
LL | |
LL | |
LL | | };
| |_____^
|
= note: consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run

error: aborting due to previous error

8 changes: 8 additions & 0 deletions tests/ui/while/while-else-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
while false {
//~^ NOTE `else` is attached to this loop
} else {
//~^ ERROR `while...else` loops are not supported
//~| NOTE consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
};
}
17 changes: 17 additions & 0 deletions tests/ui/while/while-else-err.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: `while...else` loops are not supported
--> $DIR/while-else-err.rs:4:7
|
LL | while false {
| ----- `else` is attached to this loop
LL |
LL | } else {
| _______^
LL | |
LL | |
LL | | };
| |_____^
|
= note: consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run

error: aborting due to previous error

8 changes: 8 additions & 0 deletions tests/ui/while/while-else-let-else-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
let _ = while false {
//~^ NOTE `else` is attached to this loop
} else {
//~^ ERROR `while...else` loops are not supported
//~| NOTE consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run
};
}
17 changes: 17 additions & 0 deletions tests/ui/while/while-else-let-else-err.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: `while...else` loops are not supported
--> $DIR/while-else-let-else-err.rs:4:7
|
LL | let _ = while false {
| ----- `else` is attached to this loop
LL |
LL | } else {
| _______^
LL | |
LL | |
LL | | };
| |_____^
|
= note: consider moving this `else` clause to a separate `if` statement and use a `bool` variable to control if it should run

error: aborting due to previous error

0 comments on commit 75bd7cc

Please sign in to comment.