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

Implement label break value (RFC 2046) #50045

Merged
merged 9 commits into from
May 16, 2018
2 changes: 1 addition & 1 deletion src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {

fn expr(&mut self, expr: &hir::Expr, pred: CFGIndex) -> CFGIndex {
match expr.node {
hir::ExprBlock(ref blk) => {
hir::ExprBlock(ref blk, _) => {
let blk_exit = self.block(&blk, pred);
self.add_ast_node(expr.hir_id.local_id, &[blk_exit])
}
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,10 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
expression.span,
expression.id)
}
ExprBlock(ref block) => visitor.visit_block(block),
ExprBlock(ref block, ref opt_label) => {
walk_list!(visitor, visit_label, opt_label);
visitor.visit_block(block);
}
ExprAssign(ref left_hand_expression, ref right_hand_expression) => {
visitor.visit_expr(right_hand_expression);
visitor.visit_expr(left_hand_expression)
Expand Down
10 changes: 7 additions & 3 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3048,7 +3048,7 @@ impl<'a> LoweringContext<'a> {
);
block.expr = Some(this.wrap_in_try_constructor(
"from_ok", tail, unstable_span));
hir::ExprBlock(P(block))
hir::ExprBlock(P(block), None)
})
}
ExprKind::Match(ref expr, ref arms) => hir::ExprMatch(
Expand Down Expand Up @@ -3100,7 +3100,11 @@ impl<'a> LoweringContext<'a> {
})
})
}
ExprKind::Block(ref blk) => hir::ExprBlock(self.lower_block(blk, false)),
ExprKind::Block(ref blk, opt_label) => {
hir::ExprBlock(self.lower_block(blk,
opt_label.is_some()),
self.lower_label(opt_label))
}
ExprKind::Assign(ref el, ref er) => {
hir::ExprAssign(P(self.lower_expr(el)), P(self.lower_expr(er)))
}
Expand Down Expand Up @@ -3843,7 +3847,7 @@ impl<'a> LoweringContext<'a> {
}

fn expr_block(&mut self, b: P<hir::Block>, attrs: ThinVec<Attribute>) -> hir::Expr {
self.expr(b.span, hir::ExprBlock(b), attrs)
self.expr(b.span, hir::ExprBlock(b, None), attrs)
}

