refactor the targeted_by_break field

In master, this field was an arbitrary node-id (in fact, an id for
something that doesn't even exist in the HIR -- the `catch` node).
Breaks targeting this block used that id. In the newer system, this
field is a boolean, and any breaks targeted this block will use the
id of the block.

Author: nikomatsakis

src/librustc/cfg/construct.rs

@@ -74,11 +74,11 @@ pub fn construct<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
 impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
     fn block(&mut self, blk: &hir::Block, pred: CFGIndex) -> CFGIndex {
-        if let Some(break_to_expr_id) = blk.break_to_expr_id {
+        if blk.targeted_by_break {
             let expr_exit = self.add_ast_node(blk.id, &[]);
 
             self.breakable_block_scopes.push(BlockScope {
-                block_expr_id: break_to_expr_id,
+                block_expr_id: blk.id,
                 break_index: expr_exit,
             });
 

src/librustc/hir/lowering.rs

@@ -1156,7 +1156,7 @@ impl<'a> LoweringContext<'a> {
         bounds.iter().map(|bound| self.lower_ty_param_bound(bound)).collect()
     }
 
-    fn lower_block(&mut self, b: &Block, break_to: Option<NodeId>) -> P<hir::Block> {
+    fn lower_block(&mut self, b: &Block, targeted_by_break: bool) -> P<hir::Block> {
         let mut expr = None;
 
         let mut stmts = vec![];
@@ -1179,7 +1179,7 @@ impl<'a> LoweringContext<'a> {
             expr: expr,
             rules: self.lower_block_check_mode(&b.rules),
             span: b.span,
-            break_to_expr_id: break_to,
+            targeted_by_break: targeted_by_break,
         })
     }
 
@@ -1274,7 +1274,7 @@ impl<'a> LoweringContext<'a> {
             }
             ItemKind::Fn(ref decl, unsafety, constness, abi, ref generics, ref body) => {
                 self.with_new_scopes(|this| {
-                    let body = this.lower_block(body, None);
+                    let body = this.lower_block(body, false);
                     let body = this.expr_block(body, ThinVec::new());
                     let body_id = this.record_body(body, Some(decl));
                     hir::ItemFn(this.lower_fn_decl(decl),
@@ -1368,7 +1368,7 @@ impl<'a> LoweringContext<'a> {
                                                    hir::TraitMethod::Required(names))
                     }
                     TraitItemKind::Method(ref sig, Some(ref body)) => {
-                        let body = this.lower_block(body, None);
+                        let body = this.lower_block(body, false);
                         let expr = this.expr_block(body, ThinVec::new());
                         let body_id = this.record_body(expr, Some(&sig.decl));
                         hir::TraitItemKind::Method(this.lower_method_sig(sig),
@@ -1424,7 +1424,7 @@ impl<'a> LoweringContext<'a> {
                         hir::ImplItemKind::Const(this.lower_ty(ty), body_id)
                     }
                     ImplItemKind::Method(ref sig, ref body) => {
-                        let body = this.lower_block(body, None);
+                        let body = this.lower_block(body, false);
                         let expr = this.expr_block(body, ThinVec::new());
                         let body_id = this.record_body(expr, Some(&sig.decl));
                         hir::ImplItemKind::Method(this.lower_method_sig(sig), body_id)
@@ -1848,15 +1848,15 @@ impl<'a> LoweringContext<'a> {
                                 id: id,
                                 rules: hir::DefaultBlock,
                                 span: span,
-                                break_to_expr_id: None,
+                                targeted_by_break: false,
                             });
                             P(self.expr_block(blk, ThinVec::new()))
                         }
                         _ => P(self.lower_expr(els)),
                     }
                 });
 
-                let then_blk = self.lower_block(blk, None);
+                let then_blk = self.lower_block(blk, false);
                 let then_expr = self.expr_block(then_blk, ThinVec::new());
 
                 hir::ExprIf(P(self.lower_expr(cond)), P(then_expr), else_opt)
@@ -1865,18 +1865,18 @@ impl<'a> LoweringContext<'a> {
                 self.with_loop_scope(e.id, |this|
                     hir::ExprWhile(
                         this.with_loop_condition_scope(|this| P(this.lower_expr(cond))),
-                        this.lower_block(body, None),
+                        this.lower_block(body, false),
                         this.lower_opt_sp_ident(opt_ident)))
             }
             ExprKind::Loop(ref body, opt_ident) => {
                 self.with_loop_scope(e.id, |this|
-                    hir::ExprLoop(this.lower_block(body, None),
+                    hir::ExprLoop(this.lower_block(body, false),
                                   this.lower_opt_sp_ident(opt_ident),
                                   hir::LoopSource::Loop))
             }
             ExprKind::Catch(ref body) => {
-                self.with_catch_scope(e.id, |this|
-                    hir::ExprBlock(this.lower_block(body, Some(e.id))))
+                self.with_catch_scope(body.id, |this|
+                    hir::ExprBlock(this.lower_block(body, true)))
             }
             ExprKind::Match(ref expr, ref arms) => {
                 hir::ExprMatch(P(self.lower_expr(expr)),
@@ -1894,7 +1894,7 @@ impl<'a> LoweringContext<'a> {
                     })
                 })
             }
-            ExprKind::Block(ref blk) => hir::ExprBlock(self.lower_block(blk, None)),
+            ExprKind::Block(ref blk) => hir::ExprBlock(self.lower_block(blk, false)),
             ExprKind::Assign(ref el, ref er) => {
                 hir::ExprAssign(P(self.lower_expr(el)), P(self.lower_expr(er)))
             }
@@ -2040,7 +2040,7 @@ impl<'a> LoweringContext<'a> {
 
                 // `<pat> => <body>`
                 {
-                    let body = self.lower_block(body, None);
+                    let body = self.lower_block(body, false);
                     let body_expr = P(self.expr_block(body, ThinVec::new()));
                     let pat = self.lower_pat(pat);
                     arms.push(self.arm(hir_vec![pat], body_expr));
@@ -2112,7 +2112,7 @@ impl<'a> LoweringContext<'a> {
                             let (guard, body) = if let ExprKind::If(ref cond,
                                                                     ref then,
                                                                     _) = else_expr.node {
-                                let then = self.lower_block(then, None);
+                                let then = self.lower_block(then, false);
                                 (Some(cond),
                                  self.expr_block(then, ThinVec::new()))
                             } else {
@@ -2162,7 +2162,7 @@ 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, None),
+                    this.lower_block(body, false),
                     this.expr_break(e.span, ThinVec::new()),
                     this.with_loop_condition_scope(|this| P(this.lower_expr(sub_expr))),
                 ));
@@ -2223,7 +2223,7 @@ impl<'a> LoweringContext<'a> {
                 // `::std::option::Option::Some(<pat>) => <body>`
                 let pat_arm = {
                     let body_block = self.with_loop_scope(e.id,
-                                                          |this| this.lower_block(body, None));
+                                                          |this| this.lower_block(body, false));
                     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);
@@ -2655,7 +2655,7 @@ impl<'a> LoweringContext<'a> {
             id: self.next_id(),
             rules: hir::DefaultBlock,
             span: span,
-            break_to_expr_id: None,
+            targeted_by_break: false,
         }
     }
 
@@ -2763,7 +2763,7 @@ impl<'a> LoweringContext<'a> {
             id: id,
             stmts: stmts,
             expr: Some(expr),
-            break_to_expr_id: None,
+            targeted_by_break: false,
         });
         self.expr_block(block, attrs)
     }

src/librustc/hir/mod.rs

@@ -549,9 +549,11 @@ pub struct Block {
     /// Distinguishes between `unsafe { ... }` and `{ ... }`
     pub rules: BlockCheckMode,
     pub span: Span,
-    /// The id of the expression that `break` breaks to if the block can be broken out of.
-    /// Currently only `Some(_)` for `catch {}` blocks
-    pub break_to_expr_id: Option<NodeId>,
+    /// If true, then there may exist `break 'a` values that aim to
+    /// break out of this block early. As of this writing, this is not
+    /// currently permitted in Rust itself, but it is generated as
+    /// part of `catch` statements.
+    pub targeted_by_break: bool,
 }
 
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash)]

