Skip to content

If a label is placed on the block of a loop instead of the header, suggest moving it to the header. #138589

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
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
29 changes: 23 additions & 6 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2874,7 +2874,12 @@ impl<'a> Parser<'a> {
first_pat
}

pub(crate) fn maybe_recover_unexpected_block_label(&mut self) -> bool {
/// If `loop_header` is `Some` and an unexpected block label is encountered,
/// it is suggested to be moved just before `loop_header`, else it is suggested to be removed.
pub(crate) fn maybe_recover_unexpected_block_label(
&mut self,
loop_header: Option<Span>,
) -> bool {
// Check for `'a : {`
if !(self.check_lifetime()
&& self.look_ahead(1, |t| *t == token::Colon)
Expand All @@ -2885,16 +2890,28 @@ impl<'a> Parser<'a> {
let label = self.eat_label().expect("just checked if a label exists");
self.bump(); // eat `:`
let span = label.ident.span.to(self.prev_token.span);
self.dcx()
let mut diag = self
.dcx()
.struct_span_err(span, "block label not supported here")
.with_span_label(span, "not supported here")
.with_tool_only_span_suggestion(
.with_span_label(span, "not supported here");
if let Some(loop_header) = loop_header {
diag.multipart_suggestion(
"if you meant to label the loop, move this label before the loop",
vec![
(label.ident.span.until(self.token.span), String::from("")),
(loop_header.shrink_to_lo(), format!("{}: ", label.ident)),
],
Applicability::MachineApplicable,
);
} else {
diag.tool_only_span_suggestion(
label.ident.span.until(self.token.span),
"remove this block label",
"",
Applicability::MachineApplicable,
)
.emit();
);
}
diag.emit();
true
}

Expand Down
36 changes: 25 additions & 11 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2286,7 +2286,7 @@ impl<'a> Parser<'a> {
});
}

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

Expand Down Expand Up @@ -2851,7 +2851,11 @@ impl<'a> Parser<'a> {
));
}

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

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

Expand Down Expand Up @@ -2894,11 +2898,17 @@ impl<'a> Parser<'a> {
err.span_label(lo, "while parsing the condition of this `while` expression");
err
})?;
let (attrs, body) = self.parse_inner_attrs_and_block().map_err(|mut err| {
err.span_label(lo, "while parsing the body of this `while` expression");
err.span_label(cond.span, "this `while` condition successfully parsed");
err
})?;
let (attrs, body) = self
.parse_inner_attrs_and_block(
// Only suggest moving erroneous block label to the loop header
// if there is not already a label there
opt_label.is_none().then_some(lo),
)
.map_err(|mut err| {
err.span_label(lo, "while parsing the body of this `while` expression");
err.span_label(cond.span, "this `while` condition successfully parsed");
err
})?;

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

