Add a with_cond method

Factors out the common pattern across the several places that do
arithmetic checks

Author: Aatch

src/librustc_mir/build/block.rs

@@ -12,6 +12,7 @@ use build::{BlockAnd, BlockAndExtension, Builder};
 use hair::*;
 use rustc::mir::repr::*;
 use rustc::hir;
+use syntax::codemap::Span;
 
 impl<'a,'tcx> Builder<'a,'tcx> {
     pub fn ast_block(&mut self,
@@ -79,4 +80,31 @@ impl<'a,'tcx> Builder<'a,'tcx> {
             block.unit()
         })
     }
+
+    // Helper method for generating MIR inside a conditional block.
+    pub fn with_cond<F>(&mut self, block: BasicBlock, span: Span,
+                        cond: Operand<'tcx>, f: F) -> BasicBlock
+    where F: FnOnce(&mut Builder<'a, 'tcx>, BasicBlock) -> BasicBlock {
+        let scope_id = self.innermost_scope_id();
+
+        let then_block = self.cfg.start_new_block();
+        let else_block = self.cfg.start_new_block();
+
+        self.cfg.terminate(block, scope_id, span,
+                           TerminatorKind::If {
+                               cond: cond,
+                               targets: (then_block, else_block)
+                           });
+
+        let after = f(self, then_block);
+
+        // If the returned block isn't terminated, add a branch to the "else"
+        // block
+        if !self.cfg.terminated(after) {
+            self.cfg.terminate(after, scope_id, span,
+                               TerminatorKind::Goto { target: else_block });
+        }
+
+        else_block
+    }
 }

src/librustc_mir/build/cfg.rs

@@ -86,12 +86,17 @@ impl<'tcx> CFG<'tcx> {
                      scope: ScopeId,
                      span: Span,
                      kind: TerminatorKind<'tcx>) {
-        debug_assert!(self.block_data(block).terminator.is_none(),
+        debug_assert!(!self.terminated(block),
                       "terminate: block {:?} already has a terminator set", block);
         self.block_data_mut(block).terminator = Some(Terminator {
             span: span,
             scope: scope,
             kind: kind,
         });
     }
+
+    /// Returns whether or not the given block has been terminated or not
+    pub fn terminated(&self, block: BasicBlock) -> bool {
+        self.block_data(block).terminator.is_some()
+    }
 }

src/librustc_mir/build/expr/as_rvalue.rs

@@ -88,18 +88,11 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                     this.cfg.push_assign(block, scope_id, expr_span, &is_min,
                                          Rvalue::BinaryOp(BinOp::Eq, arg.clone(), minval));
 
-                    let of_block = this.cfg.start_new_block();
-                    let ok_block = this.cfg.start_new_block();
-
-                    this.cfg.terminate(block, scope_id, expr_span,
-                                       TerminatorKind::If {
-                                           cond: Operand::Consume(is_min),
-                                           targets: (of_block, ok_block)
-                                       });
-
-                    this.panic(of_block, "attempted to negate with overflow", expr_span);
-
-                    block = ok_block;
+                    block = this.with_cond(
+                        block, expr_span, Operand::Consume(is_min), |this, block| {
+                            this.panic(block, "attempted to negate with overflow", expr_span);
+                            block
+                        });
                 }
                 block.and(Rvalue::UnaryOp(op, arg))
             }
@@ -271,21 +264,18 @@ impl<'a,'tcx> Builder<'a,'tcx> {
             let val = result_value.clone().field(val_fld, ty);
             let of = result_value.field(of_fld, bool_ty);
 
-            let success = self.cfg.start_new_block();
-            let failure = self.cfg.start_new_block();
-
-            self.cfg.terminate(block, scope_id, span,
-                               TerminatorKind::If {
-                                   cond: Operand::Consume(of),
-                                   targets: (failure, success)
-                               });
             let msg = if op == BinOp::Shl || op == BinOp::Shr {
                 "shift operation overflowed"
             } else {
                 "arithmetic operation overflowed"
             };
-            self.panic(failure, msg, span);
-            success.and(Rvalue::Use(Operand::Consume(val)))
+
+            block = self.with_cond(block, span, Operand::Consume(of), |this, block| {
+                this.panic(block, msg, span);
+                block
+            });
+
+            block.and(Rvalue::Use(Operand::Consume(val)))
         } else {
             if ty.is_integral() && (op == BinOp::Div || op == BinOp::Rem) {
                 // Checking division and remainder is more complex, since we 1. always check
@@ -305,17 +295,10 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                 self.cfg.push_assign(block, scope_id, span, &is_zero,
                                      Rvalue::BinaryOp(BinOp::Eq, rhs.clone(), zero));
 
-                let zero_block = self.cfg.start_new_block();
-                let ok_block   = self.cfg.start_new_block();
-
-                self.cfg.terminate(block, scope_id, span,
-                                   TerminatorKind::If {
-                                       cond: Operand::Consume(is_zero),
-                                       targets: (zero_block, ok_block)
-                                   });
-
-                self.panic(zero_block, zero_msg, span);
-                block = ok_block;
+                block = self.with_cond(block, span, Operand::Consume(is_zero), |this, block| {
+                    this.panic(block, zero_msg, span);
+                    block
+                });
 
                 // We only need to check for the overflow in one case:
                 // MIN / -1, and only for signed values.
@@ -339,18 +322,10 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                     self.cfg.push_assign(block, scope_id, span, &of,
                                          Rvalue::BinaryOp(BinOp::BitAnd, is_neg_1, is_min));
 
-                    let of_block = self.cfg.start_new_block();
-                    let ok_block = self.cfg.start_new_block();
-
-                    self.cfg.terminate(block, scope_id, span,
-                                       TerminatorKind::If {
-                                           cond: Operand::Consume(of),
-                                           targets: (of_block, ok_block)
-                                       });
-
-                    self.panic(of_block, overflow_msg, span);
-
-                    block = ok_block;
+                    block = self.with_cond(block, span, Operand::Consume(of), |this, block| {
+                        this.panic(block, overflow_msg, span);
+                        block
+                    });
                 }
             }