src/librustc/middle/liveness.rs

@@ -821,8 +821,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
 
     fn propagate_through_block(&mut self, blk: &hir::Block, succ: LiveNode)
                                -> LiveNode {
-        if let Some(break_to_expr_id) = blk.break_to_expr_id {
-            self.breakable_block_ln.insert(break_to_expr_id, succ);
+        if blk.targeted_by_break {
+            self.breakable_block_ln.insert(blk.id, succ);
         }
         let succ = self.propagate_through_opt_expr(blk.expr.as_ref().map(|e| &**e), succ);
         blk.stmts.iter().rev().fold(succ, |succ, stmt| {

src/librustc_mir/build/block.rs

@@ -12,15 +12,42 @@ use build::{BlockAnd, BlockAndExtension, Builder};
 use hair::*;
 use rustc::mir::*;
 use rustc::hir;
+use syntax_pos::Span;
 
 impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     pub fn ast_block(&mut self,
                      destination: &Lvalue<'tcx>,
-                     mut block: BasicBlock,
-                     ast_block: &'tcx hir::Block)
+                     block: BasicBlock,
+                     ast_block: &'tcx hir::Block,
+                     source_info: SourceInfo)
                      -> BlockAnd<()> {
-        let Block { extent, span, stmts, expr } = self.hir.mirror(ast_block);
+        let Block { extent, span, stmts, expr, targeted_by_break } = self.hir.mirror(ast_block);
         self.in_scope(extent, block, move |this| {
+            if targeted_by_break {
+                // This is a `break`-able block (currently only `catch { ... }`)
+                let exit_block = this.cfg.start_new_block();
+                let block_exit = this.in_breakable_scope(None, exit_block,
+                                                         destination.clone(), |this| {
+                    this.ast_block_stmts(destination, block, span, stmts, expr)
+                });
+                this.cfg.terminate(unpack!(block_exit), source_info,
+                                   TerminatorKind::Goto { target: exit_block });
+                exit_block.unit()
+            } else {
+                this.ast_block_stmts(destination, block, span, stmts, expr)
+            }
+        })
+    }
+
+    fn ast_block_stmts(&mut self,
+                       destination: &Lvalue<'tcx>,
+                       mut block: BasicBlock,
+                       span: Span,
+                       stmts: Vec<StmtRef<'tcx>>,
+                       expr: Option<ExprRef<'tcx>>)
+                       -> BlockAnd<()> {
+        let this = self;
+
         // This convoluted structure is to avoid using recursion as we walk down a list
         // of statements. Basically, the structure we get back is something like:
         //
@@ -96,6 +123,5 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
         // Restore the original visibility scope.
         this.visibility_scope = outer_visibility_scope;
         block.unit()
-        })
     }
 }

src/librustc_mir/build/expr/into.rs

@@ -40,19 +40,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 this.in_scope(extent, block, |this| this.into(destination, block, value))
             }
             ExprKind::Block { body: ast_block } => {
-                if let Some(_) = ast_block.break_to_expr_id {
-                    // This is a `break`-able block (currently only `catch { ... }`)
-                    let exit_block = this.cfg.start_new_block();
-                    let block_exit = this.in_breakable_scope(None, exit_block,
-                                                             destination.clone(), |this| {
-                        this.ast_block(destination, block, ast_block)
-                    });
-                    this.cfg.terminate(unpack!(block_exit), source_info,
-                                       TerminatorKind::Goto { target: exit_block });
-                    exit_block.unit()
-                } else {
-                    this.ast_block(destination, block, ast_block)
-                }
+                this.ast_block(destination, block, ast_block, source_info)
             }
             ExprKind::Match { discriminant, arms } => {
                 this.match_expr(destination, expr_span, block, discriminant, arms)

src/librustc_mir/hair/cx/block.rs

@@ -23,6 +23,7 @@ impl<'tcx> Mirror<'tcx> for &'tcx hir::Block {
         // in order to get the lexical scoping correctly.
         let stmts = mirror_stmts(cx, self.id, &*self.stmts);
         Block {
+            targeted_by_break: self.targeted_by_break,
             extent: cx.tcx.region_maps.node_extent(self.id),
             span: self.span,
             stmts: stmts,

src/librustc_mir/hair/mod.rs

@@ -31,6 +31,7 @@ pub use rustc_const_eval::pattern::{BindingMode, Pattern, PatternKind, FieldPatt
 
 #[derive(Clone, Debug)]
 pub struct Block<'tcx> {
+    pub targeted_by_break: bool,
     pub extent: CodeExtent,
     pub span: Span,
     pub stmts: Vec<StmtRef<'tcx>>,

src/librustc_typeck/check/mod.rs

@@ -416,15 +416,11 @@ pub struct EnclosingBreakables<'gcx, 'tcx> {
 }
 
 impl<'gcx, 'tcx> EnclosingBreakables<'gcx, 'tcx> {
-    fn find_breakable(&mut self, target: hir::ScopeTarget)
-        -> Option<&mut BreakableCtxt<'gcx, 'tcx>>
-    {
-        let opt_index = target.opt_id().and_then(|id| self.by_id.get(&id).cloned());
-        if let Some(ix) = opt_index {
-            Some(&mut self.stack[ix])
-        } else {
-            None
-        }
+    fn find_breakable(&mut self, target_id: ast::NodeId) -> &mut BreakableCtxt<'gcx, 'tcx> {
+        let ix = *self.by_id.get(&target_id).unwrap_or_else(|| {
+            bug!("could not find enclosing breakable with id {}", target_id);
+        });
+        &mut self.stack[ix]
     }
 }
 
@@ -3472,12 +3468,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
               tcx.mk_nil()
           }
           hir::ExprBreak(destination, ref expr_opt) => {
+            if let Some(target_id) = destination.target_id.opt_id() {
                 let coerce_to = {
                     let mut enclosing_breakables = self.enclosing_breakables.borrow_mut();
-                enclosing_breakables
-                    .find_breakable(destination.target_id).map(|ctxt| ctxt.coerce_to)
+                    enclosing_breakables.find_breakable(target_id).coerce_to
                 };
-            if let Some(coerce_to) = coerce_to {
+
                 let e_ty;
                 let cause;
                 if let Some(ref e) = *expr_opt {
@@ -3492,7 +3488,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 }
 
                 let mut enclosing_breakables = self.enclosing_breakables.borrow_mut();
-                let ctxt = enclosing_breakables.find_breakable(destination.target_id).unwrap();
+                let ctxt = enclosing_breakables.find_breakable(target_id);
 
                 let result = if let Some(ref e) = *expr_opt {
                     // Special-case the first element, as it has no "previous expressions".
@@ -4024,7 +4020,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             replace(&mut *fcx_ps, unsafety_state)
         };
 
-        let mut ty = if let Some(break_to_expr_id) = blk.break_to_expr_id {
+        let mut ty = if blk.targeted_by_break {
             let unified = self.next_ty_var(TypeVariableOrigin::TypeInference(blk.span));
             let coerce_to = expected.only_has_type(self).unwrap_or(unified);
             let ctxt = BreakableCtxt {
@@ -4034,15 +4030,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 may_break: false,
             };
 
-            let (mut ctxt, (e_ty, cause)) = self.with_breakable_ctxt(break_to_expr_id, ctxt, || {
+            let (mut ctxt, (e_ty, cause)) = self.with_breakable_ctxt(blk.id, ctxt, || {
                 for s in &blk.stmts {
                     self.check_stmt(s);
                 }
                 let coerce_to = {
                     let mut enclosing_breakables = self.enclosing_breakables.borrow_mut();
-                    enclosing_breakables.find_breakable(
-                        hir::ScopeTarget::Block(break_to_expr_id)
-                    ).unwrap().coerce_to
+                    enclosing_breakables.find_breakable(blk.id).coerce_to
                 };
                 let e_ty;
                 let cause;