Skip to content

Commit f478853

Browse files
zachs18zachs18
committed
If a label is placed on the block of a loop instead of the header, suggest moving it to the header.
1 parent 227690a commit f478853

File tree

7 files changed

+296
-25
lines changed

7 files changed

+296
-25
lines changed

‎compiler/rustc_parse/src/parser/diagnostics.rs‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2874,7 +2874,12 @@ impl<'a> Parser<'a> {
28742874
first_pat
28752875
}
28762876

2877-
pub(crate) fn maybe_recover_unexpected_block_label(&mut self) -> bool {
2877+
/// If `loop_header` is `Some` and an unexpected block label is encountered,
2878+
/// it is suggested to be moved just before `loop_header`, else it is suggested to be removed.
2879+
pub(crate) fn maybe_recover_unexpected_block_label(
2880+
&mut self,
2881+
loop_header: Option<Span>,
2882+
) -> bool {
28782883
// Check for `'a : {`
28792884
if !(self.check_lifetime()
28802885
&& self.look_ahead(1, |t| *t == token::Colon)
@@ -2885,16 +2890,28 @@ impl<'a> Parser<'a> {
28852890
let label = self.eat_label().expect("just checked if a label exists");
28862891
self.bump(); // eat `:`
28872892
let span = label.ident.span.to(self.prev_token.span);
2888-
self.dcx()
2893+
let mut diag = self
2894+
.dcx()
28892895
.struct_span_err(span, "block label not supported here")
2890-
.with_span_label(span, "not supported here")
2891-
.with_tool_only_span_suggestion(
2896+
.with_span_label(span, "not supported here");
2897+
if let Some(loop_header) = loop_header {
2898+
diag.multipart_suggestion(
2899+
"if you meant to label the loop, move this label before the loop",
2900+
vec![
2901+
(label.ident.span.until(self.token.span), String::from("")),
2902+
(loop_header.shrink_to_lo(), format!("{}: ", label.ident)),
2903+
],
2904+
Applicability::MachineApplicable,
2905+
);
2906+
} else {
2907+
diag.tool_only_span_suggestion(
28922908
label.ident.span.until(self.token.span),
28932909
"remove this block label",
28942910
"",
28952911
Applicability::MachineApplicable,
2896-
)
2897-
.emit();
2912+
);
2913+
}
2914+
diag.emit();
28982915
true
28992916
}
29002917

‎compiler/rustc_parse/src/parser/expr.rs‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2286,7 +2286,7 @@ impl<'a> Parser<'a> {
22862286
});
22872287
}
22882288

2289-
let (attrs, blk) = self.parse_block_common(lo, blk_mode, true)?;
2289+
let (attrs, blk) = self.parse_block_common(lo, blk_mode, true, None)?;
22902290
Ok(self.mk_expr_with_attrs(blk.span, ExprKind::Block(blk, opt_label), attrs))
22912291
}
22922292

@@ -2851,7 +2851,11 @@ impl<'a> Parser<'a> {
28512851
));
28522852
}
28532853

2854-
let (attrs, loop_block) = self.parse_inner_attrs_and_block()?;
2854+
let (attrs, loop_block) = self.parse_inner_attrs_and_block(
2855+
// Only suggest moving erroneous block label to the loop header
2856+
// if there is not already a label there
2857+
opt_label.is_none().then_some(lo),
2858+
)?;
28552859

28562860
let kind = ExprKind::ForLoop { pat, iter: expr, body: loop_block, label: opt_label, kind };
28572861

@@ -2894,11 +2898,17 @@ impl<'a> Parser<'a> {
28942898
err.span_label(lo, "while parsing the condition of this `while` expression");
28952899
err
28962900
})?;
2897-
let (attrs, body) = self.parse_inner_attrs_and_block().map_err(|mut err| {
2898-
err.span_label(lo, "while parsing the body of this `while` expression");
2899-
err.span_label(cond.span, "this `while` condition successfully parsed");
2900-
err
2901-
})?;
2901+
let (attrs, body) = self
2902+
.parse_inner_attrs_and_block(
2903+
// Only suggest moving erroneous block label to the loop header
2904+
// if there is not already a label there
2905+
opt_label.is_none().then_some(lo),
2906+
)
2907+
.map_err(|mut err| {
2908+
err.span_label(lo, "while parsing the body of this `while` expression");
2909+
err.span_label(cond.span, "this `while` condition successfully parsed");
2910+
err
2911+
})?;
29022912

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

