Skip to content

Suggest adding {} for 'label: non_block_expr #97759

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 4 commits into from
Jun 6, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
30 changes: 28 additions & 2 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ use rustc_ast::tokenstream::Spacing;
use rustc_ast::util::classify;
use rustc_ast::util::literal::LitError;
use rustc_ast::util::parser::{prec_let_scrutinee_needs_par, AssocOp, Fixity};
use rustc_ast::StmtKind;
use rustc_ast::{self as ast, AttrStyle, AttrVec, CaptureBy, ExprField, Lit, UnOp, DUMMY_NODE_ID};
use rustc_ast::{AnonConst, BinOp, BinOpKind, FnDecl, FnRetTy, MacCall, Param, Ty, TyKind};
use rustc_ast::{Arm, Async, BlockCheckMode, Expr, ExprKind, Label, Movability, RangeLimits};
use rustc_ast_pretty::pprust;
use rustc_data_structures::thin_vec::ThinVec;
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, PResult};
use rustc_session::lint::builtin::BREAK_WITH_LABEL_AND_LOOP;
use rustc_session::lint::BuiltinLintDiagnostics;
Expand Down Expand Up @@ -1548,9 +1550,33 @@ impl<'a> Parser<'a> {
Ok(self.mk_expr_err(lo))
} else {
let msg = "expected `while`, `for`, `loop` or `{` after a label";
self.struct_span_err(self.token.span, msg).span_label(self.token.span, msg).emit();

let mut err = self.struct_span_err(self.token.span, msg);
err.span_label(self.token.span, msg);

// Continue as an expression in an effort to recover on `'label: non_block_expr`.
self.parse_expr()
let expr = self.parse_expr().map(|expr| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you actually use ? here, move the body of the closure to below, and remove the ? on line 1614?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other if branches also return Results, so I don't think it makes sense to do that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, oops, I thought there was only two if branches. I should've clicked the button to reveal the lines above 😓

let span = expr.span;
let sugg_msg = "consider enclosing expression in a block";
let suggestions = vec![
(span.shrink_to_lo(), "{".to_owned()),
(span.shrink_to_hi(), "}".to_owned()),
];

err.multipart_suggestion_verbose(
sugg_msg,
suggestions,
Applicability::MachineApplicable,
);

// Replace `'label: non_block_expr` with `'label: {non_block_expr}` in order to supress future errors about `break 'label`.
let stmt = self.mk_stmt(span, StmtKind::Expr(expr));
let blk = self.mk_block(vec![stmt], BlockCheckMode::Default, span);
self.mk_expr(span, ExprKind::Block(blk, label), ThinVec::new())
});

err.emit();
expr
}?;

if !ate_colon && consume_colon {
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/parser/labeled-no-colon-expr.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ error: expected `while`, `for`, `loop` or `{` after a label
|
LL | 'l4 0;
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: consider enclosing expression in a block
|
LL | 'l4 {0};
| + +

error: labeled expression must be followed by `:`
--> $DIR/labeled-no-colon-expr.rs:8:9
Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/parser/recover-labeled-non-block-expr.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// run-rustfix
#![feature(label_break_value)]
fn main() {
#[allow(unused_labels)]
'label: {1 + 1}; //~ ERROR expected `while`, `for`, `loop` or `{` after a label

'label: {match () { () => break 'label, }}; //~ ERROR expected `while`, `for`, `loop` or `{` after a label

let x = 1;
let _i = 'label: {match x { //~ ERROR expected `while`, `for`, `loop` or `{` after a label
0 => 42,
1 if false => break 'label 17,
1 => {
if true {
break 'label 13
} else {
break 'label 0;
}
}
_ => 1,
}};

let other = 3;
let _val = 'label: {(1, if other == 3 { break 'label (2, 3) } else { other })}; //~ ERROR expected `while`, `for`, `loop` or `{` after a label
}
22 changes: 21 additions & 1 deletion src/test/ui/parser/recover-labeled-non-block-expr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
// run-rustfix
#![feature(label_break_value)]
fn main() {
#[allow(unused_labels)]
'label: 1 + 1; //~ ERROR expected `while`, `for`, `loop` or `{` after a label

let _recovery_witness: () = 0; //~ ERROR mismatched types
'label: match () { () => break 'label, }; //~ ERROR expected `while`, `for`, `loop` or `{` after a label

let x = 1;
let _i = 'label: match x { //~ ERROR expected `while`, `for`, `loop` or `{` after a label
0 => 42,
1 if false => break 'label 17,
1 => {
if true {
break 'label 13
} else {
break 'label 0;
}
}
_ => 1,
};

let other = 3;
let _val = 'label: (1, if other == 3 { break 'label (2, 3) } else { other }); //~ ERROR expected `while`, `for`, `loop` or `{` after a label
}
52 changes: 43 additions & 9 deletions src/test/ui/parser/recover-labeled-non-block-expr.stderr
Original file line number Diff line number Diff line change
@@ -1,17 +1,51 @@
error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/recover-labeled-non-block-expr.rs:2:13
--> $DIR/recover-labeled-non-block-expr.rs:5:13
|
LL | 'label: 1 + 1;
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: consider enclosing expression in a block
|
LL | 'label: {1 + 1};
| + +

error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/recover-labeled-non-block-expr.rs:7:13
|
LL | 'label: match () { () => break 'label, };
| ^^^^^ expected `while`, `for`, `loop` or `{` after a label
|
help: consider enclosing expression in a block
|
LL | 'label: {match () { () => break 'label, }};
| + +

error[E0308]: mismatched types
--> $DIR/recover-labeled-non-block-expr.rs:4:33
error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/recover-labeled-non-block-expr.rs:10:22
|
LL | let _i = 'label: match x {
| ^^^^^ expected `while`, `for`, `loop` or `{` after a label
|
help: consider enclosing expression in a block
|
LL ~ let _i = 'label: {match x {
LL | 0 => 42,
LL | 1 if false => break 'label 17,
LL | 1 => {
LL | if true {
LL | break 'label 13
...
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can someone explain where the second part of the suggestion (}) went?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestions only render up to 5 lines i think

Copy link
Member Author

@WaffleLapkin WaffleLapkin Jun 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that it'd show multiple parts :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a bug in the rendering code. Your suggestion is still MachineApplicable so it will apply just fine, it's just the rendered code will be cut off.

I don't know if there's much you can do to fix it in this PR specifically, but maybe you could file a bug, or look into fixing the rendering code to omit lines in the middle?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll look into it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a "bug", but rather a non-implementation of the logic for multiple "windows" into the code, like span labels have.


error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/recover-labeled-non-block-expr.rs:24:24
|
LL | let _val = 'label: (1, if other == 3 { break 'label (2, 3) } else { other });
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: consider enclosing expression in a block
|
LL | let _recovery_witness: () = 0;
| -- ^ expected `()`, found integer
| |
| expected due to this
LL | let _val = 'label: {(1, if other == 3 { break 'label (2, 3) } else { other })};
| + +

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

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