Skip to content
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

Fix ICE when there is a continue in a labeled block #121682

Closed
wants to merge 4 commits into from
Closed
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
5 changes: 5 additions & 0 deletions compiler/rustc_ast_lowering/messages.ftl
Original file line number Diff line number Diff line change
@@ -54,6 +54,11 @@ ast_lowering_clobber_abi_not_supported =

ast_lowering_closure_cannot_be_static = closures cannot be static

ast_lowering_continue_labeled_block =
`continue` pointing to a labeled block
.label = labeled blocks cannot be `continue`'d
.block_label = labeled block the `continue` points to

ast_lowering_coroutine_too_many_parameters =
too many parameters for a coroutine (expected 0 or 1 parameters)

10 changes: 10 additions & 0 deletions compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
@@ -451,3 +451,13 @@ pub(crate) struct YieldInClosure {
#[suggestion(code = "#[coroutine] ", applicability = "maybe-incorrect", style = "verbose")]
pub suggestion: Option<Span>,
}

#[derive(Diagnostic)]
#[diag(ast_lowering_continue_labeled_block, code = E0696)]
pub struct ContinueLabeledBlock {
#[primary_span]
#[label]
pub span: Span,
#[label(ast_lowering_block_label)]
pub block_span: Span,
}
20 changes: 14 additions & 6 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
@@ -15,14 +15,13 @@ use thin_vec::{thin_vec, ThinVec};

use super::errors::{
AsyncCoroutinesNotSupported, AwaitOnlyInAsyncFnAndBlocks, BaseExpressionDoubleDot,
ClosureCannotBeStatic, CoroutineTooManyParameters,
ClosureCannotBeStatic, ContinueLabeledBlock, CoroutineTooManyParameters,
FunctionalRecordUpdateDestructuringAssignment, InclusiveRangeWithNoEnd, MatchArmWithNoBody,
NeverPatternWithBody, NeverPatternWithGuard, UnderscoreExprLhsAssign,
NeverPatternWithBody, NeverPatternWithGuard, UnderscoreExprLhsAssign, YieldInClosure,
};
use super::{
ImplTraitContext, LoweringContext, ParamMode, ParenthesizedGenericArgs, ResolverAstLoweringExt,
};
use crate::errors::YieldInClosure;
use crate::{FnDeclKind, ImplTraitPosition};

