From 9d42549df40464899dda11fc9509f511046fb4c6 Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Sat, 29 Oct 2016 15:15:06 -0700 Subject: [PATCH] Implement the `loop_break_value` feature. This implements RFC 1624, tracking issue #37339. - `FnCtxt` (in typeck) gets a stack of `LoopCtxt`s, which store the currently deduced type of that loop, the desired type, and a list of break expressions currently seen. `loop` loops get a fresh type variable as their initial type (this logic is stolen from that for arrays). `while` loops get `()`. - `break {expr}` looks up the broken loop, and unifies the type of `expr` with the type of the loop. - `break` with no expr unifies the loop's type with `()`. - When building MIR, `loop` loops no longer construct a `()` value at termination of the loop; rather, the `break` expression assigns the result of the loop. `while` loops are unchanged. - `break` respects contexts in which expressions may not end with braced blocks. That is, `while break { break-value } { while-body }` is illegal; this preserves backwards compatibility. - The RFC did not make it clear, but I chose to make `break ()` inside of a `while` loop illegal, just in case we wanted to do anything with that design space in the future. This is my first time dealing with this part of rustc so I'm sure there's plenty of problems to pick on here ^_^ --- src/librustc/cfg/construct.rs | 7 +- src/librustc/hir/intravisit.rs | 8 +- src/librustc/hir/lowering.rs | 17 +- src/librustc/hir/mod.rs | 16 +- src/librustc/hir/print.rs | 8 +- src/librustc/middle/expr_use_visitor.rs | 5 +- src/librustc/middle/liveness.rs | 10 +- src/librustc/middle/region.rs | 2 +- src/librustc/middle/resolve_lifetime.rs | 2 +- src/librustc/util/common.rs | 55 ------ src/librustc_driver/driver.rs | 2 +- .../calculate_svh/svh_visitor.rs | 4 +- src/librustc_mir/build/expr/into.rs | 64 +++---- src/librustc_mir/build/expr/stmt.rs | 46 +++-- src/librustc_mir/build/mod.rs | 2 +- src/librustc_mir/build/scope.rs | 20 +- src/librustc_mir/hair/cx/expr.rs | 7 +- src/librustc_mir/hair/mod.rs | 1 + src/librustc_passes/ast_validation.rs | 2 +- src/librustc_passes/consts.rs | 2 +- src/librustc_passes/diagnostics.rs | 1 + src/librustc_passes/loops.rs | 83 ++++++-- src/librustc_resolve/lib.rs | 11 +- src/librustc_typeck/check/mod.rs | 177 +++++++++++++++--- src/librustc_typeck/check/regionck.rs | 2 +- src/libsyntax/ast.rs | 4 +- src/libsyntax/ext/build.rs | 2 +- src/libsyntax/feature_gate.rs | 7 + src/libsyntax/fold.rs | 9 +- src/libsyntax/parse/parser.rs | 20 +- src/libsyntax/print/pprust.rs | 6 +- src/libsyntax/visit.rs | 6 +- .../feature-gate-loop-break-value.rs | 15 ++ src/test/compile-fail/loop-break-value.rs | 101 ++++++++++ src/test/run-pass/loop-break-value.rs | 133 +++++++++++++ 35 files changed, 641 insertions(+), 216 deletions(-) create mode 100644 src/test/compile-fail/feature-gate-loop-break-value.rs create mode 100644 src/test/compile-fail/loop-break-value.rs create mode 100644 src/test/run-pass/loop-break-value.rs diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index be996ea1957..c399623462b 100644 --- a/src/librustc/cfg/construct.rs +++ b/src/librustc/cfg/construct.rs @@ -223,7 +223,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { expr_exit } - hir::ExprLoop(ref body, _) => { + hir::ExprLoop(ref body, _, _) => { // // [pred] // | @@ -282,9 +282,10 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { self.add_unreachable_node() } - hir::ExprBreak(label) => { + hir::ExprBreak(label, ref opt_expr) => { + let v = self.opt_expr(opt_expr, pred); let loop_scope = self.find_scope(expr, label.map(|l| l.node)); - let b = self.add_ast_node(expr.id, &[pred]); + let b = self.add_ast_node(expr.id, &[v]); self.add_exiting_edge(expr, b, loop_scope, loop_scope.break_index); self.add_unreachable_node() diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 4cfa889ec56..7dd88e36dd1 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -882,7 +882,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) { visitor.visit_block(block); walk_opt_sp_name(visitor, opt_sp_name); } - ExprLoop(ref block, ref opt_sp_name) => { + ExprLoop(ref block, ref opt_sp_name, _) => { visitor.visit_block(block); walk_opt_sp_name(visitor, opt_sp_name); } @@ -923,7 +923,11 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) { } visitor.visit_path(path, expression.id) } - ExprBreak(ref opt_sp_name) | ExprAgain(ref opt_sp_name) => { + ExprBreak(ref opt_sp_name, ref opt_expr) => { + walk_opt_sp_name(visitor, opt_sp_name); + walk_list!(visitor, visit_expr, opt_expr); + } + ExprAgain(ref opt_sp_name) => { walk_opt_sp_name(visitor, opt_sp_name); } ExprRet(ref optional_expression) => { diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 7b04259d1c3..5af7c18e1a1 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1136,7 +1136,9 @@ impl<'a> LoweringContext<'a> { self.lower_opt_sp_ident(opt_ident)) } ExprKind::Loop(ref body, opt_ident) => { - hir::ExprLoop(self.lower_block(body), self.lower_opt_sp_ident(opt_ident)) + hir::ExprLoop(self.lower_block(body), + self.lower_opt_sp_ident(opt_ident), + hir::LoopSource::Loop) } ExprKind::Match(ref expr, ref arms) => { hir::ExprMatch(P(self.lower_expr(expr)), @@ -1242,7 +1244,10 @@ impl<'a> LoweringContext<'a> { }); hir::ExprPath(hir_qself, self.lower_path(path)) } - ExprKind::Break(opt_ident) => hir::ExprBreak(self.lower_opt_sp_ident(opt_ident)), + ExprKind::Break(opt_ident, ref opt_expr) => { + hir::ExprBreak(self.lower_opt_sp_ident(opt_ident), + opt_expr.as_ref().map(|x| P(self.lower_expr(x)))) + } ExprKind::Continue(opt_ident) => hir::ExprAgain(self.lower_opt_sp_ident(opt_ident)), ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| P(self.lower_expr(x)))), ExprKind::InlineAsm(ref asm) => { @@ -1410,7 +1415,8 @@ impl<'a> LoweringContext<'a> { // `[opt_ident]: loop { ... }` let loop_block = P(self.block_expr(P(match_expr))); - let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident)); + let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident), + hir::LoopSource::WhileLet); // add attributes to the outer returned expr node let attrs = e.attrs.clone(); return hir::Expr { id: e.id, node: loop_expr, span: e.span, attrs: attrs }; @@ -1485,7 +1491,8 @@ impl<'a> LoweringContext<'a> { // `[opt_ident]: loop { ... }` let loop_block = P(self.block_expr(match_expr)); - let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident)); + let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident), + hir::LoopSource::ForLoop); let loop_expr = P(hir::Expr { id: e.id, node: loop_expr, @@ -1723,7 +1730,7 @@ impl<'a> LoweringContext<'a> { } fn expr_break(&mut self, span: Span, attrs: ThinVec) -> P { - P(self.expr(span, hir::ExprBreak(None), attrs)) + P(self.expr(span, hir::ExprBreak(None, None), attrs)) } fn expr_call(&mut self, span: Span, e: P, args: hir::HirVec) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 28ea1443724..31648765224 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -908,7 +908,7 @@ pub enum Expr_ { /// Conditionless loop (can be exited with break, continue, or return) /// /// `'label: loop { block }` - ExprLoop(P, Option>), + ExprLoop(P, Option>, LoopSource), /// A `match` block, with a source that indicates whether or not it is /// the result of a desugaring, and if so, which kind. ExprMatch(P, HirVec, MatchSource), @@ -944,7 +944,7 @@ pub enum Expr_ { /// A referencing operation (`&a` or `&mut a`) ExprAddrOf(Mutability, P), /// A `break`, with an optional label to break - ExprBreak(Option>), + ExprBreak(Option>, Option>), /// A `continue`, with an optional label ExprAgain(Option>), /// A `return`, with an optional value to be returned @@ -1002,6 +1002,18 @@ pub enum MatchSource { TryDesugar, } +/// The loop type that yielded an ExprLoop +#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] +pub enum LoopSource { + /// A `loop { .. }` loop + Loop, + /// A `while let _ = _ { .. }` loop + WhileLet, + /// A `for _ in _ { .. }` loop + ForLoop, +} + + #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] pub enum CaptureClause { CaptureByValue, diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index 9eb79e38d6f..c109e84bf61 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -1393,7 +1393,7 @@ impl<'a> State<'a> { space(&mut self.s)?; self.print_block(&blk)?; } - hir::ExprLoop(ref blk, opt_sp_name) => { + hir::ExprLoop(ref blk, opt_sp_name, _) => { if let Some(sp_name) = opt_sp_name { self.print_name(sp_name.node)?; self.word_space(":")?; @@ -1471,13 +1471,17 @@ impl<'a> State<'a> { hir::ExprPath(Some(ref qself), ref path) => { self.print_qpath(path, qself, true)? } - hir::ExprBreak(opt_name) => { + hir::ExprBreak(opt_name, ref opt_expr) => { word(&mut self.s, "break")?; space(&mut self.s)?; if let Some(name) = opt_name { self.print_name(name.node)?; space(&mut self.s)?; } + if let Some(ref expr) = *opt_expr { + self.print_expr(expr)?; + space(&mut self.s)?; + } } hir::ExprAgain(opt_name) => { word(&mut self.s, "continue")?; diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index aafb38853b3..594ed408d8c 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -472,11 +472,10 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { self.consume_exprs(inputs); } - hir::ExprBreak(..) | hir::ExprAgain(..) | hir::ExprLit(..) => {} - hir::ExprLoop(ref blk, _) => { + hir::ExprLoop(ref blk, _, _) => { self.walk_block(&blk); } @@ -514,7 +513,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { self.walk_block(&blk); } - hir::ExprRet(ref opt_expr) => { + hir::ExprBreak(_, ref opt_expr) | hir::ExprRet(ref opt_expr) => { if let Some(ref expr) = *opt_expr { self.consume_expr(&expr); } diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 582c64b6e68..4b1787ba593 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -490,7 +490,7 @@ fn visit_expr(ir: &mut IrMaps, expr: &Expr) { hir::ExprIndex(..) | hir::ExprField(..) | hir::ExprTupField(..) | hir::ExprArray(..) | hir::ExprCall(..) | hir::ExprMethodCall(..) | hir::ExprTup(..) | hir::ExprBinary(..) | hir::ExprAddrOf(..) | - hir::ExprCast(..) | hir::ExprUnary(..) | hir::ExprBreak(_) | + hir::ExprCast(..) | hir::ExprUnary(..) | hir::ExprBreak(..) | hir::ExprAgain(_) | hir::ExprLit(_) | hir::ExprRet(..) | hir::ExprBlock(..) | hir::ExprAssign(..) | hir::ExprAssignOp(..) | hir::ExprStruct(..) | hir::ExprRepeat(..) | @@ -990,7 +990,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { // Note that labels have been resolved, so we don't need to look // at the label ident - hir::ExprLoop(ref blk, _) => { + hir::ExprLoop(ref blk, _, _) => { self.propagate_through_loop(expr, LoopLoop, &blk, succ) } @@ -1035,7 +1035,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { self.propagate_through_opt_expr(o_e.as_ref().map(|e| &**e), exit_ln) } - hir::ExprBreak(opt_label) => { + hir::ExprBreak(opt_label, ref opt_expr) => { // Find which label this break jumps to let sc = self.find_loop_scope(opt_label.map(|l| l.node), expr.id, expr.span); @@ -1043,7 +1043,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { // look it up in the break loop nodes table match self.break_ln.get(&sc) { - Some(&b) => b, + Some(&b) => self.propagate_through_opt_expr(opt_expr.as_ref().map(|e| &**e), b), None => span_bug!(expr.span, "break to unknown label") } } @@ -1057,7 +1057,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { match self.cont_ln.get(&sc) { Some(&b) => b, - None => span_bug!(expr.span, "loop to unknown label") + None => span_bug!(expr.span, "continue to unknown label") } } diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 5f9a6b283c6..0dbde2d21ca 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -805,7 +805,7 @@ fn resolve_expr(visitor: &mut RegionResolutionVisitor, expr: &hir::Expr) { terminating(then.id); } - hir::ExprLoop(ref body, _) => { + hir::ExprLoop(ref body, _, _) => { terminating(body.id); } diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index a0043d0a886..41da5562e23 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -462,7 +462,7 @@ fn extract_labels(ctxt: &mut LifetimeContext, b: &hir::Expr) { fn expression_label(ex: &hir::Expr) -> Option<(ast::Name, Span)> { match ex.node { hir::ExprWhile(.., Some(label)) | - hir::ExprLoop(_, Some(label)) => Some((label.node, label.span)), + hir::ExprLoop(_, Some(label), _) => Some((label.node, label.span)), _ => None, } } diff --git a/src/librustc/util/common.rs b/src/librustc/util/common.rs index 7cd5fd78df5..e01856b2a47 100644 --- a/src/librustc/util/common.rs +++ b/src/librustc/util/common.rs @@ -19,10 +19,6 @@ use std::iter::repeat; use std::path::Path; use std::time::{Duration, Instant}; -use hir; -use hir::intravisit; -use hir::intravisit::Visitor; - // The name of the associated type for `Fn` return types pub const FN_OUTPUT_NAME: &'static str = "Output"; @@ -186,57 +182,6 @@ pub fn indenter() -> Indenter { Indenter { _cannot_construct_outside_of_this_module: () } } -struct LoopQueryVisitor

