Auto merge of #37487 - goffrie:break, r=nikomatsakis

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, loops no longer construct a `()` value at
  termination of the loop; rather, the `break` expression assigns the
  result of the loop.
- ~~I have also changed the loop scoping in MIR-building so that the test
  of a while loop is not considered to be part of that loop. This makes
  the rules consistent with #37360. The new loop scopes in typeck also
  follow this rule. That means that `loop { while (break) {} }` now
  terminates instead of looping forever. This is technically a breaking
  change.~~
- ~~On that note, expressions like `while break {}` and `if break {}` no
  longer parse because `{}` is interpreted as an expression argument to
  `break`. But no code except compiler test cases should do that anyway
  because it makes no sense.~~
- 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

@@ -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()

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) => {

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<Attribute>) -> P<hir::Expr> {
-        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<hir::Expr>, args: hir::HirVec<hir::Expr>)

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<Block>, Option<Spanned<Name>>),
+    ExprLoop(P<Block>, Option<Spanned<Name>>, 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<Expr>, HirVec<Arm>, MatchSource),
@@ -944,7 +944,7 @@ pub enum Expr_ {
     /// A referencing operation (`&a` or `&mut a`)
     ExprAddrOf(Mutability, P<Expr>),
     /// A `break`, with an optional label to break
-    ExprBreak(Option<Spanned<Name>>),
+    ExprBreak(Option<Spanned<Name>>, Option<P<Expr>>),
     /// A `continue`, with an optional label
     ExprAgain(Option<Spanned<Name>>),
     /// 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,

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")?;

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);
                 }

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,15 +1035,15 @@ 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);
 
               // Now that we know the label we're going to,
               // 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")
               }
           }
 

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);
             }
 

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,
         }
     }

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<P> where P: FnMut(&hir::Expr_) -> bool {
-    p: P,
-    flag: bool,
-}
-
-impl<'v, P> Visitor<'v> for LoopQueryVisitor<P> 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<P>(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<P> where P: FnMut(&hir::Expr) -> bool {
-    p: P,
-    flag: bool,
-}
-
-impl<'v, P> Visitor<'v> for BlockQueryVisitor<P> 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<P>(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;

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",

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),