Skip to content

Commit

Permalink
For OutsideLoop we should not suggest add 'block label in if bloc…
Browse files Browse the repository at this point in the history
…k, or we wiil get another err: block label not supported here.

fixes #123261
  • Loading branch information
surechen committed Apr 11, 2024
1 parent aa1c459 commit 8dce3a9
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 39 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ pub struct BreakInsideAsyncBlock<'a> {
pub struct OutsideLoop<'a> {
#[primary_span]
#[label]
pub span: Span,
pub spans: Vec<Span>,
pub name: &'a str,
pub is_break: bool,
#[subdiagnostic]
Expand All @@ -1103,7 +1103,7 @@ pub struct OutsideLoopSuggestion {
#[suggestion_part(code = "'block: ")]
pub block_span: Span,
#[suggestion_part(code = " 'block")]
pub break_span: Span,
pub break_spans: Vec<Span>,
}

#[derive(Diagnostic)]
Expand Down
122 changes: 102 additions & 20 deletions compiler/rustc_passes/src/loops.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use rustc_data_structures::fx::FxHashMap;
use Context::*;

use rustc_hir as hir;
Expand All @@ -24,22 +25,40 @@ enum Context {
Closure(Span),
AsyncClosure(Span),
UnlabeledBlock(Span),
IfUnlabeledBlock(Span),
LabeledBlock,
Constant,
}

#[derive(Copy, Clone)]
#[derive(Clone)]
struct BlockInfo {
name: String,
spans: Vec<Span>,
suggs: Vec<Span>,
}

#[derive(Clone)]
struct CheckLoopVisitor<'a, 'tcx> {
sess: &'a Session,
tcx: TyCtxt<'tcx>,
cx: Context,
// Used for diagnostic like when in a `if` block with some `break`s,
// we should not suggest adding `'block` label in `if` block,
// we can back to outer block and add label there.
cx_stack: Vec<Context>,
blocks: Vec<Span>,
block_breaks: FxHashMap<Span, BlockInfo>,
}

fn check_mod_loops(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
tcx.hir().visit_item_likes_in_module(
module_def_id,
&mut CheckLoopVisitor { sess: tcx.sess, tcx, cx: Normal },
);
let mut check = CheckLoopVisitor {
sess: tcx.sess,
tcx,
cx_stack: vec![Normal],
blocks: Default::default(),
block_breaks: Default::default(),
};
tcx.hir().visit_item_likes_in_module(module_def_id, &mut check);
check.report_outside_loop_error();
}