where P: FnMut(&hir::Expr_) -> bool { - p: P, - flag: bool, -} - -impl<'v, P> Visitor<'v> for LoopQueryVisitor

where P: FnMut(&hir::Expr_) -> bool { - fn visit_expr(&mut self, e: &hir::Expr) { - self.flag |= (self.p)(&e.node); - match e.node { - // Skip inner loops, since a break in the inner loop isn't a - // break inside the outer loop - hir::ExprLoop(..) | hir::ExprWhile(..) => {} - _ => intravisit::walk_expr(self, e) - } - } -} - -// Takes a predicate p, returns true iff p is true for any subexpressions -// of b -- skipping any inner loops (loop, while, loop_body) -pub fn loop_query

(b: &hir::Block, p: P) -> bool where P: FnMut(&hir::Expr_) -> bool { - let mut v = LoopQueryVisitor { - p: p, - flag: false, - }; - intravisit::walk_block(&mut v, b); - return v.flag; -} - -struct BlockQueryVisitor

where P: FnMut(&hir::Expr) -> bool { - p: P, - flag: bool, -} - -impl<'v, P> Visitor<'v> for BlockQueryVisitor

where P: FnMut(&hir::Expr) -> bool { - fn visit_expr(&mut self, e: &hir::Expr) { - self.flag |= (self.p)(e); - intravisit::walk_expr(self, e) - } -} - -// Takes a predicate p, returns true iff p is true for any subexpressions -// of b -- skipping any inner loops (loop, while, loop_body) -pub fn block_query

(b: &hir::Block, p: P) -> bool where P: FnMut(&hir::Expr) -> bool { - let mut v = BlockQueryVisitor { - p: p, - flag: false, - }; - intravisit::walk_block(&mut v, &b); - return v.flag; -} - pub trait MemoizationMap { type Key: Clone; type Value: Clone; diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 228119e6cc7..756b0457b78 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -855,7 +855,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, time(time_passes, "loop checking", - || loops::check_crate(sess, &hir_map)); + || loops::check_crate(sess, &resolutions.def_map, &hir_map)); time(time_passes, "static item recursion checking", diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 4bad264ac87..fd0856393fc 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -322,7 +322,7 @@ fn saw_expr<'a>(node: &'a Expr_, ExprType(..) => (SawExprType, false), ExprIf(..) => (SawExprIf, false), ExprWhile(..) => (SawExprWhile, false), - ExprLoop(_, id) => (SawExprLoop(id.map(|id| id.node.as_str())), false), + ExprLoop(_, id, _) => (SawExprLoop(id.map(|id| id.node.as_str())), false), ExprMatch(..) => (SawExprMatch, false), ExprClosure(cc, _, _, _) => (SawExprClosure(cc), false), ExprBlock(..) => (SawExprBlock, false), @@ -335,7 +335,7 @@ fn saw_expr<'a>(node: &'a Expr_, ExprIndex(..) => (SawExprIndex, true), ExprPath(ref qself, _) => (SawExprPath(qself.as_ref().map(|q| q.position)), false), ExprAddrOf(m, _) => (SawExprAddrOf(m), false), - ExprBreak(id) => (SawExprBreak(id.map(|id| id.node.as_str())), false), + ExprBreak(id, _) => (SawExprBreak(id.map(|id| id.node.as_str())), false), ExprAgain(id) => (SawExprAgain(id.map(|id| id.node.as_str())), false), ExprRet(..) => (SawExprRet, false), ExprInlineAsm(ref a,..) => (SawExprInlineAsm(a), false), diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 5fa08442221..5a77de08070 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -169,41 +169,39 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { this.cfg.terminate(block, source_info, TerminatorKind::Goto { target: loop_block }); - let might_break = this.in_loop_scope(loop_block, exit_block, move |this| { - // conduct the test, if necessary - let body_block; - if let Some(cond_expr) = opt_cond_expr { - // This loop has a condition, ergo its exit_block is reachable. - this.find_loop_scope(expr_span, None).might_break = true; + this.in_loop_scope( + loop_block, exit_block, destination.clone(), + move |this| { + // conduct the test, if necessary + let body_block; + if let Some(cond_expr) = opt_cond_expr { + let loop_block_end; + let cond = unpack!( + loop_block_end = this.as_operand(loop_block, cond_expr)); + body_block = this.cfg.start_new_block(); + this.cfg.terminate(loop_block_end, source_info, + TerminatorKind::If { + cond: cond, + targets: (body_block, exit_block) + }); - let loop_block_end; - let cond = unpack!(loop_block_end = this.as_operand(loop_block, cond_expr)); - body_block = this.cfg.start_new_block(); - this.cfg.terminate(loop_block_end, source_info, - TerminatorKind::If { - cond: cond, - targets: (body_block, exit_block) - }); - } else { - body_block = loop_block; + // if the test is false, there's no `break` to assign `destination`, so + // we have to do it; this overwrites any `break`-assigned value but it's + // always `()` anyway + this.cfg.push_assign_unit(exit_block, source_info, destination); + } else { + body_block = loop_block; + } + + // The “return” value of the loop body must always be an unit. We therefore + // introduce a unit temporary as the destination for the loop body. + let tmp = this.get_unit_temp(); + // Execute the body, branching back to the test. + let body_block_end = unpack!(this.into(&tmp, body_block, body)); + this.cfg.terminate(body_block_end, source_info, + TerminatorKind::Goto { target: loop_block }); } - - // The “return” value of the loop body must always be an unit, but we cannot - // reuse that as a “return” value of the whole loop expressions, because some - // loops are diverging (e.g. `loop {}`). Thus, we introduce a unit temporary as - // the destination for the loop body and assign the loop’s own “return” value - // immediately after the iteration is finished. - let tmp = this.get_unit_temp(); - // Execute the body, branching back to the test. - let body_block_end = unpack!(this.into(&tmp, body_block, body)); - this.cfg.terminate(body_block_end, source_info, - TerminatorKind::Goto { target: loop_block }); - }); - // If the loop may reach its exit_block, we assign an empty tuple to the - // destination to keep the MIR well-formed. - if might_break { - this.cfg.push_assign_unit(exit_block, source_info, destination); - } + ); exit_block.unit() } ExprKind::Call { ty, fun, args } => { diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 4a1926e7c57..f04d630379a 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -11,9 +11,7 @@ use build::{BlockAnd, BlockAndExtension, Builder}; use build::scope::LoopScope; use hair::*; -use rustc::middle::region::CodeExtent; use rustc::mir::*; -use syntax_pos::Span; impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { @@ -79,14 +77,28 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { block.unit() } ExprKind::Continue { label } => { - this.break_or_continue(expr_span, label, block, - |loop_scope| loop_scope.continue_block) + let LoopScope { continue_block, extent, .. } = + *this.find_loop_scope(expr_span, label); + this.exit_scope(expr_span, extent, block, continue_block); + this.cfg.start_new_block().unit() } - ExprKind::Break { label } => { - this.break_or_continue(expr_span, label, block, |loop_scope| { - loop_scope.might_break = true; - loop_scope.break_block - }) + ExprKind::Break { label, value } => { + let (break_block, extent, destination) = { + let LoopScope { + break_block, + extent, + ref break_destination, + .. + } = *this.find_loop_scope(expr_span, label); + (break_block, extent, break_destination.clone()) + }; + if let Some(value) = value { + unpack!(block = this.into(&destination, block, value)) + } else { + this.cfg.push_assign_unit(block, source_info, &destination) + } + this.exit_scope(expr_span, extent, block, break_block); + this.cfg.start_new_block().unit() } ExprKind::Return { value } => { block = match value { @@ -115,20 +127,4 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } - fn break_or_continue(&mut self, - span: Span, - label: Option, - block: BasicBlock, - exit_selector: F) - -> BlockAnd<()> - where F: FnOnce(&mut LoopScope) -> BasicBlock - { - let (exit_block, extent) = { - let loop_scope = self.find_loop_scope(span, label); - (exit_selector(loop_scope), loop_scope.extent) - }; - self.exit_scope(span, extent, block, exit_block); - self.cfg.start_new_block().unit() - } - } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index d281b2a32d0..5713ee45b9d 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -38,7 +38,7 @@ pub struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { /// the current set of loops; see the `scope` module for more /// details - loop_scopes: Vec, + loop_scopes: Vec>, /// the vector of all scopes that we have created thus far; /// we track this for debuginfo later diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 4d9b6c0e05a..e5fac94a8a4 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -177,7 +177,7 @@ struct FreeData<'tcx> { } #[derive(Clone, Debug)] -pub struct LoopScope { +pub struct LoopScope<'tcx> { /// Extent of the loop pub extent: CodeExtent, /// Where the body of the loop begins @@ -185,8 +185,9 @@ pub struct LoopScope { /// Block to branch into when the loop terminates (either by being `break`-en out from, or by /// having its condition to become false) pub break_block: BasicBlock, - /// Indicates the reachability of the break_block for this loop - pub might_break: bool + /// The destination of the loop expression itself (i.e. where to put the result of a `break` + /// expression) + pub break_destination: Lvalue<'tcx>, } impl<'tcx> Scope<'tcx> { @@ -246,10 +247,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// /// Returns the might_break attribute of the LoopScope used. pub fn in_loop_scope(&mut self, - loop_block: BasicBlock, - break_block: BasicBlock, - f: F) - -> bool + loop_block: BasicBlock, + break_block: BasicBlock, + break_destination: Lvalue<'tcx>, + f: F) where F: FnOnce(&mut Builder<'a, 'gcx, 'tcx>) { let extent = self.extent_of_innermost_scope(); @@ -257,13 +258,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { extent: extent.clone(), continue_block: loop_block, break_block: break_block, - might_break: false + break_destination: break_destination, }; self.loop_scopes.push(loop_scope); f(self); let loop_scope = self.loop_scopes.pop().unwrap(); assert!(loop_scope.extent == extent); - loop_scope.might_break } /// Convenience wrapper that pushes a scope and then executes `f` @@ -386,7 +386,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { pub fn find_loop_scope(&mut self, span: Span, label: Option) - -> &mut LoopScope { + -> &mut LoopScope<'tcx> { let loop_scopes = &mut self.loop_scopes; match label { None => { diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 8a434cdff17..6fa26725935 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -558,8 +558,9 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, }, hir::ExprRet(ref v) => ExprKind::Return { value: v.to_ref() }, - hir::ExprBreak(label) => - ExprKind::Break { label: label.map(|_| loop_label(cx, expr)) }, + hir::ExprBreak(label, ref value) => + ExprKind::Break { label: label.map(|_| loop_label(cx, expr)), + value: value.to_ref() }, hir::ExprAgain(label) => ExprKind::Continue { label: label.map(|_| loop_label(cx, expr)) }, hir::ExprMatch(ref discr, ref arms, _) => @@ -572,7 +573,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, hir::ExprWhile(ref cond, ref body, _) => ExprKind::Loop { condition: Some(cond.to_ref()), body: block::to_expr_ref(cx, body) }, - hir::ExprLoop(ref body, _) => + hir::ExprLoop(ref body, _, _) => ExprKind::Loop { condition: None, body: block::to_expr_ref(cx, body) }, hir::ExprField(ref source, name) => { diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index e211334e547..50eee772396 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -202,6 +202,7 @@ pub enum ExprKind<'tcx> { }, Break { label: Option, + value: Option>, }, Continue { label: Option, diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 89c3efaafcd..22b584f0d04 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -106,7 +106,7 @@ impl<'a> Visitor for AstValidator<'a> { ExprKind::Loop(_, Some(ident)) | ExprKind::WhileLet(.., Some(ident)) | ExprKind::ForLoop(.., Some(ident)) | - ExprKind::Break(Some(ident)) | + ExprKind::Break(Some(ident), _) | ExprKind::Continue(Some(ident)) => { self.check_label(ident.node, ident.span, expr.id); } diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs index 5df8accd8ce..4d8520ed044 100644 --- a/src/librustc_passes/consts.rs +++ b/src/librustc_passes/consts.rs @@ -610,7 +610,7 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node hir::ExprLoop(..) | // More control flow (also not very meaningful). - hir::ExprBreak(_) | + hir::ExprBreak(..) | hir::ExprAgain(_) | hir::ExprRet(_) | diff --git a/src/librustc_passes/diagnostics.rs b/src/librustc_passes/diagnostics.rs index 89b8aa81411..b2ef1abd2a4 100644 --- a/src/librustc_passes/diagnostics.rs +++ b/src/librustc_passes/diagnostics.rs @@ -228,4 +228,5 @@ pub impl Foo for Bar { register_diagnostics! { E0472, // asm! is unsupported on this target E0561, // patterns aren't allowed in function pointer types + E0571, // `break` with a value in a non-`loop`-loop } diff --git a/src/librustc_passes/loops.rs b/src/librustc_passes/loops.rs index 724100e0223..c909e75afc1 100644 --- a/src/librustc_passes/loops.rs +++ b/src/librustc_passes/loops.rs @@ -12,34 +12,56 @@ use self::Context::*; use rustc::session::Session; use rustc::dep_graph::DepNode; +use rustc::hir::def::{Def, DefMap}; use rustc::hir::map::Map; use rustc::hir::intravisit::{self, Visitor}; use rustc::hir; use syntax_pos::Span; +#[derive(Clone, Copy, PartialEq)] +enum LoopKind { + Loop(hir::LoopSource), + WhileLoop, +} + +impl LoopKind { + fn name(self) -> &'static str { + match self { + LoopKind::Loop(hir::LoopSource::Loop) => "loop", + LoopKind::Loop(hir::LoopSource::WhileLet) => "while let", + LoopKind::Loop(hir::LoopSource::ForLoop) => "for", + LoopKind::WhileLoop => "while", + } + } +} + #[derive(Clone, Copy, PartialEq)] enum Context { Normal, - Loop, + Loop(LoopKind), Closure, } #[derive(Copy, Clone)] -struct CheckLoopVisitor<'a> { +struct CheckLoopVisitor<'a, 'ast: 'a> { sess: &'a Session, + def_map: &'a DefMap, + hir_map: &'a Map<'ast>, cx: Context, } -pub fn check_crate(sess: &Session, map: &Map) { +pub fn check_crate(sess: &Session, def_map: &DefMap, map: &Map) { let _task = map.dep_graph.in_task(DepNode::CheckLoops); let krate = map.krate(); krate.visit_all_item_likes(&mut CheckLoopVisitor { sess: sess, + def_map: def_map, + hir_map: map, cx: Normal, }.as_deep_visitor()); } -impl<'a, 'v> Visitor<'v> for CheckLoopVisitor<'a> { +impl<'a, 'ast, 'v> Visitor<'v> for CheckLoopVisitor<'a, 'ast> { fn visit_item(&mut self, i: &hir::Item) { self.with_context(Normal, |v| intravisit::walk_item(v, i)); } @@ -51,25 +73,62 @@ impl<'a, 'v> Visitor<'v> for CheckLoopVisitor<'a> { fn visit_expr(&mut self, e: &hir::Expr) { match e.node { hir::ExprWhile(ref e, ref b, _) => { - self.visit_expr(&e); - self.with_context(Loop, |v| v.visit_block(&b)); + self.with_context(Loop(LoopKind::WhileLoop), |v| { + v.visit_expr(&e); + v.visit_block(&b); + }); } - hir::ExprLoop(ref b, _) => { - self.with_context(Loop, |v| v.visit_block(&b)); + hir::ExprLoop(ref b, _, source) => { + self.with_context(Loop(LoopKind::Loop(source)), |v| v.visit_block(&b)); } hir::ExprClosure(.., ref b, _) => { self.with_context(Closure, |v| v.visit_expr(&b)); } - hir::ExprBreak(_) => self.require_loop("break", e.span), + hir::ExprBreak(ref opt_label, ref opt_expr) => { + if opt_expr.is_some() { + let loop_kind = if opt_label.is_some() { + let loop_def = self.def_map.get(&e.id).unwrap().full_def(); + if loop_def == Def::Err { + None + } else if let Def::Label(loop_id) = loop_def { + Some(match self.hir_map.expect_expr(loop_id).node { + hir::ExprWhile(..) => LoopKind::WhileLoop, + hir::ExprLoop(_, _, source) => LoopKind::Loop(source), + ref r => span_bug!(e.span, + "break label resolved to a non-loop: {:?}", r), + }) + } else { + span_bug!(e.span, "break resolved to a non-label") + } + } else if let Loop(kind) = self.cx { + Some(kind) + } else { + // `break` outside a loop - caught below + None + }; + match loop_kind { + 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, + &format!("can only break with a value inside `loop`")) + .emit(); + } + } + } + self.require_loop("break", e.span); + } hir::ExprAgain(_) => self.require_loop("continue", e.span), _ => intravisit::walk_expr(self, e), } } } -impl<'a> CheckLoopVisitor<'a> { +impl<'a, 'ast> CheckLoopVisitor<'a, 'ast> { fn with_context(&mut self, cx: Context, f: F) - where F: FnOnce(&mut CheckLoopVisitor<'a>) + where F: FnOnce(&mut CheckLoopVisitor<'a, 'ast>) { let old_cx = self.cx; self.cx = cx; @@ -79,7 +138,7 @@ impl<'a> CheckLoopVisitor<'a> { fn require_loop(&self, name: &str, span: Span) { match self.cx { - Loop => {} + Loop(_) => {} Closure => { struct_span_err!(self.sess, span, E0267, "`{}` inside of a closure", name) .span_label(span, &format!("cannot break inside of a closure")) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index c72ba7bb687..ab3d361a940 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3074,22 +3074,25 @@ impl<'a> Resolver<'a> { visit::walk_expr(self, expr); } - ExprKind::Break(Some(label)) | ExprKind::Continue(Some(label)) => { + ExprKind::Break(Some(label), _) | ExprKind::Continue(Some(label)) => { match self.search_label(label.node) { None => { self.record_def(expr.id, err_path_resolution()); resolve_error(self, label.span, - ResolutionError::UndeclaredLabel(&label.node.name.as_str())) + ResolutionError::UndeclaredLabel(&label.node.name.as_str())); } Some(def @ Def::Label(_)) => { // Since this def is a label, it is never read. - self.record_def(expr.id, PathResolution::new(def)) + self.record_def(expr.id, PathResolution::new(def)); } Some(_) => { - span_bug!(expr.span, "label wasn't mapped to a label def!") + span_bug!(expr.span, "label wasn't mapped to a label def!"); } } + + // visit `break` argument if any + visit::walk_expr(self, expr); } ExprKind::IfLet(ref pattern, ref subexpression, ref if_block, ref optional_else) => { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index bb6b2a3116b..eeb7bb28700 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -103,7 +103,7 @@ use session::{Session, CompileResult}; use CrateCtxt; use TypeAndSubsts; use lint; -use util::common::{block_query, ErrorReported, indenter, loop_query}; +use util::common::{ErrorReported, indenter}; use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, NodeMap}; use std::cell::{Cell, Ref, RefCell}; @@ -407,6 +407,34 @@ impl Diverges { } } +#[derive(Clone)] +pub struct LoopCtxt<'gcx, 'tcx> { + unified: Ty<'tcx>, + coerce_to: Ty<'tcx>, + break_exprs: Vec<&'gcx hir::Expr>, + may_break: bool, +} + +#[derive(Clone)] +pub struct EnclosingLoops<'gcx, 'tcx> { + stack: Vec>, + by_id: NodeMap, +} + +impl<'gcx, 'tcx> EnclosingLoops<'gcx, 'tcx> { + fn find_loop(&mut self, id: Option) -> Option<&mut LoopCtxt<'gcx, 'tcx>> { + if let Some(id) = id { + if let Some(ix) = self.by_id.get(&id).cloned() { + Some(&mut self.stack[ix]) + } else { + None + } + } else { + self.stack.last_mut() + } + } +} + #[derive(Clone)] pub struct FnCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { ast_ty_to_ty_cache: RefCell>>, @@ -433,6 +461,8 @@ pub struct FnCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { /// Whether any child nodes have any type errors. has_errors: Cell, + enclosing_loops: RefCell>, + inh: &'a Inherited<'a, 'gcx, 'tcx>, } @@ -1503,6 +1533,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ast::CRATE_NODE_ID)), diverges: Cell::new(Diverges::Maybe), has_errors: Cell::new(false), + enclosing_loops: RefCell::new(EnclosingLoops { + stack: Vec::new(), + by_id: NodeMap(), + }), inh: inh, } } @@ -3584,7 +3618,74 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } tcx.mk_nil() } - hir::ExprBreak(_) => { tcx.types.never } + hir::ExprBreak(ref label_opt, ref expr_opt) => { + let loop_id = if label_opt.is_some() { + let loop_def = tcx.expect_def(expr.id); + if let Def::Label(loop_id) = loop_def { + Some(Some(loop_id)) + } else if loop_def == Def::Err { + // an error was already printed, so just ignore it + None + } else { + span_bug!(expr.span, "break label resolved to a non-label"); + } + } else { + Some(None) + }; + if let Some(loop_id) = loop_id { + let coerce_to = { + let mut enclosing_loops = self.enclosing_loops.borrow_mut(); + enclosing_loops.find_loop(loop_id).map(|ctxt| ctxt.coerce_to) + }; + if let Some(coerce_to) = coerce_to { + let e_ty; + let cause; + if let Some(ref e) = *expr_opt { + // Recurse without `enclosing_loops` borrowed. + e_ty = self.check_expr_with_hint(e, coerce_to); + cause = self.misc(e.span); + // Notably, the recursive call may alter coerce_to - must not keep using it! + } else { + // `break` without argument acts like `break ()`. + e_ty = tcx.mk_nil(); + cause = self.misc(expr.span); + } + let mut enclosing_loops = self.enclosing_loops.borrow_mut(); + let ctxt = enclosing_loops.find_loop(loop_id).unwrap(); + + let result = if let Some(ref e) = *expr_opt { + // Special-case the first element, as it has no "previous expressions". + let result = if !ctxt.may_break { + self.try_coerce(e, e_ty, ctxt.coerce_to) + } else { + self.try_find_coercion_lub(&cause, || ctxt.break_exprs.iter().cloned(), + ctxt.unified, e, e_ty) + }; + + ctxt.break_exprs.push(e); + result + } else { + self.eq_types(true, &cause, e_ty, ctxt.unified) + .map(|InferOk { obligations, .. }| { + // FIXME(#32730) propagate obligations + assert!(obligations.is_empty()); + e_ty + }) + }; + match result { + Ok(ty) => ctxt.unified = ty, + Err(err) => { + self.report_mismatched_types(&cause, ctxt.unified, e_ty, err); + } + } + + ctxt.may_break = true; + } + // Otherwise, we failed to find the enclosing loop; this can only happen if the + // `break` was not inside a loop at all, which is caught by the loop-checking pass. + } + tcx.types.never + } hir::ExprAgain(_) => { tcx.types.never } hir::ExprRet(ref expr_opt) => { if let Some(ref e) = *expr_opt { @@ -3635,12 +3736,22 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { expr.span, expected) } hir::ExprWhile(ref cond, ref body, _) => { - self.check_expr_has_type(&cond, tcx.types.bool); - let cond_diverging = self.diverges.get(); - self.check_block_no_value(&body); + let unified = self.tcx.mk_nil(); + let coerce_to = unified; + let ctxt = LoopCtxt { + unified: unified, + coerce_to: coerce_to, + break_exprs: vec![], + may_break: true, + }; + self.with_loop_ctxt(expr.id, ctxt, || { + self.check_expr_has_type(&cond, tcx.types.bool); + let cond_diverging = self.diverges.get(); + self.check_block_no_value(&body); - // We may never reach the body so it diverging means nothing. - self.diverges.set(cond_diverging); + // We may never reach the body so it diverging means nothing. + self.diverges.set(cond_diverging); + }); if self.has_errors.get() { tcx.types.err @@ -3648,14 +3759,25 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { tcx.mk_nil() } } - hir::ExprLoop(ref body, _) => { - self.check_block_no_value(&body); - if may_break(tcx, expr.id, &body) { + hir::ExprLoop(ref body, _, _) => { + let unified = self.next_ty_var(); + let coerce_to = expected.only_has_type(self).unwrap_or(unified); + let ctxt = LoopCtxt { + unified: unified, + coerce_to: coerce_to, + break_exprs: vec![], + may_break: false, + }; + + let ctxt = self.with_loop_ctxt(expr.id, ctxt, || { + self.check_block_no_value(&body); + }); + if ctxt.may_break { // No way to know whether it's diverging because // of a `break` or an outer `break` or `return. self.diverges.set(Diverges::Maybe); - tcx.mk_nil() + ctxt.unified } else { tcx.types.never } @@ -4531,27 +4653,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.tcx.types.err }) } -} -// Returns true if b contains a break that can exit from b -pub fn may_break(tcx: TyCtxt, id: ast::NodeId, b: &hir::Block) -> bool { - // First: is there an unlabeled break immediately - // inside the loop? - (loop_query(&b, |e| { - match *e { - hir::ExprBreak(None) => true, - _ => false + fn with_loop_ctxt(&self, id: ast::NodeId, ctxt: LoopCtxt<'gcx, 'tcx>, f: F) + -> LoopCtxt<'gcx, 'tcx> { + let index; + { + let mut enclosing_loops = self.enclosing_loops.borrow_mut(); + index = enclosing_loops.stack.len(); + enclosing_loops.by_id.insert(id, index); + enclosing_loops.stack.push(ctxt); } - })) || - // Second: is there a labeled break with label - // nested anywhere inside the loop? - (block_query(b, |e| { - if let hir::ExprBreak(Some(_)) = e.node { - tcx.expect_def(e.id) == Def::Label(id) - } else { - false + f(); + { + let mut enclosing_loops = self.enclosing_loops.borrow_mut(); + debug_assert!(enclosing_loops.stack.len() == index + 1); + enclosing_loops.by_id.remove(&id).expect("missing loop context"); + (enclosing_loops.stack.pop().expect("missing loop context")) } - })) + } } pub fn check_bounds_are_used<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 0cd481baef1..c613b62bf2d 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -742,7 +742,7 @@ impl<'a, 'gcx, 'tcx, 'v> Visitor<'v> for RegionCtxt<'a, 'gcx, 'tcx> { self.check_expr_fn_block(expr, &body); } - hir::ExprLoop(ref body, _) => { + hir::ExprLoop(ref body, _, _) => { let repeating_scope = self.set_repeating_scope(body.id); intravisit::walk_expr(self, expr); self.set_repeating_scope(repeating_scope); diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index bb07efdd9e7..2a911aceb9d 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -999,8 +999,8 @@ pub enum ExprKind { /// A referencing operation (`&a` or `&mut a`) AddrOf(Mutability, P), - /// A `break`, with an optional label to break - Break(Option), + /// A `break`, with an optional label to break, and an optional expression + Break(Option, Option>), /// A `continue`, with an optional label Continue(Option), /// A `return`, with an optional value to be returned diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 324afc20051..a208b934d79 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -777,7 +777,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { fn expr_break(&self, sp: Span) -> P { - self.expr(sp, ast::ExprKind::Break(None)) + self.expr(sp, ast::ExprKind::Break(None, None)) } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 16d4adf1705..aa6a29b78b0 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -313,6 +313,9 @@ declare_features! ( (active, link_cfg, "1.14.0", Some(37406)), (active, use_extern_macros, "1.15.0", Some(35896)), + + // Allows `break {expr}` with a value inside `loop`s. + (active, loop_break_value, "1.14.0", Some(37339)), ); declare_features! ( @@ -1189,6 +1192,10 @@ impl<'a> Visitor for PostExpansionVisitor<'a> { } } } + ast::ExprKind::Break(_, Some(_)) => { + gate_feature_post!(&self, loop_break_value, e.span, + "`break` with a value is experimental"); + } _ => {} } visit::walk_expr(self, e); diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index ff0255a2f21..6af8efb2a19 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1238,10 +1238,11 @@ pub fn noop_fold_expr(Expr {id, node, span, attrs}: Expr, folder: &mu }); ExprKind::Path(qself, folder.fold_path(path)) } - ExprKind::Break(opt_ident) => ExprKind::Break(opt_ident.map(|label| - respan(folder.new_span(label.span), - folder.fold_ident(label.node))) - ), + ExprKind::Break(opt_ident, opt_expr) => { + ExprKind::Break(opt_ident.map(|label| respan(folder.new_span(label.span), + folder.fold_ident(label.node))), + opt_expr.map(|e| folder.fold_expr(e))) + } ExprKind::Continue(opt_ident) => ExprKind::Continue(opt_ident.map(|label| respan(folder.new_span(label.span), folder.fold_ident(label.node))) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 4997e464c2b..10797c0038a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2260,15 +2260,25 @@ impl<'a> Parser<'a> { ex = ExprKind::Ret(None); } } else if self.eat_keyword(keywords::Break) { - if self.token.is_lifetime() { - ex = ExprKind::Break(Some(Spanned { + let lt = if self.token.is_lifetime() { + let spanned_lt = Spanned { node: self.get_lifetime(), span: self.span - })); + }; self.bump(); + Some(spanned_lt) } else { - ex = ExprKind::Break(None); - } + None + }; + let e = if self.token.can_begin_expr() + && !(self.token == token::OpenDelim(token::Brace) + && self.restrictions.contains( + Restrictions::RESTRICTION_NO_STRUCT_LITERAL)) { + Some(self.parse_expr()?) + } else { + None + }; + ex = ExprKind::Break(lt, e); hi = self.prev_span.hi; } else if self.token.is_keyword(keywords::Let) { // Catch this syntax error here, instead of in `check_strict_keywords`, so diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 3820f5ea90c..c28b9d00501 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -2191,13 +2191,17 @@ impl<'a> State<'a> { ast::ExprKind::Path(Some(ref qself), ref path) => { try!(self.print_qpath(path, qself, true)) } - ast::ExprKind::Break(opt_ident) => { + ast::ExprKind::Break(opt_ident, ref opt_expr) => { try!(word(&mut self.s, "break")); try!(space(&mut self.s)); if let Some(ident) = opt_ident { try!(self.print_ident(ident.node)); try!(space(&mut self.s)); } + if let Some(ref expr) = *opt_expr { + try!(self.print_expr(expr)); + try!(space(&mut self.s)); + } } ast::ExprKind::Continue(opt_ident) => { try!(word(&mut self.s, "continue")); diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 7c1ff617ab6..da36225fb32 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -746,7 +746,11 @@ pub fn walk_expr(visitor: &mut V, expression: &Expr) { } visitor.visit_path(path, expression.id) } - ExprKind::Break(ref opt_sp_ident) | ExprKind::Continue(ref opt_sp_ident) => { + ExprKind::Break(ref opt_sp_ident, ref opt_expr) => { + walk_opt_sp_ident(visitor, opt_sp_ident); + walk_list!(visitor, visit_expr, opt_expr); + } + ExprKind::Continue(ref opt_sp_ident) => { walk_opt_sp_ident(visitor, opt_sp_ident); } ExprKind::Ret(ref optional_expression) => { diff --git a/src/test/compile-fail/feature-gate-loop-break-value.rs b/src/test/compile-fail/feature-gate-loop-break-value.rs new file mode 100644 index 00000000000..1632c40d59f --- /dev/null +++ b/src/test/compile-fail/feature-gate-loop-break-value.rs @@ -0,0 +1,15 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + loop { + break 123; //~ ERROR `break` with a value is experimental + } +} diff --git a/src/test/compile-fail/loop-break-value.rs b/src/test/compile-fail/loop-break-value.rs new file mode 100644 index 00000000000..d4f29597486 --- /dev/null +++ b/src/test/compile-fail/loop-break-value.rs @@ -0,0 +1,101 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(loop_break_value)] +#![feature(never_type)] + +fn main() { + let val: ! = loop { break break; }; + //~^ ERROR mismatched types + + loop { + if true { + break "asdf"; + } else { + break 123; //~ ERROR mismatched types + } + }; + + let _: i32 = loop { + break "asdf"; //~ ERROR mismatched types + }; + + let _: i32 = 'outer_loop: loop { + loop { + break 'outer_loop "nope"; //~ ERROR mismatched types + break "ok"; + }; + }; + + 'while_loop: while true { + break; + break (); //~ ERROR `break` with value from a `while` loop + loop { + break 'while_loop 123; + //~^ ERROR `break` with value from a `while` loop + //~| ERROR mismatched types + break 456; + break 789; + }; + } + + 'while_let_loop: while let Some(_) = Some(()) { + if break () { //~ ERROR `break` with value from a `while let` loop + break; + break None; + //~^ ERROR `break` with value from a `while let` loop + //~| ERROR mismatched types + } + loop { + break 'while_let_loop "nope"; + //~^ ERROR `break` with value from a `while let` loop + //~| ERROR mismatched types + break 33; + }; + } + + 'for_loop: for _ in &[1,2,3] { + break (); //~ ERROR `break` with value from a `for` loop + break [()]; + //~^ ERROR `break` with value from a `for` loop + //~| ERROR mismatched types + loop { + break Some(3); + break 'for_loop Some(17); + //~^ ERROR `break` with value from a `for` loop + //~| ERROR mismatched types + }; + } + + let _: i32 = 'a: loop { + let _: () = 'b: loop { + break ('c: loop { + break; + break 'c 123; //~ ERROR mismatched types + }); + break 'a 123; + }; + }; + + loop { + break (break, break); //~ ERROR mismatched types + }; + + loop { + break; + break 2; //~ ERROR mismatched types + }; + + loop { + break 2; + break; //~ ERROR mismatched types + break 4; + }; +} diff --git a/src/test/run-pass/loop-break-value.rs b/src/test/run-pass/loop-break-value.rs new file mode 100644 index 00000000000..6a5e051c0c7 --- /dev/null +++ b/src/test/run-pass/loop-break-value.rs @@ -0,0 +1,133 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(loop_break_value)] +#![feature(never_type)] + +#[allow(unused)] +fn never_returns() { + loop { + break loop {}; + } +} + +pub fn main() { + let value = 'outer: loop { + if 1 == 1 { + break 13; + } else { + let _never: ! = loop { + break loop { + break 'outer panic!(); + } + }; + } + }; + assert_eq!(value, 13); + + let x = [1, 3u32, 5]; + let y = [17]; + let z = []; + let coerced: &[_] = loop { + match 2 { + 1 => break &x, + 2 => break &y, + 3 => break &z, + _ => (), + } + }; + assert_eq!(coerced, &[17u32]); + + let trait_unified = loop { + break if true { + break Default::default() + } else { + break [13, 14] + }; + }; + assert_eq!(trait_unified, [0, 0]); + + let trait_unified_2 = loop { + if false { + break [String::from("Hello")] + } else { + break Default::default() + }; + }; + assert_eq!(trait_unified_2, [""]); + + let trait_unified_3 = loop { + break if false { + break [String::from("Hello")] + } else { + ["Yes".into()] + }; + }; + assert_eq!(trait_unified_3, ["Yes"]); + + let regular_break = loop { + if true { + break; + } else { + break break Default::default(); + } + }; + assert_eq!(regular_break, ()); + + let regular_break_2 = loop { + if true { + break Default::default(); + } else { + break; + } + }; + assert_eq!(regular_break_2, ()); + + let regular_break_3 = loop { + break if true { + Default::default() + } else { + break; + } + }; + assert_eq!(regular_break_3, ()); + + let regular_break_4 = loop { + break (); + break; + }; + assert_eq!(regular_break_4, ()); + + let regular_break_5 = loop { + break; + break (); + }; + assert_eq!(regular_break_5, ()); + + let nested_break_value = 'outer2: loop { + let _a: u32 = 'inner: loop { + if true { + break 'outer2 "hello"; + } else { + break 'inner 17; + } + }; + panic!(); + }; + assert_eq!(nested_break_value, "hello"); + + let break_from_while_cond = loop { + while break { + panic!(); + } + break 123; + }; + assert_eq!(break_from_while_cond, 123); +}