From b7a41c7a91c3d462766cf29610af937f9276cb59 Mon Sep 17 00:00:00 2001 From: surechen Date: Mon, 8 Apr 2024 16:19:09 +0800 Subject: [PATCH] For OutsideLoop we should not suggest add `'block` label in `if` block, or we wiil get another err: block label not supported here. fixes #123261 --- compiler/rustc_passes/src/errors.rs | 4 +- compiler/rustc_passes/src/loops.rs | 121 +++++++++++++++--- .../loop-if-else-break-issue-123261.fixed | 31 +++++ .../loops/loop-if-else-break-issue-123261.rs | 31 +++++ .../loop-if-else-break-issue-123261.stderr | 59 +++++++++ .../break-in-unlabeled-block-in-macro.stderr | 38 +++--- 6 files changed, 243 insertions(+), 41 deletions(-) create mode 100644 tests/ui/loops/loop-if-else-break-issue-123261.fixed create mode 100644 tests/ui/loops/loop-if-else-break-issue-123261.rs create mode 100644 tests/ui/loops/loop-if-else-break-issue-123261.stderr diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 3f26ea4507d41..bf9199675dc3d 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1091,7 +1091,7 @@ pub struct BreakInsideAsyncBlock<'a> { pub struct OutsideLoop<'a> { #[primary_span] #[label] - pub span: Span, + pub spans: Vec, pub name: &'a str, pub is_break: bool, #[subdiagnostic] @@ -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, } #[derive(Diagnostic)] diff --git a/compiler/rustc_passes/src/loops.rs b/compiler/rustc_passes/src/loops.rs index e10a22cdf31b9..55184d580e616 100644 --- a/compiler/rustc_passes/src/loops.rs +++ b/compiler/rustc_passes/src/loops.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; use Context::*; use rustc_hir as hir; @@ -24,22 +25,38 @@ enum Context { Closure(Span), AsyncClosure(Span), UnlabeledBlock(Span), + IfUnlabeledBlock(Span), LabeledBlock, Constant, } -#[derive(Copy, Clone)] +#[derive(Clone)] +struct BlockInfo { + name: String, + spans: Vec, + suggs: Vec, +} + +#[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, + block_breaks: BTreeMap, } 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], + 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) { @@ -82,6 +99,41 @@ 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)); } @@ -102,11 +154,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)); } @@ -179,7 +234,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"); @@ -201,7 +256,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), } @@ -213,15 +268,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 }); @@ -230,11 +284,24 @@ 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(|| 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, + }); } } } @@ -246,7 +313,7 @@ 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 }); @@ -254,4 +321,18 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> { } false } + + fn report_outside_loop_error(&mut self) { + self.block_breaks.iter().for_each(|(s, block)| { + 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(), + }), + }); + }); + } } diff --git a/tests/ui/loops/loop-if-else-break-issue-123261.fixed b/tests/ui/loops/loop-if-else-break-issue-123261.fixed new file mode 100644 index 0000000000000..f9e88c18ad02c --- /dev/null +++ b/tests/ui/loops/loop-if-else-break-issue-123261.fixed @@ -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; + }; +} diff --git a/tests/ui/loops/loop-if-else-break-issue-123261.rs b/tests/ui/loops/loop-if-else-break-issue-123261.rs new file mode 100644 index 0000000000000..a1f9dabe891fd --- /dev/null +++ b/tests/ui/loops/loop-if-else-break-issue-123261.rs @@ -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; + }; +} diff --git a/tests/ui/loops/loop-if-else-break-issue-123261.stderr b/tests/ui/loops/loop-if-else-break-issue-123261.stderr new file mode 100644 index 0000000000000..7bd192fc00b0d --- /dev/null +++ b/tests/ui/loops/loop-if-else-break-issue-123261.stderr @@ -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`. diff --git a/tests/ui/parser/break-in-unlabeled-block-in-macro.stderr b/tests/ui/parser/break-in-unlabeled-block-in-macro.stderr index 9407e8ac0029d..2f46cb36750be 100644 --- a/tests/ui/parser/break-in-unlabeled-block-in-macro.stderr +++ b/tests/ui/parser/break-in-unlabeled-block-in-macro.stderr @@ -21,16 +21,21 @@ 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:27:19 - | -LL | foo!(stmt break ()); - | ^^^^^^^^ cannot `break` outside of a loop or labeled block + --> $DIR/break-in-unlabeled-block-in-macro.rs:33:17 | -help: consider labeling this block to be able to break within it +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 ~ 'block: { -LL ~ foo!(stmt break 'block ()); +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:12:11 @@ -48,21 +53,16 @@ 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 + --> $DIR/break-in-unlabeled-block-in-macro.rs:27:19 | -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 | foo!(stmt break ()); + | ^^^^^^^^ cannot `break` outside of a loop or labeled block | -LL | break () - | ^^^^^^^^ cannot `break` outside of a loop or labeled block -... -LL | bar!() - | ------ in this macro invocation +help: consider labeling this block to be able to break within it + | +LL ~ 'block: { +LL ~ foo!(stmt break 'block ()); | - = 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