Normalize labeled and unlabeled breaks

src/librustc/cfg/construct.rs

@@ -579,17 +579,12 @@ fn add_returning_edge(&mut self,
 
     fn find_scope(&self,
                   expr: &hir::Expr,
-                  label: Option<hir::Label>) -> LoopScope {
-        match label {
-            None => *self.loop_scopes.last().unwrap(),
-            Some(label) => {
-                for l in &self.loop_scopes {
-                    if l.loop_id == label.loop_id {
-                        return *l;
-                    }
-                }
-                span_bug!(expr.span, "no loop scope for id {}", label.loop_id);
+                  label: hir::Label) -> LoopScope {
+        for l in &self.loop_scopes {
+            if l.loop_id == label.loop_id {
+                return *l;
             }
         }
+        span_bug!(expr.span, "no loop scope for id {}", label.loop_id);
     }
 }

src/librustc/hir/intravisit.rs

@@ -1006,18 +1006,18 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
         ExprPath(ref qpath) => {
             visitor.visit_qpath(qpath, expression.id, expression.span);
         }
-        ExprBreak(None, ref opt_expr) => {
+        ExprBreak(label, ref opt_expr) => {
+            label.ident.map(|ident| {
+                visitor.visit_def_mention(Def::Label(label.loop_id));
+                visitor.visit_name(ident.span, ident.node.name);
+            });
             walk_list!(visitor, visit_expr, opt_expr);
         }
-        ExprBreak(Some(label), ref opt_expr) => {
-            visitor.visit_def_mention(Def::Label(label.loop_id));
-            visitor.visit_name(label.span, label.name);
-            walk_list!(visitor, visit_expr, opt_expr);
-        }
-        ExprAgain(None) => {}
-        ExprAgain(Some(label)) => {
-            visitor.visit_def_mention(Def::Label(label.loop_id));
-            visitor.visit_name(label.span, label.name);
+        ExprAgain(label) => {
+            label.ident.map(|ident| {
+                visitor.visit_def_mention(Def::Label(label.loop_id));
+                visitor.visit_name(ident.span, ident.node.name);
+            });
         }
         ExprRet(ref optional_expression) => {
             walk_list!(visitor, visit_expr, optional_expression);

src/librustc/hir/lowering.rs

@@ -50,6 +50,7 @@
 
 use std::collections::BTreeMap;
 use std::iter;
+use std::mem;
 
 use syntax::attr;
 use syntax::ast::*;
@@ -79,6 +80,8 @@ pub struct LoweringContext<'a> {
     impl_items: BTreeMap<hir::ImplItemId, hir::ImplItem>,
     bodies: FxHashMap<hir::BodyId, hir::Body>,
 
+    loop_scopes: Vec<NodeId>,
+
     type_def_lifetime_params: DefIdMap<usize>,
 }
 
@@ -112,6 +115,7 @@ pub fn lower_crate(sess: &Session,
         trait_items: BTreeMap::new(),
         impl_items: BTreeMap::new(),
         bodies: FxHashMap(),
+        loop_scopes: Vec::new(),
         type_def_lifetime_params: DefIdMap(),
     }.lower_crate(krate)
 }
@@ -244,6 +248,27 @@ fn allow_internal_unstable(&self, reason: &'static str, mut span: Span) -> Span
         span
     }
 
+    fn with_loop_scope<T, F>(&mut self, loop_id: NodeId, f: F) -> T
+        where F: FnOnce(&mut LoweringContext) -> T
+    {
+        let len = self.loop_scopes.len();
+        self.loop_scopes.push(loop_id);
+        let result = f(self);
+        assert_eq!(len + 1, self.loop_scopes.len(),
+            "Loop scopes should be added and removed in stack order");
+        self.loop_scopes.pop().unwrap();
+        result
+    }
+
+    fn with_new_loop_scopes<T, F>(&mut self, f: F) -> T
+        where F: FnOnce(&mut LoweringContext) -> T
+    {
+        let loop_scopes = mem::replace(&mut self.loop_scopes, Vec::new());
+        let result = f(self);
+        mem::replace(&mut self.loop_scopes, loop_scopes);
+        result
+    }
+
     fn with_parent_def<T, F>(&mut self, parent_id: NodeId, f: F) -> T
         where F: FnOnce(&mut LoweringContext) -> T
     {
@@ -271,17 +296,23 @@ fn lower_opt_sp_ident(&mut self, o_id: Option<Spanned<Ident>>) -> Option<Spanned
         o_id.map(|sp_ident| respan(sp_ident.span, sp_ident.node.name))
     }
 
-    fn lower_label(&mut self, id: NodeId, label: Option<Spanned<Ident>>) -> Option<hir::Label> {
-        label.map(|sp_ident| {
-            hir::Label {
-                span: sp_ident.span,
-                name: sp_ident.node.name,
+    fn lower_label(&mut self, label: Option<(NodeId, Spanned<Ident>)>) -> hir::Label {
+        match label {
+            Some((id, label_ident)) => hir::Label {
+                ident: Some(label_ident),
                 loop_id: match self.expect_full_def(id) {
                     Def::Label(loop_id) => loop_id,
                     _ => DUMMY_NODE_ID
                 }
+            },
+            None => hir::Label {
+                ident: None,
+                loop_id: match self.loop_scopes.last() {
+                    Some(innermost_loop_id) => *innermost_loop_id,
+                    _ => DUMMY_NODE_ID
+                }
             }
-        })
+        }
     }
 
     fn lower_attrs(&mut self, attrs: &Vec<Attribute>) -> hir::HirVec<Attribute> {
@@ -992,15 +1023,17 @@ fn lower_item_kind(&mut self,
                                self.record_body(value, None))
             }
             ItemKind::Fn(ref decl, unsafety, constness, abi, ref generics, ref body) => {
-                let body = self.lower_block(body);
-                let body = self.expr_block(body, ThinVec::new());
-                let body_id = self.record_body(body, Some(decl));
-                hir::ItemFn(self.lower_fn_decl(decl),
-                            self.lower_unsafety(unsafety),
-                            self.lower_constness(constness),
-                            abi,
-                            self.lower_generics(generics),
-                            body_id)
+                self.with_new_loop_scopes(|this| {
+                    let body = this.lower_block(body);
+                    let body = this.expr_block(body, ThinVec::new());
+                    let body_id = this.record_body(body, Some(decl));
+                    hir::ItemFn(this.lower_fn_decl(decl),
+                                              this.lower_unsafety(unsafety),
+                                              this.lower_constness(constness),
+                                              abi,
+                                              this.lower_generics(generics),
+                                              body_id)
+                })
             }
             ItemKind::Mod(ref m) => hir::ItemMod(self.lower_mod(m)),
             ItemKind::ForeignMod(ref nm) => hir::ItemForeignMod(self.lower_foreign_mod(nm)),
@@ -1562,13 +1595,17 @@ fn lower_expr(&mut self, e: &Expr) -> hir::Expr {
                     hir::ExprIf(P(self.lower_expr(cond)), self.lower_block(blk), else_opt)
                 }
                 ExprKind::While(ref cond, ref body, opt_ident) => {
-                    hir::ExprWhile(P(self.lower_expr(cond)), self.lower_block(body),
-                                   self.lower_opt_sp_ident(opt_ident))
+                    self.with_loop_scope(e.id, |this|
+                        hir::ExprWhile(
+                            P(this.lower_expr(cond)),
+                            this.lower_block(body),
+                            this.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::LoopSource::Loop)
+                    self.with_loop_scope(e.id, |this|
+                        hir::ExprLoop(this.lower_block(body),
+                                      this.lower_opt_sp_ident(opt_ident),
+                                      hir::LoopSource::Loop))
                 }
                 ExprKind::Match(ref expr, ref arms) => {
                     hir::ExprMatch(P(self.lower_expr(expr)),
@@ -1576,12 +1613,14 @@ fn lower_expr(&mut self, e: &Expr) -> hir::Expr {
                                    hir::MatchSource::Normal)
                 }
                 ExprKind::Closure(capture_clause, ref decl, ref body, fn_decl_span) => {
-                    self.with_parent_def(e.id, |this| {
-                        let expr = this.lower_expr(body);
-                        hir::ExprClosure(this.lower_capture_clause(capture_clause),
-                                         this.lower_fn_decl(decl),
-                                         this.record_body(expr, Some(decl)),
-                                         fn_decl_span)
+                    self.with_new_loop_scopes(|this| {
+                        this.with_parent_def(e.id, |this| {
+                            let expr = this.lower_expr(body);
+                            hir::ExprClosure(this.lower_capture_clause(capture_clause),
+                                             this.lower_fn_decl(decl),
+                                             this.record_body(expr, Some(decl)),
+                                             fn_decl_span)
+                        })
                     })
                 }
                 ExprKind::Block(ref blk) => hir::ExprBlock(self.lower_block(blk)),
@@ -1660,10 +1699,13 @@ fn make_struct(this: &mut LoweringContext,
                     hir::ExprPath(self.lower_qpath(e.id, qself, path, ParamMode::Optional))
                 }
                 ExprKind::Break(opt_ident, ref opt_expr) => {
-                    hir::ExprBreak(self.lower_label(e.id, opt_ident),
+                    hir::ExprBreak(
+                        self.lower_label(opt_ident.map(|ident| (e.id, ident))),
                                    opt_expr.as_ref().map(|x| P(self.lower_expr(x))))
                 }
-                ExprKind::Continue(opt_ident) => hir::ExprAgain(self.lower_label(e.id, opt_ident)),
+                ExprKind::Continue(opt_ident) =>
+                    hir::ExprAgain(
+                        self.lower_label(opt_ident.map(|ident| (e.id, ident)))),
                 ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| P(self.lower_expr(x)))),
                 ExprKind::InlineAsm(ref asm) => {
                     let hir_asm = hir::InlineAsm {
@@ -1804,9 +1846,14 @@ fn make_struct(this: &mut LoweringContext,
                     //     }
                     //   }
 
+                    let (body, break_expr, sub_expr) = self.with_loop_scope(e.id, |this| (
+                        this.lower_block(body),
+                        this.expr_break(e.span, ThinVec::new()),
+                        P(this.lower_expr(sub_expr)),
+                    ));
+
                     // `<pat> => <body>`
                     let pat_arm = {
-                        let body = self.lower_block(body);
                         let body_expr = P(self.expr_block(body, ThinVec::new()));
                         let pat = self.lower_pat(pat);
                         self.arm(hir_vec![pat], body_expr)
@@ -1815,13 +1862,11 @@ fn make_struct(this: &mut LoweringContext,
                     // `_ => break`
                     let break_arm = {
                         let pat_under = self.pat_wild(e.span);
-                        let break_expr = self.expr_break(e.span, ThinVec::new());
                         self.arm(hir_vec![pat_under], break_expr)
                     };
 
                     // `match <sub_expr> { ... }`
                     let arms = hir_vec![pat_arm, break_arm];
-                    let sub_expr = P(self.lower_expr(sub_expr));
                     let match_expr = self.expr(e.span,
                                                hir::ExprMatch(sub_expr,
                                                               arms,
@@ -1863,7 +1908,7 @@ fn make_struct(this: &mut LoweringContext,
 
                     // `::std::option::Option::Some(<pat>) => <body>`
                     let pat_arm = {
-                        let body_block = self.lower_block(body);
+                        let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body));
                         let body_expr = P(self.expr_block(body_block, ThinVec::new()));
                         let pat = self.lower_pat(pat);
                         let some_pat = self.pat_some(e.span, pat);
@@ -1873,7 +1918,8 @@ fn make_struct(this: &mut LoweringContext,
 
                     // `::std::option::Option::None => break`
                     let break_arm = {
-                        let break_expr = self.expr_break(e.span, ThinVec::new());
+                        let break_expr = self.with_loop_scope(e.id, |this|
+                            this.expr_break(e.span, ThinVec::new()));
                         let pat = self.pat_none(e.span);
                         self.arm(hir_vec![pat], break_expr)
                     };
@@ -2151,7 +2197,8 @@ fn field(&mut self, name: Name, expr: P<hir::Expr>, span: Span) -> hir::Field {
     }
 
     fn expr_break(&mut self, span: Span, attrs: ThinVec<Attribute>) -> P<hir::Expr> {
-        P(self.expr(span, hir::ExprBreak(None, None), attrs))
+        let expr_break = hir::ExprBreak(self.lower_label(None), None);
+        P(self.expr(span, expr_break, attrs))
     }
 
     fn expr_call(&mut self, span: Span, e: P<hir::Expr>, args: hir::HirVec<hir::Expr>)

src/librustc/hir/mod.rs

@@ -36,7 +36,7 @@
 use syntax_pos::{Span, ExpnId, DUMMY_SP};
 use syntax::codemap::{self, Spanned};
 use syntax::abi::Abi;
-use syntax::ast::{Name, NodeId, DUMMY_NODE_ID, AsmDialect};
+use syntax::ast::{Ident, Name, NodeId, DUMMY_NODE_ID, AsmDialect};
 use syntax::ast::{Attribute, Lit, StrStyle, FloatTy, IntTy, UintTy, MetaItem};
 use syntax::ptr::P;
 use syntax::symbol::{Symbol, keywords};
@@ -959,9 +959,9 @@ pub enum Expr_ {
     /// A referencing operation (`&a` or `&mut a`)
     ExprAddrOf(Mutability, P<Expr>),
     /// A `break`, with an optional label to break
-    ExprBreak(Option<Label>, Option<P<Expr>>),
+    ExprBreak(Label, Option<P<Expr>>),
     /// A `continue`, with an optional label
-    ExprAgain(Option<Label>),
+    ExprAgain(Label),
     /// A `return`, with an optional value to be returned
     ExprRet(Option<P<Expr>>),
 
@@ -1033,8 +1033,7 @@ pub enum LoopSource {
 
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
 pub struct Label {
-    pub span: Span,
-    pub name: Name,
+    pub ident: Option<Spanned<Ident>>,
     pub loop_id: NodeId
 }
 

src/librustc/hir/print.rs

@@ -1354,11 +1354,11 @@ pub fn print_expr(&mut self, expr: &hir::Expr) -> io::Result<()> {
             hir::ExprPath(ref qpath) => {
                 self.print_qpath(qpath, true)?
             }
-            hir::ExprBreak(opt_label, ref opt_expr) => {
+            hir::ExprBreak(label, ref opt_expr) => {
                 word(&mut self.s, "break")?;
                 space(&mut self.s)?;
-                if let Some(label) = opt_label {
-                    self.print_name(label.name)?;
+                if let Some(label_ident) = label.ident {
+                    self.print_name(label_ident.node.name)?;
                     space(&mut self.s)?;
                 }
                 if let Some(ref expr) = *opt_expr {
@@ -1366,11 +1366,11 @@ pub fn print_expr(&mut self, expr: &hir::Expr) -> io::Result<()> {
                     space(&mut self.s)?;
                 }
             }
-            hir::ExprAgain(opt_label) => {
+            hir::ExprAgain(label) => {
                 word(&mut self.s, "continue")?;
                 space(&mut self.s)?;
-                if let Some(label) = opt_label {
-                    self.print_name(label.name)?;
+                if let Some(label_ident) = label.ident {
+                    self.print_name(label_ident.node.name)?;
                     space(&mut self.s)?
                 }
             }

src/librustc/middle/liveness.rs

@@ -675,23 +675,6 @@ fn write_vars<F>(&self,
         Ok(())
     }
 
-    fn find_loop_scope(&self,
-                       opt_label: Option<hir::Label>,
-                       sp: Span)
-                       -> NodeId {
-        match opt_label {
-            Some(label) => label.loop_id,
-            None => {
-                // Vanilla 'break' or 'continue', so use the enclosing
-                // loop scope
-                if self.loop_scope.is_empty() {
-                    span_bug!(sp, "break outside loop");
-                } else {
-                    *self.loop_scope.last().unwrap()
-                }
-            }
-        }
-    }
 
     #[allow(unused_must_use)]
     fn ln_str(&self, ln: LiveNode) -> String {
@@ -1018,9 +1001,9 @@ fn propagate_through_expr(&mut self, expr: &Expr, succ: LiveNode)
             self.propagate_through_opt_expr(o_e.as_ref().map(|e| &**e), exit_ln)
           }
 
-          hir::ExprBreak(opt_label, ref opt_expr) => {
+          hir::ExprBreak(label, ref opt_expr) => {
               // Find which label this break jumps to
-              let sc = self.find_loop_scope(opt_label, expr.span);
+              let sc = label.loop_id;
 
               // Now that we know the label we're going to,
               // look it up in the break loop nodes table
@@ -1031,9 +1014,9 @@ fn propagate_through_expr(&mut self, expr: &Expr, succ: LiveNode)
               }
           }
 
-          hir::ExprAgain(opt_label) => {
+          hir::ExprAgain(label) => {
               // Find which label this expr continues to
-              let sc = self.find_loop_scope(opt_label, expr.span);
+              let sc = label.loop_id;
 
               // Now that we know the label we're going to,
               // look it up in the continue loop nodes table

src/librustc_incremental/calculate_svh/svh_visitor.rs

@@ -338,8 +338,10 @@ fn saw_expr<'a>(node: &'a Expr_,
         ExprIndex(..)            => (SawExprIndex, true),
         ExprPath(_)              => (SawExprPath, false),
         ExprAddrOf(m, _)         => (SawExprAddrOf(m), false),
-        ExprBreak(label, _)      => (SawExprBreak(label.map(|l| l.name.as_str())), false),
-        ExprAgain(label)         => (SawExprAgain(label.map(|l| l.name.as_str())), false),
+        ExprBreak(label, _)      => (SawExprBreak(label.ident.map(|i|
+                                                    i.node.name.as_str())), false),
+        ExprAgain(label)         => (SawExprAgain(label.ident.map(|i|
+                                                    i.node.name.as_str())), false),
         ExprRet(..)              => (SawExprRet, false),
         ExprInlineAsm(ref a,..)  => (SawExprInlineAsm(StableInlineAsm(a)), false),
         ExprStruct(..)           => (SawExprStruct, false),

src/librustc_mir/build/scope.rs

@@ -385,22 +385,14 @@ pub fn new_visibility_scope(&mut self, span: Span) -> VisibilityScope {
     /// resolving `break` and `continue`.
     pub fn find_loop_scope(&mut self,
                            span: Span,
-                           label: Option<CodeExtent>)
+                           label: CodeExtent)
                            -> &mut LoopScope<'tcx> {
-        let loop_scopes = &mut self.loop_scopes;
-        match label {
-            None => {
-                // no label? return the innermost loop scope
-                loop_scopes.iter_mut().rev().next()
-            }
-            Some(label) => {
-                // otherwise, find the loop-scope with the correct id
-                loop_scopes.iter_mut()
-                           .rev()
-                           .filter(|loop_scope| loop_scope.extent == label)
-                           .next()
-            }
-        }.unwrap_or_else(|| span_bug!(span, "no enclosing loop scope found?"))
+        // find the loop-scope with the correct id
+        self.loop_scopes.iter_mut()
+                   .rev()
+                   .filter(|loop_scope| loop_scope.extent == label)
+                   .next()
+            .unwrap_or_else(|| span_bug!(span, "no enclosing loop scope found?"))
     }
 
     /// Given a span and the current visibility scope, make a SourceInfo.

src/librustc_mir/hair/cx/expr.rs

@@ -606,13 +606,13 @@ 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, ref value) => {
             ExprKind::Break {
-                label: label.map(|label| cx.tcx.region_maps.node_extent(label.loop_id)),
+                label: cx.tcx.region_maps.node_extent(label.loop_id),
                 value: value.to_ref(),
             }
         }
         hir::ExprAgain(label) => {
             ExprKind::Continue {
-                label: label.map(|label| cx.tcx.region_maps.node_extent(label.loop_id)),
+                label: cx.tcx.region_maps.node_extent(label.loop_id),
             }
         }
         hir::ExprMatch(ref discr, ref arms, _) => {

src/librustc_mir/hair/mod.rs

@@ -205,11 +205,11 @@ pub enum ExprKind<'tcx> {
         arg: ExprRef<'tcx>,
     },
     Break {
-        label: Option<CodeExtent>,
+        label: CodeExtent,
         value: Option<ExprRef<'tcx>>,
     },
     Continue {
-        label: Option<CodeExtent>,
+        label: CodeExtent,
     },
     Return {
         value: Option<ExprRef<'tcx>>,

src/librustc_passes/loops.rs

@@ -88,22 +88,15 @@ fn visit_expr(&mut self, e: &'hir hir::Expr) {
             }
             hir::ExprBreak(label, ref opt_expr) => {
                 if opt_expr.is_some() {
-                    let loop_kind = if let Some(label) = label {
-                        if label.loop_id == ast::DUMMY_NODE_ID {
-                            None
-                        } else {
-                            Some(match self.hir_map.expect_expr(label.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 if let Loop(kind) = self.cx {
-                        Some(kind)
-                    } else {
-                        // `break` outside a loop - caught below
+                    let loop_kind = if label.loop_id == ast::DUMMY_NODE_ID {
                         None
+                    } else {
+                        Some(match self.hir_map.expect_expr(label.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),
+                        })
                     };
                     match loop_kind {
                         None | Some(LoopKind::Loop(hir::LoopSource::Loop)) => (),

src/librustc_typeck/check/mod.rs

@@ -425,15 +425,11 @@ pub struct EnclosingLoops<'gcx, 'tcx> {
 }
 
 impl<'gcx, 'tcx> EnclosingLoops<'gcx, 'tcx> {
-    fn find_loop(&mut self, id: Option<ast::NodeId>) -> 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
-            }
+    fn find_loop(&mut self, id: ast::NodeId) -> Option<&mut LoopCtxt<'gcx, 'tcx>> {
+        if let Some(ix) = self.by_id.get(&id).cloned() {
+            Some(&mut self.stack[ix])
         } else {
-            self.stack.last_mut()
+            None
         }
     }
 }
@@ -3596,7 +3592,7 @@ fn check_expr_kind(&self,
               tcx.mk_nil()
           }
           hir::ExprBreak(label, ref expr_opt) => {
-            let loop_id = label.map(|l| l.loop_id);
+            let loop_id = label.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)

src/test/run-pass/loop-break-value.rs

@@ -122,12 +122,4 @@ pub fn main() {
         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);
 }