Auto merge of #39864 - cramertj:normalize-breaks, r=nikomatsakis

Normalize labeled and unlabeled breaks

Part of #39849.

src/librustc/cfg/construct.rs

@@ -220,15 +220,24 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
                 // Note that `break` and `continue` statements
                 // may cause additional edges.
 
-                // Is the condition considered part of the loop?
                 let loopback = self.add_dummy_node(&[pred]);              // 1
-                let cond_exit = self.expr(&cond, loopback);             // 2
-                let expr_exit = self.add_ast_node(expr.id, &[cond_exit]); // 3
+
+                // Create expr_exit without pred (cond_exit)
+                let expr_exit = self.add_ast_node(expr.id, &[]);         // 3
+
+                // The LoopScope needs to be on the loop_scopes stack while evaluating the
+                // condition and the body of the loop (both can break out of the loop)
                 self.loop_scopes.push(LoopScope {
                     loop_id: expr.id,
                     continue_index: loopback,
                     break_index: expr_exit
                 });
+
+                let cond_exit = self.expr(&cond, loopback);             // 2
+
+                // Add pred (cond_exit) to expr_exit
+                self.add_contained_edge(cond_exit, expr_exit);
+
                 let body_exit = self.block(&body, cond_exit);          // 4
                 self.add_contained_edge(body_exit, loopback);            // 5
                 self.loop_scopes.pop();
@@ -294,17 +303,17 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
                 self.add_unreachable_node()
             }
 