Expand All @@ -2912,7 +2922,11 @@ impl<'a> Parser<'a> {
/// Parses `loop { ... }` (`loop` token already eaten).
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()?;
let (attrs, body) = self.parse_inner_attrs_and_block(
// Only suggest moving erroneous block label to the loop header
// if there is not already a label there
opt_label.is_none().then_some(lo),
)?;
self.recover_loop_else("loop", lo)?;
Ok(self.mk_expr_with_attrs(
lo.to(self.prev_token.span),
Expand Down Expand Up @@ -2962,7 +2976,7 @@ impl<'a> Parser<'a> {
Applicability::MaybeIncorrect, // speculative
);
}
if self.maybe_recover_unexpected_block_label() {
if self.maybe_recover_unexpected_block_label(None) {
e.cancel();
self.bump();
} else {
Expand Down Expand Up @@ -3376,7 +3390,7 @@ impl<'a> Parser<'a> {

/// Parses a `try {...}` expression (`try` token already eaten).
fn parse_try_block(&mut self, span_lo: Span) -> PResult<'a, P<Expr>> {
let (attrs, body) = self.parse_inner_attrs_and_block()?;
let (attrs, body) = self.parse_inner_attrs_and_block(None)?;
if self.eat_keyword(exp!(Catch)) {
Err(self.dcx().create_err(errors::CatchAfterTry { span: self.prev_token.span }))
} else {
Expand Down Expand Up @@ -3424,7 +3438,7 @@ impl<'a> Parser<'a> {
}
let capture_clause = self.parse_capture_clause()?;
let decl_span = lo.to(self.prev_token.span);
let (attrs, body) = self.parse_inner_attrs_and_block()?;
let (attrs, body) = self.parse_inner_attrs_and_block(None)?;
let kind = ExprKind::Gen(capture_clause, body, kind, decl_span);
Ok(self.mk_expr_with_attrs(lo.to(self.prev_token.span), kind, attrs))
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2529,7 +2529,7 @@ impl<'a> Parser<'a> {
*sig_hi = self.prev_token.span;
(AttrVec::new(), None)
} else if self.check(exp!(OpenBrace)) || self.token.is_whole_block() {
self.parse_block_common(self.token.span, BlockCheckMode::Default, false)
self.parse_block_common(self.token.span, BlockCheckMode::Default, false, None)
.map(|(attrs, body)| (attrs, Some(body)))?
} else if self.token == token::Eq {
// Recover `fn foo() = $expr;`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ impl<'a> Parser<'a> {
self.psess.gated_spans.gate(sym::inline_const_pat, span);
}
self.expect_keyword(exp!(Const))?;
let (attrs, blk) = self.parse_inner_attrs_and_block()?;
let (attrs, blk) = self.parse_inner_attrs_and_block(None)?;
let anon_const = AnonConst {
id: DUMMY_NODE_ID,
value: self.mk_expr(blk.span, ExprKind::Block(blk, None)),
Expand Down
22 changes: 16 additions & 6 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ impl<'a> Parser<'a> {

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

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

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

let maybe_ident = self.prev_token.clone();
self.maybe_recover_unexpected_block_label();
self.maybe_recover_unexpected_block_label(loop_header);
if !self.eat(exp!(OpenBrace)) {
return self.error_block_no_opening_brace();
}
Expand Down
90 changes: 90 additions & 0 deletions tests/ui/loops/label-on-block-suggest-move.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// see https://github.com/rust-lang/rust/issues/138585
#![allow(break_with_label_and_loop)] // doesn't work locally

fn main() {
loop 'a: {}
//~^ ERROR: block label not supported here
//~| HELP: if you meant to label the loop, move this label before the loop
while false 'a: {}
//~^ ERROR: block label not supported here
//~| HELP: if you meant to label the loop, move this label before the loop
for i in [0] 'a: {}
//~^ ERROR: block label not supported here
//~| HELP: if you meant to label the loop, move this label before the loop
'a: loop {
// first block is parsed as the break expr's value with or without parens
while break 'a 'b: {} 'c: {}
//~^ ERROR: block label not supported here
//~| HELP: if you meant to label the loop, move this label before the loop
while break 'a ('b: {}) 'c: {}
//~^ ERROR: block label not supported here
//~| HELP: if you meant to label the loop, move this label before the loop

// without the parens, the first block is parsed as the while-loop's body
// (see the 'no errors' section)
// #[allow(break_with_label_and_loop)] (doesn't work locally)
while (break 'a {}) 'c: {}
//~^ ERROR: block label not supported here
//~| HELP: if you meant to label the loop, move this label before the loop
}

// do not suggest moving the label if there is already a label on the loop
'a: loop 'b: {}
//~^ ERROR: block label not supported here
//~| HELP: remove this block label
'a: while false 'b: {}
//~^ ERROR: block label not supported here
//~| HELP: remove this block label
'a: for i in [0] 'b: {}
//~^ ERROR: block label not supported here
//~| HELP: remove this block label
'a: loop {
// first block is parsed as the break expr's value with or without parens
'd: while break 'a 'b: {} 'c: {}
//~^ ERROR: block label not supported here
//~| HELP: remove this block label
'd: while break 'a ('b: {}) 'c: {}
//~^ ERROR: block label not supported here
//~| HELP: remove this block label

// without the parens, the first block is parsed as the while-loop's body
// (see the 'no errors' section)
// #[allow(break_with_label_and_loop)] (doesn't work locally)
'd: while (break 'a {}) 'c: {}
//~^ ERROR: block label not supported here
//~| HELP: remove this block label
}

// no errors
loop { 'a: {} }
'a: loop { 'b: {} }
while false { 'a: {} }
'a: while false { 'b: {} }
for i in [0] { 'a: {} }
'a: for i in [0] { 'b: {} }
'a: {}
'a: { 'b: {} }
'a: loop {
// first block is parsed as the break expr's value if it is a labeled block
while break 'a 'b: {} {}
'd: while break 'a 'b: {} {}
while break 'a ('b: {}) {}
'd: while break 'a ('b: {}) {}
// first block is parsed as the while-loop's body if it has no label
// (the break expr is parsed as having no value),
// so the second block is a normal stmt-block, and the label is allowed
while break 'a {} 'c: {}
while break 'a {} {}
'd: while break 'a {} 'c: {}
'd: while break 'a {} {}
}

// unrelated errors that should not be affected
'a: 'b: {}
//~^ ERROR: expected `while`, `for`, `loop` or `{` after a label
//~| HELP: consider removing the label
loop { while break 'b: {} {} }
//~^ ERROR: parentheses are required around this expression to avoid confusion with a labeled break expression
//~| HELP: wrap the expression in parentheses
//~| ERROR: `break` or `continue` with no label in the condition of a `while` loop [E0590]
}
Loading
Loading