Skip to content

Commit

Permalink
Add a with_cond method
Browse files Browse the repository at this point in the history
Factors out the common pattern across the several places that do
arithmetic checks
  • Loading branch information
Aatch authored and eddyb committed Jun 5, 2016
1 parent 73f3054 commit f2c983b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 46 deletions.
28 changes: 28 additions & 0 deletions src/librustc_mir/build/block.rs
Expand Up @@ -12,6 +12,7 @@ use build::{BlockAnd, BlockAndExtension, Builder};
use hair::*;
use rustc::mir::repr::*;
use rustc::hir;
use syntax::codemap::Span;

impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
pub fn ast_block(&mut self,
Expand Down Expand Up @@ -81,4 +82,31 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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, 'gcx, '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
}
}
7 changes: 6 additions & 1 deletion src/librustc_mir/build/cfg.rs
Expand Up @@ -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()
}
}
65 changes: 20 additions & 45 deletions src/librustc_mir/build/expr/as_rvalue.rs
Expand Up @@ -88,18 +88,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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))
}
Expand Down Expand Up @@ -268,21 +261,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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
Expand All @@ -302,17 +292,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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.
Expand All @@ -336,18 +319,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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
});
}
}

Expand Down

0 comments on commit f2c983b

Please sign in to comment.