@@ -2912,7 +2922,11 @@ impl<'a> Parser<'a> {
29122922
/// Parses `loop { ... }` (`loop` token already eaten).
29132923
fn parse_expr_loop(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'a, P<Expr>> {
29142924
let loop_span = self.prev_token.span;
2915-
let (attrs, body) = self.parse_inner_attrs_and_block()?;
2925+
let (attrs, body) = self.parse_inner_attrs_and_block(
2926+
// Only suggest moving erroneous block label to the loop header
2927+
// if there is not already a label there
2928+
opt_label.is_none().then_some(lo),
2929+
)?;
29162930
self.recover_loop_else("loop", lo)?;
29172931
Ok(self.mk_expr_with_attrs(
29182932
lo.to(self.prev_token.span),
@@ -2962,7 +2976,7 @@ impl<'a> Parser<'a> {
29622976
Applicability::MaybeIncorrect, // speculative
29632977
);
29642978
}
2965-
if self.maybe_recover_unexpected_block_label() {
2979+
if self.maybe_recover_unexpected_block_label(None) {
29662980
e.cancel();
29672981
self.bump();
29682982
} else {
@@ -3376,7 +3390,7 @@ impl<'a> Parser<'a> {
33763390

33773391
/// Parses a `try {...}` expression (`try` token already eaten).
33783392
fn parse_try_block(&mut self, span_lo: Span) -> PResult<'a, P<Expr>> {
3379-
let (attrs, body) = self.parse_inner_attrs_and_block()?;
3393+
let (attrs, body) = self.parse_inner_attrs_and_block(None)?;
33803394
if self.eat_keyword(exp!(Catch)) {
33813395
Err(self.dcx().create_err(errors::CatchAfterTry { span: self.prev_token.span }))
33823396
} else {
@@ -3424,7 +3438,7 @@ impl<'a> Parser<'a> {
34243438
}
34253439
let capture_clause = self.parse_capture_clause()?;
34263440
let decl_span = lo.to(self.prev_token.span);
3427-
let (attrs, body) = self.parse_inner_attrs_and_block()?;
3441+
let (attrs, body) = self.parse_inner_attrs_and_block(None)?;
34283442
let kind = ExprKind::Gen(capture_clause, body, kind, decl_span);
34293443
Ok(self.mk_expr_with_attrs(lo.to(self.prev_token.span), kind, attrs))
34303444
}

‎compiler/rustc_parse/src/parser/item.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2529,7 +2529,7 @@ impl<'a> Parser<'a> {
25292529
*sig_hi = self.prev_token.span;
25302530
(AttrVec::new(), None)
25312531
} else if self.check(exp!(OpenBrace)) || self.token.is_whole_block() {
2532-
self.parse_block_common(self.token.span, BlockCheckMode::Default, false)
2532+
self.parse_block_common(self.token.span, BlockCheckMode::Default, false, None)
25332533
.map(|(attrs, body)| (attrs, Some(body)))?
25342534
} else if self.token == token::Eq {
25352535
// Recover `fn foo() = $expr;`.

‎compiler/rustc_parse/src/parser/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,7 @@ impl<'a> Parser<'a> {
13741374
self.psess.gated_spans.gate(sym::inline_const_pat, span);
13751375
}
13761376
self.expect_keyword(exp!(Const))?;
1377-
let (attrs, blk) = self.parse_inner_attrs_and_block()?;
1377+
let (attrs, blk) = self.parse_inner_attrs_and_block(None)?;
13781378
let anon_const = AnonConst {
13791379
id: DUMMY_NODE_ID,
13801380
value: self.mk_expr(blk.span, ExprKind::Block(blk, None)),

‎compiler/rustc_parse/src/parser/stmt.rs‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ impl<'a> Parser<'a> {
482482

483483
/// Parses a block. No inner attributes are allowed.
484484
pub fn parse_block(&mut self) -> PResult<'a, P<Block>> {
485-
let (attrs, block) = self.parse_inner_attrs_and_block()?;
485+
let (attrs, block) = self.parse_inner_attrs_and_block(None)?;
486486
if let [.., last] = &*attrs {
487487
let suggest_to_outer = match &last.kind {
488488
ast::AttrKind::Normal(attr) => attr.item.is_valid_for_outer_style(),
@@ -660,22 +660,32 @@ impl<'a> Parser<'a> {
660660
Err(self.error_block_no_opening_brace_msg(Cow::from(msg)))
661661
}
662662

663-
/// Parses a block. Inner attributes are allowed.
664-
pub(super) fn parse_inner_attrs_and_block(&mut self) -> PResult<'a, (AttrVec, P<Block>)> {
665-
self.parse_block_common(self.token.span, BlockCheckMode::Default, true)
663+
/// Parses a block. Inner attributes are allowed, block labels are not.
664+
///
665+
/// If `loop_header` is `Some` and an unexpected block label is encountered,
666+
/// it is suggested to be moved just before `loop_header`, else it is suggested to be removed.
667+
pub(super) fn parse_inner_attrs_and_block(
668+
&mut self,
669+
loop_header: Option<Span>,
670+
) -> PResult<'a, (AttrVec, P<Block>)> {
671+
self.parse_block_common(self.token.span, BlockCheckMode::Default, true, loop_header)
666672
}
667673

668-
/// Parses a block. Inner attributes are allowed.
674+
/// Parses a block. Inner attributes are allowed, block labels are not.
675+
///
676+
/// If `loop_header` is `Some` and an unexpected block label is encountered,
677+
/// it is suggested to be moved just before `loop_header`, else it is suggested to be removed.
669678
pub(super) fn parse_block_common(
670679
&mut self,
671680
lo: Span,
672681
blk_mode: BlockCheckMode,
673682
can_be_struct_literal: bool,
683+
loop_header: Option<Span>,
674684
) -> PResult<'a, (AttrVec, P<Block>)> {
675685
maybe_whole!(self, NtBlock, |block| (AttrVec::new(), block));
676686

677687
let maybe_ident = self.prev_token.clone();
678-
self.maybe_recover_unexpected_block_label();
688+
self.maybe_recover_unexpected_block_label(loop_header);
679689
if !self.eat(exp!(OpenBrace)) {
680690
return self.error_block_no_opening_brace();
681691
}

‎tests/ui/loops/label-on-block-suggest-move.rs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// see https://github.com/rust-lang/rust/issues/138585
2+
#![allow(break_with_label_and_loop)] // doesn't work locally
3+
4+
fn main() {
5+
loop 'a: {}
6+
//~^ ERROR: block label not supported here
7+
//~| HELP: if you meant to label the loop, move this label before the loop
8+
while false 'a: {}
9+
//~^ ERROR: block label not supported here
10+
//~| HELP: if you meant to label the loop, move this label before the loop
11+
for i in [0] 'a: {}
12+
//~^ ERROR: block label not supported here
13+
//~| HELP: if you meant to label the loop, move this label before the loop
14+
'a: loop {
15+
// first block is parsed as the break expr's value with or without parens
16+
while break 'a 'b: {} 'c: {}
17+
//~^ ERROR: block label not supported here
18+
//~| HELP: if you meant to label the loop, move this label before the loop
19+
while break 'a ('b: {}) 'c: {}
20+
//~^ ERROR: block label not supported here
21+
//~| HELP: if you meant to label the loop, move this label before the loop
22+
23+
// without the parens, the first block is parsed as the while-loop's body
24+
// (see the 'no errors' section)
25+
// #[allow(break_with_label_and_loop)] (doesn't work locally)
26+
while (break 'a {}) 'c: {}
27+
//~^ ERROR: block label not supported here
28+
//~| HELP: if you meant to label the loop, move this label before the loop
29+
}
30+
31+
// do not suggest moving the label if there is already a label on the loop
32+
'a: loop 'b: {}
33+
//~^ ERROR: block label not supported here
34+
//~| HELP: remove this block label
35+
'a: while false 'b: {}
36+
//~^ ERROR: block label not supported here
37+
//~| HELP: remove this block label
38+
'a: for i in [0] 'b: {}
39+
//~^ ERROR: block label not supported here
40+
//~| HELP: remove this block label
41+
'a: loop {
42+
// first block is parsed as the break expr's value with or without parens
43+
'd: while break 'a 'b: {} 'c: {}
44+
//~^ ERROR: block label not supported here
45+
//~| HELP: remove this block label
46+
'd: while break 'a ('b: {}) 'c: {}
47+
//~^ ERROR: block label not supported here
48+
//~| HELP: remove this block label
49+
50+
// without the parens, the first block is parsed as the while-loop's body
51+
// (see the 'no errors' section)
52+
// #[allow(break_with_label_and_loop)] (doesn't work locally)
53+
'd: while (break 'a {}) 'c: {}
54+
//~^ ERROR: block label not supported here
55+
//~| HELP: remove this block label
56+
}
57+
58+
// no errors
59+
loop { 'a: {} }
60+
'a: loop { 'b: {} }
61+
while false { 'a: {} }
62+
'a: while false { 'b: {} }
63+
for i in [0] { 'a: {} }
64+
'a: for i in [0] { 'b: {} }
65+
'a: {}
66+
'a: { 'b: {} }
67+
'a: loop {
68+
// first block is parsed as the break expr's value if it is a labeled block
69+
while break 'a 'b: {} {}
70+
'd: while break 'a 'b: {} {}
71+
while break 'a ('b: {}) {}
72+
'd: while break 'a ('b: {}) {}
73+
// first block is parsed as the while-loop's body if it has no label
74+
// (the break expr is parsed as having no value),
75+
// so the second block is a normal stmt-block, and the label is allowed
76+
while break 'a {} 'c: {}
77+
while break 'a {} {}
78+
'd: while break 'a {} 'c: {}
79+
'd: while break 'a {} {}
80+
}
81+
82+
// unrelated errors that should not be affected
83+
'a: 'b: {}
84+
//~^ ERROR: expected `while`, `for`, `loop` or `{` after a label
85+
//~| HELP: consider removing the label
86+
loop { while break 'b: {} {} }
87+
//~^ ERROR: parentheses are required around this expression to avoid confusion with a labeled break expression
88+
//~| HELP: wrap the expression in parentheses
89+
//~| ERROR: `break` or `continue` with no label in the condition of a `while` loop [E0590]
90+
}

0 commit comments

Comments
 (0)