impl<'hir> LoweringContext<'_, 'hir> {
@@ -291,7 +290,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ExprKind::Break(self.lower_jump_destination(e.id, *opt_label), opt_expr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do the same thing with break?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait, maybe we are not doing it. This just assumes that the label is a loop https://github.com/rust-lang/rust/blob/master/compiler/rustc_ast_lowering/src/expr.rs#L1446-L1448

}
ExprKind::Continue(opt_label) => {
hir::ExprKind::Continue(self.lower_jump_destination(e.id, *opt_label))
if opt_label.is_some()
&& let Some((_, is_loop, block_span)) = self.resolver.get_label_res(e.id)
&& !is_loop
{
hir::ExprKind::Err(
self.dcx().emit_err(ContinueLabeledBlock { span: e.span, block_span }),
)
} else {
hir::ExprKind::Continue(self.lower_jump_destination(e.id, *opt_label))
}
}
ExprKind::Ret(e) => {
let e = e.as_ref().map(|x| self.lower_expr(x));
@@ -1470,8 +1478,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn lower_loop_destination(&mut self, destination: Option<(NodeId, Label)>) -> hir::Destination {
let target_id = match destination {
Some((id, _)) => {
if let Some(loop_id) = self.resolver.get_label_res(id) {
Ok(self.lower_node_id(loop_id))
if let Some((id, _is_loop, _)) = self.resolver.get_label_res(id) {
Ok(self.lower_node_id(id))
} else {
Err(hir::LoopIdError::UnresolvedLabel)
}
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
@@ -252,7 +252,7 @@ impl ResolverAstLowering {
}

/// Obtains resolution for a label with the given `NodeId`.
fn get_label_res(&self, id: NodeId) -> Option<NodeId> {
fn get_label_res(&self, id: NodeId) -> Option<(NodeId, bool, Span)> {
self.label_res_map.get(&id).copied()
}

3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -203,7 +203,8 @@ pub struct ResolverAstLowering {
/// Resolutions for import nodes, which have multiple resolutions in different namespaces.
pub import_res_map: NodeMap<hir::def::PerNS<Option<Res<ast::NodeId>>>>,
/// Resolutions for labels (node IDs of their corresponding blocks or loops).
pub label_res_map: NodeMap<ast::NodeId>,
/// The boolean stores if the node is loop. The span is the span of the node.
pub label_res_map: NodeMap<(ast::NodeId, bool, Span)>,
/// Resolutions for lifetimes.
pub lifetimes_res_map: NodeMap<LifetimeRes>,
/// Lifetime parameters that lowering will have to introduce.
5 changes: 0 additions & 5 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
@@ -95,11 +95,6 @@ passes_collapse_debuginfo =
passes_confusables = attribute should be applied to an inherent method
.label = not an inherent method

passes_continue_labeled_block =
`continue` pointing to a labeled block
.label = labeled blocks cannot be `continue`'d
.block_label = labeled block the `continue` points to

passes_coverage_not_fn_or_closure =
attribute should be applied to a function definition or closure
.label = not a function or closure
10 changes: 0 additions & 10 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1044,16 +1044,6 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'_, G> for BreakNonLoop<'a> {
}
}

#[derive(Diagnostic)]
#[diag(passes_continue_labeled_block, code = E0696)]
pub struct ContinueLabeledBlock {
#[primary_span]
#[label]
pub span: Span,
#[label(passes_block_label)]
pub block_span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_break_inside_closure, code = E0267)]
pub struct BreakInsideClosure<'a> {
14 changes: 5 additions & 9 deletions compiler/rustc_passes/src/loops.rs
Original file line number Diff line number Diff line change
@@ -15,8 +15,8 @@ use rustc_span::{BytePos, Span};
use Context::*;

use crate::errors::{
BreakInsideClosure, BreakInsideCoroutine, BreakNonLoop, ContinueLabeledBlock, OutsideLoop,
OutsideLoopSuggestion, UnlabeledCfInWhileCondition, UnlabeledInLabeledBlock,
BreakInsideClosure, BreakInsideCoroutine, BreakNonLoop, OutsideLoop, OutsideLoopSuggestion,
UnlabeledCfInWhileCondition, UnlabeledInLabeledBlock,
};

#[derive(Clone, Copy, Debug, PartialEq)]
@@ -266,13 +266,9 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
self.require_label_in_labeled_block(e.span, &destination, "continue");

match destination.target_id {
Ok(loop_id) => {
if let Node::Block(block) = self.tcx.hir_node(loop_id) {
self.sess.dcx().emit_err(ContinueLabeledBlock {
span: e.span,
block_span: block.span,
});
}
Ok(_loop_id) => {
// We have already insured that the loop exists while lowering the ast.
// See `compiler/rustc_ast_lowering/src/expr.rs::LoweringContext::lower_expr_mut`
}
Err(hir::LoopIdError::UnlabeledCfInWhileCondition) => {
self.sess.dcx().emit_err(UnlabeledCfInWhileCondition {
44 changes: 31 additions & 13 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
@@ -672,7 +672,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
last_block_rib: Option<Rib<'a>>,

/// The current set of local scopes, for labels.
label_ribs: Vec<Rib<'a, NodeId>>,
label_ribs: Vec<Rib<'a, (NodeId, bool, Span)>>,

/// The current set of local scopes for lifetimes.
lifetime_ribs: Vec<LifetimeRib>,
@@ -2316,7 +2316,10 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {

/// Searches the current set of local scopes for labels. Returns the `NodeId` of the resolved
/// label and reports an error if the label is not found or is unreachable.
fn resolve_label(&mut self, mut label: Ident) -> Result<(NodeId, Span), ResolutionError<'a>> {
fn resolve_label(
&mut self,
mut label: Ident,
) -> Result<((NodeId, bool, Span), Span), ResolutionError<'a>> {
let mut suggestion = None;

for i in (0..self.label_ribs.len()).rev() {
@@ -4333,7 +4336,14 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
Ok(Some(result))
}

fn with_resolved_label(&mut self, label: Option<Label>, id: NodeId, f: impl FnOnce(&mut Self)) {
fn with_resolved_label(
&mut self,
label: Option<Label>,
id: NodeId,
is_loop: bool,
span: Span,
f: impl FnOnce(&mut Self),
) {
if let Some(label) = label {
if label.ident.as_str().as_bytes()[1] != b'_' {
self.diag_metadata.unused_labels.insert(id, label.ident.span);
@@ -4345,16 +4355,22 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {

self.with_label_rib(RibKind::Normal, |this| {
let ident = label.ident.normalize_to_macro_rules();
this.label_ribs.last_mut().unwrap().bindings.insert(ident, id);
this.label_ribs.last_mut().unwrap().bindings.insert(ident, (id, is_loop, span));
f(this);
});
} else {
f(self);
}
}

fn resolve_labeled_block(&mut self, label: Option<Label>, id: NodeId, block: &'ast Block) {
self.with_resolved_label(label, id, |this| this.visit_block(block));
fn resolve_labeled_block(
&mut self,
label: Option<Label>,
id: NodeId,
block: &'ast Block,
is_loop: bool,
) {
self.with_resolved_label(label, id, is_loop, block.span, |this| this.visit_block(block));
}

fn resolve_block(&mut self, block: &'ast Block) {
@@ -4497,10 +4513,10 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {

ExprKind::Break(Some(label), _) | ExprKind::Continue(Some(label)) => {
match self.resolve_label(label.ident) {
Ok((node_id, _)) => {
Ok((node, _)) => {
// Since this res is a label, it is never read.
self.r.label_res_map.insert(expr.id, node_id);
self.diag_metadata.unused_labels.remove(&node_id);
self.r.label_res_map.insert(expr.id, node);
self.diag_metadata.unused_labels.remove(&node.0);
}
Err(error) => {
self.report_error(label.ident.span, error);
@@ -4535,11 +4551,11 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
}

ExprKind::Loop(ref block, label, _) => {
self.resolve_labeled_block(label, expr.id, block)
self.resolve_labeled_block(label, expr.id, block, true)
}

ExprKind::While(ref cond, ref block, label) => {
self.with_resolved_label(label, expr.id, |this| {
self.with_resolved_label(label, expr.id, true, block.span, |this| {
this.with_rib(ValueNS, RibKind::Normal, |this| {
let old = this.diag_metadata.in_if_condition.replace(cond);
this.visit_expr(cond);
@@ -4553,11 +4569,13 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
self.visit_expr(iter);
self.with_rib(ValueNS, RibKind::Normal, |this| {
this.resolve_pattern_top(pat, PatternSource::For);
this.resolve_labeled_block(label, expr.id, body);
this.resolve_labeled_block(label, expr.id, body, true);
});
}

ExprKind::Block(ref block, label) => self.resolve_labeled_block(label, block.id, block),
ExprKind::Block(ref block, label) => {
self.resolve_labeled_block(label, block.id, block, false)
}

// Equivalent to `visit::walk_expr` + passing some context to children.
ExprKind::Field(ref subexpression, _) => {
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -935,7 +935,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
if let Some(err_code) = err.code {
if err_code == E0425 {
for label_rib in &self.label_ribs {
for (label_ident, node_id) in &label_rib.bindings {
for (label_ident, (node_id, _, _)) in &label_rib.bindings {
let ident = path.last().unwrap().ident;
if format!("'{ident}") == label_ident.to_string() {
err.span_label(label_ident.span, "a label with a similar name exists");
3 changes: 2 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1017,7 +1017,8 @@ pub struct Resolver<'a, 'tcx> {
/// Resolutions for import nodes, which have multiple resolutions in different namespaces.
import_res_map: NodeMap<PerNS<Option<Res>>>,
/// Resolutions for labels (node IDs of their corresponding blocks or loops).
label_res_map: NodeMap<NodeId>,
/// The boolean stores if the node is loop. The span is the span of the node.
label_res_map: NodeMap<(NodeId, bool, Span)>,
/// Resolutions for lifetimes.
lifetimes_res_map: NodeMap<LifetimeRes>,
/// Lifetime parameters that lowering will have to introduce.
7 changes: 0 additions & 7 deletions tests/crashes/113379.rs

This file was deleted.

12 changes: 6 additions & 6 deletions tests/ui/label/label_break_value_continue.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
error[E0695]: unlabeled `continue` inside of a labeled block
--> $DIR/label_break_value_continue.rs:6:9
|
LL | continue;
| ^^^^^^^^ `continue` statements that would diverge to or through a labeled block need to bear a label

error[E0696]: `continue` pointing to a labeled block
--> $DIR/label_break_value_continue.rs:13:9
|
@@ -13,6 +7,12 @@ LL | | continue 'b;
LL | | }
| |_____- labeled block the `continue` points to

error[E0695]: unlabeled `continue` inside of a labeled block
--> $DIR/label_break_value_continue.rs:6:9
|
LL | continue;
| ^^^^^^^^ `continue` statements that would diverge to or through a labeled block need to bear a label

error[E0695]: unlabeled `continue` inside of a labeled block
--> $DIR/label_break_value_continue.rs:21:13
|
28 changes: 28 additions & 0 deletions tests/ui/lowering/cont-in-block/cont-in-fn-issue-113379.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Fixes: #113379

trait Trait<const S: usize> {}

struct Bug<T>
where
T: Trait<
{
'b: {
//~^ ERROR [E0308]
continue 'b; //~ ERROR [E0696]
}
},
>,
{
t: T,
}

fn f() -> impl Sized {
'b: {
continue 'b;
//~^ ERROR [E0696]
}
}

fn main() {
f();
}
33 changes: 33 additions & 0 deletions tests/ui/lowering/cont-in-block/cont-in-fn-issue-113379.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error[E0696]: `continue` pointing to a labeled block
--> $DIR/cont-in-fn-issue-113379.rs:11:17
|
LL | / 'b: {
LL | |
LL | | continue 'b;
| | ^^^^^^^^^^^ labeled blocks cannot be `continue`'d
LL | | }
| |_____________- labeled block the `continue` points to

error[E0696]: `continue` pointing to a labeled block
--> $DIR/cont-in-fn-issue-113379.rs:21:9
|
LL | / 'b: {
LL | | continue 'b;
| | ^^^^^^^^^^^ labeled blocks cannot be `continue`'d
LL | |
LL | | }
| |_____- labeled block the `continue` points to

error[E0308]: mismatched types
--> $DIR/cont-in-fn-issue-113379.rs:9:13
|
LL | / 'b: {
LL | |
LL | | continue 'b;
LL | | }
| |_____________^ expected `usize`, found `()`

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0308, E0696.
For more information about an error, try `rustc --explain E0308`.
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
//@ known-bug: #121623
// Fixes: #121623

fn main() {
match () {
_ => 'b: {
continue 'b;
//~^ ERROR [E0696]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0696]: `continue` pointing to a labeled block
--> $DIR/cont-in-match-arm-issue-121623.rs:6:13
|
LL | _ => 'b: {
| ______________-
LL | | continue 'b;
| | ^^^^^^^^^^^ labeled blocks cannot be `continue`'d
LL | |
LL | | }
| |_________- labeled block the `continue` points to

error: aborting due to 1 previous error

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