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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions compiler/rustc_ast_lowering/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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> {
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
14 changes: 5 additions & 9 deletions compiler/rustc_passes/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand Down
44 changes: 31 additions & 13 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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, _) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
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
|
Expand All @@ -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
|
Expand Down
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`.
Loading