Skip to content

Commit 2e51e09

Browse files
committed
Auto merge of #24500 - pnkfelix:oflo-checked-neg, r=<try>
Add conditional overflow-checking to signed negate operator. I argue this can land independently of #24420 , because one can write the implementation of `wrapped_neg()` inline if necessary (as illustrated in two cases on this PR). This needs to go into beta channel.
2 parents ac2b6f6 + f9c6780 commit 2e51e09

File tree

4 files changed

+46
-20
lines changed

4 files changed

+46
-20
lines changed

Diff for: src/libcore/num/mod.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1261,7 +1261,12 @@ macro_rules! int_impl {
12611261
#[stable(feature = "rust1", since = "1.0.0")]
12621262
#[inline]
12631263
pub fn abs(self) -> $T {
1264-
if self.is_negative() { -self } else { self }
1264+
if self.is_negative() {
1265+
// FIXME (#24420) use wrapping_neg instead after it lands.
1266+
(!self).wrapping_add(1)
1267+
} else {
1268+
self
1269+
}
12651270
}
12661271

12671272
/// Returns a number representing sign of `self`.

Diff for: src/librustc_trans/trans/base.rs

+20-15
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,25 @@ fn cast_shift_rhs<F, G>(op: ast::BinOp_,
566566
}
567567
}
568568

569+
pub fn llty_and_min_for_signed_ty<'blk, 'tcx>(cx: Block<'blk, 'tcx>,
570+
val_t: Ty<'tcx>) -> (Type, u64) {
571+
match val_t.sty {
572+
ty::ty_int(t) => {
573+
let llty = Type::int_from_ty(cx.ccx(), t);
574+
let min = match t {
575+
ast::TyIs if llty == Type::i32(cx.ccx()) => i32::MIN as u64,
576+
ast::TyIs => i64::MIN as u64,
577+
ast::TyI8 => i8::MIN as u64,
578+
ast::TyI16 => i16::MIN as u64,
579+
ast::TyI32 => i32::MIN as u64,
580+
ast::TyI64 => i64::MIN as u64,
581+
};
582+
(llty, min)
583+
}
584+
_ => unreachable!(),
585+
}
586+
}
587+
569588
pub fn fail_if_zero_or_overflows<'blk, 'tcx>(
570589
cx: Block<'blk, 'tcx>,
571590
call_info: NodeIdAndSpan,
@@ -620,21 +639,7 @@ pub fn fail_if_zero_or_overflows<'blk, 'tcx>(
620639
// signed division/remainder which would trigger overflow. For unsigned
621640
// integers, no action beyond checking for zero need be taken.
622641
if is_signed {
623-
let (llty, min) = match rhs_t.sty {
624-
ty::ty_int(t) => {
625-
let llty = Type::int_from_ty(cx.ccx(), t);
626-
let min = match t {
627-
ast::TyIs if llty == Type::i32(cx.ccx()) => i32::MIN as u64,
628-
ast::TyIs => i64::MIN as u64,
629-
ast::TyI8 => i8::MIN as u64,
630-
ast::TyI16 => i16::MIN as u64,
631-
ast::TyI32 => i32::MIN as u64,
632-
ast::TyI64 => i64::MIN as u64,
633-
};
634-
(llty, min)
635-
}
636-
_ => unreachable!(),
637-
};
642+
let (llty, min) = llty_and_min_for_signed_ty(cx, rhs_t);
638643
let minus_one = ICmp(bcx, llvm::IntEQ, rhs,
639644
C_integral(llty, !0, false), debug_loc);
640645
with_cond(bcx, minus_one, |bcx| {

Diff for: src/librustc_trans/trans/expr.rs

+18-3
Original file line numberDiff line numberDiff line change
@@ -1530,11 +1530,26 @@ fn trans_unary<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
15301530
ast::UnNeg => {
15311531
let datum = unpack_datum!(bcx, trans(bcx, sub_expr));
15321532
let val = datum.to_llscalarish(bcx);
1533-
let llneg = {
1533+
let (bcx, llneg) = {
15341534
if ty::type_is_fp(un_ty) {
1535-
FNeg(bcx, val, debug_loc)
1535+
let result = FNeg(bcx, val, debug_loc);
1536+
(bcx, result)
15361537
} else {
1537-
Neg(bcx, val, debug_loc)
1538+
let is_signed = ty::type_is_signed(un_ty);
1539+
let result = Neg(bcx, val, debug_loc);
1540+
let bcx = if bcx.ccx().check_overflow() && is_signed {
1541+
let (llty, min) = base::llty_and_min_for_signed_ty(bcx, un_ty);
1542+
let is_min = ICmp(bcx, llvm::IntEQ, val,
1543+
C_integral(llty, min, true), debug_loc);
1544+
with_cond(bcx, is_min, |bcx| {
1545+
let msg = InternedString::new(
1546+
"attempted to negate with overflow");
1547+
controlflow::trans_fail(bcx, expr_info(expr), msg)
1548+
})
1549+
} else {
1550+
bcx
1551+
};
1552+
(bcx, result)
15381553
}
15391554
};
15401555
immediate_rvalue_bcx(bcx, llneg, un_ty).to_expr_datumblock()

Diff for: src/libserialize/json.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1540,7 +1540,8 @@ impl<T: Iterator<Item=char>> Parser<T> {
15401540
F64Value(res)
15411541
} else {
15421542
if neg {
1543-
let res = -(res as i64);
1543+
// FIXME (#24420) use wrapping_neg instead after it lands.
1544+
let res = !(res as i64).wrapping_add(1);
15441545

15451546
// Make sure we didn't underflow.
15461547
if res > 0 {

0 commit comments

Comments
 (0)