-            hir::ExprBreak(label, ref opt_expr) => {
+            hir::ExprBreak(destination, ref opt_expr) => {
                 let v = self.opt_expr(opt_expr, pred);
-                let loop_scope = self.find_scope(expr, label);
+                let loop_scope = self.find_scope(expr, destination);
                 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()
             }
 
-            hir::ExprAgain(label) => {
-                let loop_scope = self.find_scope(expr, label);
+            hir::ExprAgain(destination) => {
+                let loop_scope = self.find_scope(expr, destination);
                 let a = self.add_ast_node(expr.id, &[pred]);
                 self.add_exiting_edge(expr, a,
                                       loop_scope, loop_scope.continue_index);
@@ -579,17 +588,18 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
 
     fn find_scope(&self,
                   expr: &hir::Expr,
-                  label: Option<hir::Label>) -> LoopScope {
-        match label {
-            None => *self.loop_scopes.last().unwrap(),
-            Some(label) => {
+                  destination: hir::Destination) -> LoopScope {
+
+        match destination.loop_id.into() {
+            Ok(loop_id) => {
                 for l in &self.loop_scopes {
-                    if l.loop_id == label.loop_id {
+                    if l.loop_id == loop_id {
                         return *l;
                     }
                 }
-                span_bug!(expr.span, "no loop scope for id {}", label.loop_id);
+                span_bug!(expr.span, "no loop scope for id {}", loop_id);
             }
+            Err(err) => span_bug!(expr.span, "loop scope error: {}",  err)
         }
     }
 }

src/librustc/hir/intravisit.rs

@@ -1006,18 +1006,22 @@ 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| {
+                if let Ok(loop_id) = label.loop_id.into() {
+                    visitor.visit_def_mention(Def::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| {
+                if let Ok(loop_id) = label.loop_id.into() {
+                    visitor.visit_def_mention(Def::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 util::nodemap::{DefIdMap, NodeMap, FxHashMap};
 
 use std::collections::BTreeMap;
 use std::iter;
+use std::mem;
 
 use syntax::attr;
 use syntax::ast::*;
@@ -79,6 +80,9 @@ pub struct LoweringContext<'a> {
     impl_items: BTreeMap<hir::ImplItemId, hir::ImplItem>,
     bodies: FxHashMap<hir::BodyId, hir::Body>,
 
+    loop_scopes: Vec<NodeId>,
+    is_in_loop_condition: bool,
+
     type_def_lifetime_params: DefIdMap<usize>,
 }
 
@@ -112,6 +116,8 @@ pub fn lower_crate(sess: &Session,
         trait_items: BTreeMap::new(),
         impl_items: BTreeMap::new(),
         bodies: FxHashMap(),
+        loop_scopes: Vec::new(),
+        is_in_loop_condition: false,
         type_def_lifetime_params: DefIdMap(),
     }.lower_crate(krate)
 }
@@ -244,6 +250,55 @@ impl<'a> LoweringContext<'a> {
         span
     }
 
+    fn with_loop_scope<T, F>(&mut self, loop_id: NodeId, f: F) -> T
+        where F: FnOnce(&mut LoweringContext) -> T
+    {
+        // We're no longer in the base loop's condition; we're in another loop.
+        let was_in_loop_condition = self.is_in_loop_condition;
+        self.is_in_loop_condition = false;
+
+        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();
+
+        self.is_in_loop_condition = was_in_loop_condition;
+
+        result
+    }
+
+    fn with_loop_condition_scope<T, F>(&mut self, f: F) -> T
+        where F: FnOnce(&mut LoweringContext) -> T
+    {
+        let was_in_loop_condition = self.is_in_loop_condition;
+        self.is_in_loop_condition = true;
+
+        let result = f(self);
+
+        self.is_in_loop_condition = was_in_loop_condition;
+
+        result
+    }
+
+    fn with_new_loop_scopes<T, F>(&mut self, f: F) -> T
+        where F: FnOnce(&mut LoweringContext) -> T
+    {
+        let was_in_loop_condition = self.is_in_loop_condition;
+        self.is_in_loop_condition = false;
+
+        let loop_scopes = mem::replace(&mut self.loop_scopes, Vec::new());
+        let result = f(self);
+        mem::replace(&mut self.loop_scopes, loop_scopes);
+
+        self.is_in_loop_condition = was_in_loop_condition;
+
+        result
+    }
+
     fn with_parent_def<T, F>(&mut self, parent_id: NodeId, f: F) -> T
         where F: FnOnce(&mut LoweringContext) -> T
     {
@@ -271,17 +326,24 @@ impl<'a> LoweringContext<'a> {
         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,
-                loop_id: match self.expect_full_def(id) {
-                    Def::Label(loop_id) => loop_id,
-                    _ => DUMMY_NODE_ID
+    fn lower_destination(&mut self, destination: Option<(NodeId, Spanned<Ident>)>)
+        -> hir::Destination
+    {
+        match destination {
+            Some((id, label_ident)) => hir::Destination {
+                ident: Some(label_ident),
+                loop_id: if let Def::Label(loop_id) = self.expect_full_def(id) {
+                    hir::LoopIdResult::Ok(loop_id)
+                } else {
+                    hir::LoopIdResult::Err(hir::LoopIdError::UnresolvedLabel)
                 }
+            },
+            None => hir::Destination {
+                ident: None,
+                loop_id: self.loop_scopes.last().map(|innermost_loop_id| Ok(*innermost_loop_id))
+                            .unwrap_or(Err(hir::LoopIdError::OutsideLoopScope)).into()
             }
-        })
+        }
     }
 
     fn lower_attrs(&mut self, attrs: &Vec<Attribute>) -> hir::HirVec<Attribute> {
@@ -992,15 +1054,17 @@ impl<'a> LoweringContext<'a> {
                                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,26 +1626,32 @@ impl<'a> LoweringContext<'a> {
                     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(
+                            this.with_loop_condition_scope(|this| 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)),
                                    arms.iter().map(|x| self.lower_arm(x)).collect(),
                                    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 +1730,29 @@ impl<'a> LoweringContext<'a> {
                     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),
-                                   opt_expr.as_ref().map(|x| P(self.lower_expr(x))))
+                    let label_result = if self.is_in_loop_condition && opt_ident.is_none() {
+                        hir::Destination {
+                            ident: opt_ident,
+                            loop_id: Err(hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
+                        }
+                    } else {
+                        self.lower_destination(opt_ident.map(|ident| (e.id, ident)))
+                    };
+                    hir::ExprBreak(
+                            label_result,
+                            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(
+                        if self.is_in_loop_condition && opt_ident.is_none() {
+                            hir::Destination {
+                                ident: opt_ident,
+                                loop_id: Err(
+                                    hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
+                            }
+                        } else {
+                            self.lower_destination(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 +1893,16 @@ impl<'a> LoweringContext<'a> {
                     //     }
                     //   }
 
+                    // Note that the block AND the condition are evaluated in the loop scope.
+                    // This is done to allow `break` from inside the condition of the loop.
+                    let (body, break_expr, sub_expr) = self.with_loop_scope(e.id, |this| (
+                        this.lower_block(body),
+                        this.expr_break(e.span, ThinVec::new()),
+                        this.with_loop_condition_scope(|this| 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 +1911,11 @@ impl<'a> LoweringContext<'a> {
                     // `_ => 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 +1957,7 @@ impl<'a> LoweringContext<'a> {
 
                     // `::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 +1967,8 @@ impl<'a> LoweringContext<'a> {
 
                     // `::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 +2246,8 @@ impl<'a> LoweringContext<'a> {
     }
 
     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_destination(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>)