fn expr_tuple(&mut self, sp: Span, exprs: hir::HirVec<hir::Expr>) -> P<hir::Expr> {
Expand Down
9 changes: 4 additions & 5 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,9 +778,8 @@ pub struct Block {
pub rules: BlockCheckMode,
pub span: Span,
/// If true, then there may exist `break 'a` values that aim to
/// break out of this block early. As of this writing, this is not
/// currently permitted in Rust itself, but it is generated as
/// part of `catch` statements.
/// break out of this block early.
/// Used by `'label: {}` blocks and by `catch` statements.
pub targeted_by_break: bool,
/// If true, don't emit return value type errors as the parser had
/// to recover from a parse error so this block will not have an
Expand Down Expand Up @@ -1381,8 +1380,8 @@ pub enum Expr_ {
/// This may also be a generator literal, indicated by the final boolean,
/// in that case there is an GeneratorClause.
ExprClosure(CaptureClause, P<FnDecl>, BodyId, Span, Option<GeneratorMovability>),
/// A block (`{ ... }`)
ExprBlock(P<Block>),
/// A block (`'label: { ... }`)
ExprBlock(P<Block>, Option<Label>),

/// An assignment (`a = foo()`)
ExprAssign(P<Expr>, P<Expr>),
Expand Down
16 changes: 12 additions & 4 deletions src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ impl<'a> State<'a> {
self.print_else(e.as_ref().map(|e| &**e))
}
// "final else"
hir::ExprBlock(ref b) => {
hir::ExprBlock(ref b, _) => {
self.cbox(indent_unit - 1)?;
self.ibox(0)?;
self.s.word(" else ")?;
Expand Down Expand Up @@ -1377,7 +1377,11 @@ impl<'a> State<'a> {
// empty box to satisfy the close.
self.ibox(0)?;
}
hir::ExprBlock(ref blk) => {
hir::ExprBlock(ref blk, opt_label) => {
if let Some(label) = opt_label {
self.print_name(label.name)?;
self.word_space(":")?;
}
// containing cbox, will be closed by print-block at }
self.cbox(indent_unit)?;
// head-box, will be closed by print-block after {
Expand Down Expand Up @@ -1893,7 +1897,11 @@ impl<'a> State<'a> {
self.word_space("=>")?;

match arm.body.node {
hir::ExprBlock(ref blk) => {
hir::ExprBlock(ref blk, opt_label) => {
if let Some(label) = opt_label {
self.print_name(label.name)?;
self.word_space(":")?;
}
// the block will close the pattern's ibox
self.print_block_unclosed_indent(&blk, indent_unit)?;

Expand Down Expand Up @@ -2299,7 +2307,7 @@ fn expr_requires_semi_to_be_stmt(e: &hir::Expr) -> bool {
match e.node {
hir::ExprIf(..) |
hir::ExprMatch(..) |
hir::ExprBlock(_) |
hir::ExprBlock(..) |
hir::ExprWhile(..) |
hir::ExprLoop(..) => false,
_ => true,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ impl_stable_hash_for!(enum hir::Expr_ {
ExprLoop(body, label, loop_src),
ExprMatch(matchee, arms, match_src),
ExprClosure(capture_clause, decl, body_id, span, gen),
ExprBlock(blk),
ExprBlock(blk, label),
ExprAssign(lhs, rhs),
ExprAssignOp(op, lhs, rhs),
ExprField(owner, field_name),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
self.consume_expr(&rhs);
}

hir::ExprBlock(ref blk) => {
hir::ExprBlock(ref blk, _) => {
self.walk_block(&blk);
}

Expand Down
4 changes: 3 additions & 1 deletion src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
succ
}

hir::ExprBlock(ref blk) => {
// Note that labels have been resolved, so we don't need to look
// at the label ident
hir::ExprBlock(ref blk, _) => {
self.propagate_through_block(&blk, succ)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1247,7 +1247,7 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
hir::ExprCast(ref subexpr, _) => {
record_rvalue_scope_if_borrow_expr(visitor, &subexpr, blk_id)
}
hir::ExprBlock(ref block) => {
hir::ExprBlock(ref block, _) => {
if let Some(ref subexpr) = block.expr {
record_rvalue_scope_if_borrow_expr(
visitor, &subexpr, blk_id);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl UnsafeCode {

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnsafeCode {
fn check_expr(&mut self, cx: &LateContext, e: &hir::Expr) {
if let hir::ExprBlock(ref blk) = e.node {
if let hir::ExprBlock(ref blk, _) = e.node {
// Don't warn about generated blocks, that'll just pollute the output.
if blk.rules == hir::UnsafeBlock(hir::UserProvided) {
self.report_unsafe(cx, blk.span, "usage of an `unsafe` block");
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), block, move |this| {
this.in_scope((region_scope, source_info), LintLevel::Inherited, block, move |this| {
if targeted_by_break {
// This is a `break`-able block (currently only `catch { ... }`)
// This is a `break`-able block
let exit_block = this.cfg.start_new_block();
let block_exit = this.in_breakable_scope(
None, exit_block, destination.clone(), |this| {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/hair/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
}
}

hir::ExprBlock(ref blk) => ExprKind::Block { body: &blk },
hir::ExprBlock(ref blk, _) => ExprKind::Block { body: &blk },

hir::ExprAssign(ref lhs, ref rhs) => {
ExprKind::Assign {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/add_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
}
// Check if this is an unsafe block, or an item
match node {
Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block), ..}) => {
Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block, _), ..}) => {
if block_is_unsafe(&*block) {
// Found an unsafe block, we can bail out here.
return true;
Expand Down
39 changes: 39 additions & 0 deletions src/librustc_passes/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,44 @@ let result = loop { // ok!
i += 1;
};
```
"##,

E0695: r##"
A `break` statement without a label appeared inside a labeled block.

Example of erroneous code:

```compile_fail,E0695
# #![feature(label_break_value)]
loop {
'a: {
break;
}
}
```

Make sure to always label the `break`:

```
# #![feature(label_break_value)]
'l: loop {
'a: {
break 'l;
}
}
```

Or if you want to `break` the labeled block:

```
# #![feature(label_break_value)]
loop {
'a: {
break 'a;
}
break;
}
```
"##
}

Expand All @@ -271,4 +309,5 @@ register_diagnostics! {
E0642, // patterns aren't allowed in methods without bodies
E0666, // nested `impl Trait` is illegal
E0667, // `impl Trait` in projections
E0696, // `continue` pointing to a labeled block
}
62 changes: 54 additions & 8 deletions src/librustc_passes/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc::session::Session;

use rustc::hir::map::Map;
use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap};
use rustc::hir;
use rustc::hir::{self, Destination};
use syntax::ast;
use syntax_pos::Span;

Expand All @@ -39,6 +39,7 @@ enum Context {
Normal,
Loop(LoopKind),
Closure,
LabeledBlock,
}

#[derive(Copy, Clone)]
Expand Down Expand Up @@ -84,7 +85,16 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
hir::ExprClosure(.., b, _, _) => {
self.with_context(Closure, |v| v.visit_nested_body(b));
}
hir::ExprBlock(ref b, Some(_label)) => {
self.with_context(LabeledBlock, |v| v.visit_block(&b));
}
hir::ExprBreak(label, ref opt_expr) => {
if self.require_label_in_labeled_block(e.span, &label, "break") {
// If we emitted an error about an unlabeled break in a labeled
// block, we don't need any further checking for this break any more
return;
}

let loop_id = match label.target_id.into() {
Ok(loop_id) => loop_id,
Err(hir::LoopIdError::OutsideLoopScope) => ast::DUMMY_NODE_ID,
Expand All @@ -94,6 +104,7 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
},
Err(hir::LoopIdError::UnresolvedLabel) => ast::DUMMY_NODE_ID,
};

if loop_id != ast::DUMMY_NODE_ID {
match self.hir_map.find(loop_id).unwrap() {
hir::map::NodeBlock(_) => return,
Expand All @@ -113,13 +124,15 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
})
};
match loop_kind {
None | Some(LoopKind::Loop(hir::LoopSource::Loop)) => (),
None |
Some(LoopKind::Loop(hir::LoopSource::Loop)) => (),
Some(kind) => {
struct_span_err!(self.sess, e.span, E0571,
"`break` with value from a `{}` loop",
kind.name())
.span_label(e.span,
"can only break with a value inside `loop`")
"can only break with a value inside \
`loop` or breakable block")
.span_suggestion(e.span,
&format!("instead, use `break` on its own \
without a value inside this `{}` loop",
Expand All @@ -130,13 +143,29 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
}
}

self.require_loop("break", e.span);
self.require_break_cx("break", e.span);
}
hir::ExprAgain(label) => {
if let Err(hir::LoopIdError::UnlabeledCfInWhileCondition) = label.target_id {
self.emit_unlabled_cf_in_while_condition(e.span, "continue");
self.require_label_in_labeled_block(e.span, &label, "continue");

match label.target_id {
Ok(loop_id) => {
if let hir::map::NodeBlock(block) = self.hir_map.find(loop_id).unwrap() {
struct_span_err!(self.sess, e.span, E0696,
"`continue` pointing to a labeled block")
.span_label(e.span,
"labeled blocks cannot be `continue`'d")
.span_note(block.span,
"labeled block the continue points to")
.emit();
}
}
Err(hir::LoopIdError::UnlabeledCfInWhileCondition) => {
self.emit_unlabled_cf_in_while_condition(e.span, "continue");
}
_ => {}
}
self.require_loop("continue", e.span)
self.require_break_cx("continue", e.span)
},
_ => intravisit::walk_expr(self, e),
}
Expand All @@ -153,8 +182,9 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> {
self.cx = old_cx;
}

fn require_loop(&self, name: &str, span: Span) {
fn require_break_cx(&self, name: &str, span: Span) {
match self.cx {
LabeledBlock |
Loop(_) => {}
Closure => {
struct_span_err!(self.sess, span, E0267, "`{}` inside of a closure", name)
Expand All @@ -169,6 +199,22 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> {
}
}

fn require_label_in_labeled_block(&mut self, span: Span, label: &Destination, cf_type: &str)
-> bool
{
if self.cx == LabeledBlock {
if label.label.is_none() {
struct_span_err!(self.sess, span, E0695,
"unlabeled `{}` inside of a labeled block", cf_type)
.span_label(span,
format!("`{}` statements that would diverge to or through \
a labeled block need to bear a label", cf_type))
.emit();
return true;
}
}
return false;
}
fn emit_unlabled_cf_in_while_condition(&mut self, span: Span, cf_type: &str) {
struct_span_err!(self.sess, span, E0590,
"`break` or `continue` with no label in the condition of a `while` loop")
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_passes/rvalue_promotion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node
let mut callee = &**callee;
loop {
callee = match callee.node {
hir::ExprBlock(ref block) => match block.expr {
hir::ExprBlock(ref block, _) => match block.expr {
Some(ref tail) => &tail,
None => break
},
Expand Down Expand Up @@ -404,7 +404,7 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node
}
}

hir::ExprBlock(_) |
hir::ExprBlock(..) |
hir::ExprIndex(..) |
hir::ExprField(..) |
hir::ExprArray(_) |
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3753,6 +3753,8 @@ impl<'a> Resolver<'a> {
self.ribs[ValueNS].pop();
}

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

// Equivalent to `visit::walk_expr` + passing some context to children.
ExprKind::Field(ref subexpression, _) => {
self.resolve_expr(subexpression, Some(expr));
Expand Down
Loading