pub(crate) fn provide(providers: &mut Providers) {
Expand Down Expand Up @@ -82,6 +101,35 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {

fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) {
match e.kind {
hir::ExprKind::If(cond, then, else_opt) => {
self.visit_expr(cond);
if let hir::ExprKind::Block(ref b, None) = then.kind
&& matches!(
self.cx_stack.last(),
Some(&Normal) | Some(&Constant) | Some(&UnlabeledBlock(_)) | Some(&IfUnlabeledBlock(_))
)
{
self.with_context(IfUnlabeledBlock(b.span.shrink_to_lo()), |v| {
v.visit_block(b)
});
} else {
self.visit_expr(then);
}
if let Some(else_expr) = else_opt {
if let hir::ExprKind::Block(ref b, None) = else_expr.kind
&& matches!(
self.cx_stack.last(),
Some(&Normal) | Some(&Constant) | Some(&UnlabeledBlock(_)) | Some(&IfUnlabeledBlock(_))
)
{
self.with_context(IfUnlabeledBlock(b.span.shrink_to_lo()), |v| {
v.visit_block(b)
});
} else {
self.visit_expr(else_expr);
}
}
}
hir::ExprKind::Loop(ref b, _, source, _) => {
self.with_context(Loop(source), |v| v.visit_block(b));
}
Expand All @@ -102,11 +150,14 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
hir::ExprKind::Block(ref b, Some(_label)) => {
self.with_context(LabeledBlock, |v| v.visit_block(b));
}
hir::ExprKind::Block(ref b, None) if matches!(self.cx, Fn) => {
hir::ExprKind::Block(ref b, None) if matches!(self.cx_stack.last(), Some(&Fn)) => {
self.with_context(Normal, |v| v.visit_block(b));
}
hir::ExprKind::Block(ref b, None)
if matches!(self.cx, Normal | Constant | UnlabeledBlock(_)) =>
if matches!(
self.cx_stack.last(),
Some(&Normal) | Some(&Constant) | Some(&UnlabeledBlock(_))
) =>
{
self.with_context(UnlabeledBlock(b.span.shrink_to_lo()), |v| v.visit_block(b));
}
Expand Down Expand Up @@ -179,7 +230,7 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
Some(label) => sp_lo.with_hi(label.ident.span.hi()),
None => sp_lo.shrink_to_lo(),
};
self.require_break_cx("break", e.span, label_sp);
self.require_break_cx("break", e.span, label_sp, self.cx_stack.len() - 1);
}
hir::ExprKind::Continue(destination) => {
self.require_label_in_labeled_block(e.span, &destination, "continue");
Expand All @@ -201,7 +252,7 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
}
Err(_) => {}
}
self.require_break_cx("continue", e.span, e.span)
self.require_break_cx("continue", e.span, e.span, self.cx_stack.len() - 1)
}
_ => intravisit::walk_expr(self, e),
}
Expand All @@ -213,15 +264,14 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> {
where
F: FnOnce(&mut CheckLoopVisitor<'a, 'hir>),
{
let old_cx = self.cx;
self.cx = cx;
self.cx_stack.push(cx);
f(self);
self.cx = old_cx;
self.cx_stack.pop();
}

fn require_break_cx(&self, name: &str, span: Span, break_span: Span) {
fn require_break_cx(&mut self, name: &str, span: Span, break_span: Span, cx_pos: usize) {
let is_break = name == "break";
match self.cx {
match self.cx_stack[cx_pos] {
LabeledBlock | Loop(_) => {}
Closure(closure_span) => {
self.sess.dcx().emit_err(BreakInsideClosure { span, closure_span, name });
Expand All @@ -230,11 +280,27 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> {
self.sess.dcx().emit_err(BreakInsideAsyncBlock { span, closure_span, name });
}
UnlabeledBlock(block_span) if is_break && block_span.eq_ctxt(break_span) => {
let suggestion = Some(OutsideLoopSuggestion { block_span, break_span });
self.sess.dcx().emit_err(OutsideLoop { span, name, is_break, suggestion });
let block = self.block_breaks.entry(block_span).or_insert_with(|| {
self.blocks.push(block_span);
BlockInfo {
name: name.to_string(),
spans: vec![],
suggs: vec![],
}
});
block.spans.push(span);
block.suggs.push(break_span);
}
IfUnlabeledBlock(_) if is_break => {
self.require_break_cx(name, span, break_span, cx_pos - 1);
}
Normal | Constant | Fn | UnlabeledBlock(_) => {
self.sess.dcx().emit_err(OutsideLoop { span, name, is_break, suggestion: None });
Normal | Constant | Fn | UnlabeledBlock(_) | IfUnlabeledBlock(_) => {
self.sess.dcx().emit_err(OutsideLoop {
spans: vec![span],
name,
is_break,
suggestion: None,
});
}
}
}
Expand All @@ -246,12 +312,28 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> {
cf_type: &str,
) -> bool {
if !span.is_desugaring(DesugaringKind::QuestionMark)
&& self.cx == LabeledBlock
&& self.cx_stack.last() == Some(&LabeledBlock)
&& label.label.is_none()
{
self.sess.dcx().emit_err(UnlabeledInLabeledBlock { span, cf_type });
return true;
}
false
}

fn report_outside_loop_error(&mut self) {
self.blocks.iter().for_each(|s| {
if let Some(block) = self.block_breaks.get(s) {
self.sess.dcx().emit_err(OutsideLoop {
spans: block.spans.clone(),
name: &block.name,
is_break: true,
suggestion: Some(OutsideLoopSuggestion {
block_span: *s,
break_spans: block.suggs.clone(),
}),
});
}
});
}
}
31 changes: 31 additions & 0 deletions tests/ui/loops/loop-if-else-break-issue-123261.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//@ run-rustfix

#![allow(unused)]

fn main() {
let n = 1;
let m = 2;
let x = 'block: {
if n == 0 {
break 'block 1; //~ ERROR [E0268]
} else {
break 'block 2;
}
};

let y = 'block: {
if n == 0 {
break 'block 1; //~ ERROR [E0268]
}
break 'block 0;
};

let z = 'block: {
if n == 0 {
if m > 1 {
break 'block 3; //~ ERROR [E0268]
}
}
break 'block 1;
};
}
31 changes: 31 additions & 0 deletions tests/ui/loops/loop-if-else-break-issue-123261.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//@ run-rustfix

#![allow(unused)]

fn main() {
let n = 1;
let m = 2;
let x = {
if n == 0 {
break 1; //~ ERROR [E0268]
} else {
break 2;
}
};

let y = {
if n == 0 {
break 1; //~ ERROR [E0268]
}
break 0;
};

let z = {
if n == 0 {
if m > 1 {
break 3; //~ ERROR [E0268]
}
}
break 1;
};
}
59 changes: 59 additions & 0 deletions tests/ui/loops/loop-if-else-break-issue-123261.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
error[E0268]: `break` outside of a loop or labeled block
--> $DIR/loop-if-else-break-issue-123261.rs:10:13
|
LL | break 1;
| ^^^^^^^ cannot `break` outside of a loop or labeled block
LL | } else {
LL | break 2;
| ^^^^^^^ cannot `break` outside of a loop or labeled block
|
help: consider labeling this block to be able to break within it
|
LL ~ let x = 'block: {
LL | if n == 0 {
LL ~ break 'block 1;
LL | } else {
LL ~ break 'block 2;
|

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/loop-if-else-break-issue-123261.rs:18:13
|
LL | break 1;
| ^^^^^^^ cannot `break` outside of a loop or labeled block
LL | }
LL | break 0;
| ^^^^^^^ cannot `break` outside of a loop or labeled block
|
help: consider labeling this block to be able to break within it
|
LL ~ let y = 'block: {
LL | if n == 0 {
LL ~ break 'block 1;
LL | }
LL ~ break 'block 0;
|

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/loop-if-else-break-issue-123261.rs:26:17
|
LL | break 3;
| ^^^^^^^ cannot `break` outside of a loop or labeled block
...
LL | break 1;
| ^^^^^^^ cannot `break` outside of a loop or labeled block
|
help: consider labeling this block to be able to break within it
|
LL ~ let z = 'block: {
LL | if n == 0 {
LL | if m > 1 {
LL ~ break 'block 3;
LL | }
LL | }
LL ~ break 'block 1;
|

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0268`.
34 changes: 17 additions & 17 deletions tests/ui/parser/break-in-unlabeled-block-in-macro.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,23 @@ LL | foo!(());
|
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/break-in-unlabeled-block-in-macro.rs:33:17
|
LL | foo!(=> break ());
| ^^^^^^^^ cannot `break` outside of a loop or labeled block

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/break-in-unlabeled-block-in-macro.rs:38:17
|
LL | break ()
| ^^^^^^^^ cannot `break` outside of a loop or labeled block
...
LL | bar!()
| ------ in this macro invocation
|
= note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/break-in-unlabeled-block-in-macro.rs:27:19
|
Expand Down Expand Up @@ -47,23 +64,6 @@ help: consider labeling this block to be able to break within it
LL | 'block: { break 'block $e; }
| +++++++ ++++++

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/break-in-unlabeled-block-in-macro.rs:33:17
|
LL | foo!(=> break ());
| ^^^^^^^^ cannot `break` outside of a loop or labeled block

error[E0268]: `break` outside of a loop or labeled block
--> $DIR/break-in-unlabeled-block-in-macro.rs:38:17
|
LL | break ()
| ^^^^^^^^ cannot `break` outside of a loop or labeled block
...
LL | bar!()
| ------ in this macro invocation
|
= note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 6 previous errors

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

0 comments on commit 8dce3a9

Please sign in to comment.