From dee5024227b069ca787ae0f606a628bcfa94f90b Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sun, 22 Feb 2015 16:18:32 +0100 Subject: [PATCH 01/26] Avoid ICE when `FnCtxt::local_ty` cannot find type; issue error then, return `ty_err`. --- src/librustc_typeck/check/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fd6ba79ec21bb..cd72fd85cb73b 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1363,10 +1363,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match self.inh.locals.borrow().get(&nid) { Some(&t) => t, None => { - self.tcx().sess.span_bug( + self.tcx().sess.span_err( span, - &format!("no type for local variable {}", - nid)); + &format!("no type for local variable {}", nid)); + self.tcx().types.err } } } From 00ccc7af1eb9091de7f24edf9eacb3da119d5b27 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 19 Feb 2015 12:41:10 +0100 Subject: [PATCH 02/26] Make `test/run-pass/backtrace.rs` more robust about own host environment. Namely, I have been annoyed in the past when I have done `RUST_BACKTRACE=1 make check` only to discover (again) that such a trick causes this test to fail, because it assumes that the `RUST_BACKTRACE` environment variable is not set. --- src/test/run-pass/backtrace.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/run-pass/backtrace.rs b/src/test/run-pass/backtrace.rs index 6f76322cb778d..879b3e920ab96 100644 --- a/src/test/run-pass/backtrace.rs +++ b/src/test/run-pass/backtrace.rs @@ -53,7 +53,9 @@ fn runtest(me: &str) { "bad output: {}", s); // Make sure the stack trace is *not* printed - let p = template.clone().arg("fail").spawn().unwrap(); + // (Remove RUST_BACKTRACE from our own environment, in case developer + // is running `make check` with it on.) + let p = template.clone().arg("fail").env_remove("RUST_BACKTRACE").spawn().unwrap(); let out = p.wait_with_output().unwrap(); assert!(!out.status.success()); let s = str::from_utf8(&out.error).unwrap(); From cdfff9db35d037c51dfd5c2bac2174f651294adb Mon Sep 17 00:00:00 2001 From: Corey Richardson Date: Tue, 6 Jan 2015 00:56:30 -0500 Subject: [PATCH 03/26] rustc: implement arithmetic overflow checking Adds overflow checking to integer addition, multiplication, and subtraction when `-Z force-overflow-checks` is true, or if `--cfg ndebug` is not passed to the compiler. On overflow, it panics with `arithmetic operation overflowed`. Also adds `overflowing_add`, `overflowing_sub`, and `overflowing_mul` intrinsics for doing unchecked arithmetic. [breaking-change] --- src/libcore/intrinsics.rs | 11 +++ src/librustc/session/config.rs | 3 +- src/librustc_trans/trans/base.rs | 9 +- src/librustc_trans/trans/context.rs | 9 +- src/librustc_trans/trans/expr.rs | 125 +++++++++++++++++++++++++- src/librustc_trans/trans/intrinsic.rs | 5 ++ src/librustc_typeck/check/mod.rs | 3 + src/test/run-fail/overflowing-add.rs | 15 ++++ src/test/run-fail/overflowing-mul.rs | 15 ++++ src/test/run-fail/overflowing-sub.rs | 15 ++++ 10 files changed, 203 insertions(+), 7 deletions(-) create mode 100644 src/test/run-fail/overflowing-add.rs create mode 100644 src/test/run-fail/overflowing-mul.rs create mode 100644 src/test/run-fail/overflowing-sub.rs diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 1ca243134cc6c..ed12913609125 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -546,3 +546,14 @@ extern "rust-intrinsic" { /// Performs checked `u64` multiplication. pub fn u64_mul_with_overflow(x: u64, y: u64) -> (u64, bool); } + +// SNAP 880fb89 +#[cfg(not(stage0))] +extern "rust-intrinsic" { + /// Returns (a + b) mod 2^N, where N is the width of N in bits. + pub fn overflowing_add(a: T, b: T) -> T; + /// Returns (a - b) mod 2^N, where N is the width of N in bits. + pub fn overflowing_sub(a: T, b: T) -> T; + /// Returns (a * b) mod 2^N, where N is the width of N in bits. + pub fn overflowing_mul(a: T, b: T) -> T; +} diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index efcde8b2fa1ac..53c1f4e4a4089 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -259,7 +259,6 @@ pub enum CrateType { CrateTypeStaticlib, } - #[derive(Clone)] pub enum Passes { SomePasses(Vec), @@ -585,6 +584,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "Adds unstable command line options to rustc interface"), print_enum_sizes: bool = (false, parse_bool, "Print the size of enums and their variants"), + force_overflow_checks: Option = (None, parse_opt_bool, + "Force overflow checks on or off"), } pub fn default_lib_output() -> CrateType { diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 7a6960d379026..149dad476545e 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -3102,6 +3102,12 @@ pub fn trans_crate<'tcx>(analysis: ty::CrateAnalysis<'tcx>) let ty::CrateAnalysis { ty_cx: tcx, export_map, reachable, name, .. } = analysis; let krate = tcx.map.krate(); + let check_overflow = if let Some(v) = tcx.sess.opts.debugging_opts.force_overflow_checks { + v + } else { + !attr::contains_name(&krate.config, "ndebug") + }; + // Before we touch LLVM, make sure that multithreading is enabled. unsafe { use std::sync::{Once, ONCE_INIT}; @@ -3129,7 +3135,8 @@ pub fn trans_crate<'tcx>(analysis: ty::CrateAnalysis<'tcx>) export_map, Sha256::new(), link_meta.clone(), - reachable); + reachable, + check_overflow); { let ccx = shared_ccx.get_ccx(0); diff --git a/src/librustc_trans/trans/context.rs b/src/librustc_trans/trans/context.rs index 3586a9dda2067..9777398bddcc3 100644 --- a/src/librustc_trans/trans/context.rs +++ b/src/librustc_trans/trans/context.rs @@ -69,6 +69,7 @@ pub struct SharedCrateContext<'tcx> { symbol_hasher: RefCell, tcx: ty::ctxt<'tcx>, stats: Stats, + check_overflow: bool, available_monomorphizations: RefCell>, available_drop_glues: RefCell, String>>, @@ -245,7 +246,8 @@ impl<'tcx> SharedCrateContext<'tcx> { export_map: ExportMap, symbol_hasher: Sha256, link_meta: LinkMeta, - reachable: NodeSet) + reachable: NodeSet, + check_overflow: bool) -> SharedCrateContext<'tcx> { let (metadata_llcx, metadata_llmod) = unsafe { create_context_and_module(&tcx.sess, "metadata") @@ -274,6 +276,7 @@ impl<'tcx> SharedCrateContext<'tcx> { llvm_insns: RefCell::new(FnvHashMap()), fn_stats: RefCell::new(Vec::new()), }, + check_overflow: check_overflow, available_monomorphizations: RefCell::new(FnvHashSet()), available_drop_glues: RefCell::new(FnvHashMap()), }; @@ -743,6 +746,10 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &format!("the type `{}` is too big for the current architecture", obj.repr(self.tcx()))) } + + pub fn check_overflow(&self) -> bool { + self.shared.check_overflow + } } fn declare_intrinsic(ccx: &CrateContext, key: & &'static str) -> Option { diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 27f9b9506a58a..60455119d5872 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -82,6 +82,7 @@ use trans::machine::{llsize_of, llsize_of_alloc}; use trans::type_::Type; use syntax::{ast, ast_util, codemap}; +use syntax::parse::token::InternedString; use syntax::ptr::P; use syntax::parse::token; use std::iter::repeat; @@ -1709,8 +1710,8 @@ fn trans_eager_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, }; let is_float = ty::type_is_fp(intype); let is_signed = ty::type_is_signed(intype); - let rhs = base::cast_shift_expr_rhs(bcx, op, lhs, rhs); + let info = expr_info(binop_expr); let binop_debug_loc = binop_expr.debug_loc(); @@ -1720,21 +1721,30 @@ fn trans_eager_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, if is_float { FAdd(bcx, lhs, rhs, binop_debug_loc) } else { - Add(bcx, lhs, rhs, binop_debug_loc) + let (newbcx, res) = with_overflow_check( + bcx, OverflowOp::Add, info, lhs_t, lhs, rhs, binop_debug_loc); + bcx = newbcx; + res } } ast::BiSub => { if is_float { FSub(bcx, lhs, rhs, binop_debug_loc) } else { - Sub(bcx, lhs, rhs, binop_debug_loc) + let (newbcx, res) = with_overflow_check( + bcx, OverflowOp::Sub, info, lhs_t, lhs, rhs, binop_debug_loc); + bcx = newbcx; + res } } ast::BiMul => { if is_float { FMul(bcx, lhs, rhs, binop_debug_loc) } else { - Mul(bcx, lhs, rhs, binop_debug_loc) + let (newbcx, res) = with_overflow_check( + bcx, OverflowOp::Mul, info, lhs_t, lhs, rhs, binop_debug_loc); + bcx = newbcx; + res } } ast::BiDiv => { @@ -2314,3 +2324,110 @@ fn deref_once<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, DatumBlock { bcx: bcx, datum: datum } } } + +enum OverflowOp { + Add, + Sub, + Mul, +} + +impl OverflowOp { + fn to_intrinsic_name(&self, tcx: &ty::ctxt, ty: Ty) -> &'static str { + use syntax::ast::IntTy::*; + use syntax::ast::UintTy::*; + use middle::ty::{ty_int, ty_uint}; + + let new_sty = match ty.sty { + ty_int(TyIs(_)) => match &tcx.sess.target.target.target_pointer_width[..] { + "32" => ty_int(TyI32), + "64" => ty_int(TyI64), + _ => panic!("unsupported target word size") + }, + ty_uint(TyUs(_)) => match &tcx.sess.target.target.target_pointer_width[..] { + "32" => ty_uint(TyU32), + "64" => ty_uint(TyU64), + _ => panic!("unsupported target word size") + }, + ref t @ ty_uint(_) | ref t @ ty_int(_) => t.clone(), + _ => panic!("tried to get overflow intrinsic for non-int type") + }; + + match *self { + OverflowOp::Add => match new_sty { + ty_int(TyI8) => "llvm.sadd.with.overflow.i8", + ty_int(TyI16) => "llvm.sadd.with.overflow.i16", + ty_int(TyI32) => "llvm.sadd.with.overflow.i32", + ty_int(TyI64) => "llvm.sadd.with.overflow.i64", + + ty_uint(TyU8) => "llvm.uadd.with.overflow.i8", + ty_uint(TyU16) => "llvm.uadd.with.overflow.i16", + ty_uint(TyU32) => "llvm.uadd.with.overflow.i32", + ty_uint(TyU64) => "llvm.uadd.with.overflow.i64", + + _ => unreachable!(), + }, + OverflowOp::Sub => match new_sty { + ty_int(TyI8) => "llvm.ssub.with.overflow.i8", + ty_int(TyI16) => "llvm.ssub.with.overflow.i16", + ty_int(TyI32) => "llvm.ssub.with.overflow.i32", + ty_int(TyI64) => "llvm.ssub.with.overflow.i64", + + ty_uint(TyU8) => "llvm.usub.with.overflow.i8", + ty_uint(TyU16) => "llvm.usub.with.overflow.i16", + ty_uint(TyU32) => "llvm.usub.with.overflow.i32", + ty_uint(TyU64) => "llvm.usub.with.overflow.i64", + + _ => unreachable!(), + }, + OverflowOp::Mul => match new_sty { + ty_int(TyI8) => "llvm.smul.with.overflow.i8", + ty_int(TyI16) => "llvm.smul.with.overflow.i16", + ty_int(TyI32) => "llvm.smul.with.overflow.i32", + ty_int(TyI64) => "llvm.smul.with.overflow.i64", + + ty_uint(TyU8) => "llvm.umul.with.overflow.i8", + ty_uint(TyU16) => "llvm.umul.with.overflow.i16", + ty_uint(TyU32) => "llvm.umul.with.overflow.i32", + ty_uint(TyU64) => "llvm.umul.with.overflow.i64", + + _ => unreachable!(), + }, + } + } +} + + +fn with_overflow_check<'a, 'b>(bcx: Block<'a, 'b>, oop: OverflowOp, info: NodeIdAndSpan, + lhs_t: Ty, lhs: ValueRef, rhs: ValueRef, binop_debug_loc: DebugLoc) + -> (Block<'a, 'b>, ValueRef) { + if bcx.unreachable.get() { return (bcx, _Undef(lhs)); } + if bcx.ccx().check_overflow() { + let name = oop.to_intrinsic_name(bcx.tcx(), lhs_t); + let llfn = bcx.ccx().get_intrinsic(&name); + + let val = Call(bcx, llfn, &[lhs, rhs], None, binop_debug_loc); + let result = ExtractValue(bcx, val, 0); // iN operation result + let overflow = ExtractValue(bcx, val, 1); // i1 "did it overflow?" + + let cond = ICmp(bcx, llvm::IntEQ, overflow, C_integral(Type::i1(bcx.ccx()), 1, false), + binop_debug_loc); + + let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1"); + Call(bcx, expect, &[cond, C_integral(Type::i1(bcx.ccx()), 0, false)], + None, binop_debug_loc); + + let bcx = + base::with_cond(bcx, cond, |bcx| + controlflow::trans_fail(bcx, info, + InternedString::new("arithmetic operation overflowed"))); + + (bcx, result) + } else { + let res = match oop { + OverflowOp::Add => Add(bcx, lhs, rhs, binop_debug_loc), + OverflowOp::Sub => Sub(bcx, lhs, rhs, binop_debug_loc), + OverflowOp::Mul => Mul(bcx, lhs, rhs, binop_debug_loc), + }; + (bcx, res) + } +} diff --git a/src/librustc_trans/trans/intrinsic.rs b/src/librustc_trans/trans/intrinsic.rs index 54644c92869cf..916492195c258 100644 --- a/src/librustc_trans/trans/intrinsic.rs +++ b/src/librustc_trans/trans/intrinsic.rs @@ -660,6 +660,11 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, llargs[0], llargs[1], call_debug_location), + + (_, "overflowing_add") => Add(bcx, llargs[0], llargs[1], call_debug_location), + (_, "overflowing_sub") => Sub(bcx, llargs[0], llargs[1], call_debug_location), + (_, "overflowing_mul") => Mul(bcx, llargs[0], llargs[1], call_debug_location), + (_, "return_address") => { if !fcx.caller_expects_out_pointer { tcx.sess.span_err(call_info.span, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index cd72fd85cb73b..273aadc383c69 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5491,6 +5491,9 @@ pub fn check_intrinsic_type(ccx: &CrateCtxt, it: &ast::ForeignItem) { (0, vec!(tcx.types.u64, tcx.types.u64), ty::mk_tup(tcx, vec!(tcx.types.u64, tcx.types.bool))), + "overflowing_add" | "overflowing_sub" | "overflowing_mul" => + (1, vec![param(ccx, 0), param(ccx, 0)], param(ccx, 0)), + "return_address" => (0, vec![], ty::mk_imm_ptr(tcx, tcx.types.u8)), "assume" => (0, vec![tcx.types.bool], ty::mk_nil(tcx)), diff --git a/src/test/run-fail/overflowing-add.rs b/src/test/run-fail/overflowing-add.rs new file mode 100644 index 0000000000000..c3e41110d20da --- /dev/null +++ b/src/test/run-fail/overflowing-add.rs @@ -0,0 +1,15 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// error-pattern:thread '
' panicked at 'arithmatic operation overflowed' + +fn main() { + let x = 200u8 + 200u8 + 200u8; +} diff --git a/src/test/run-fail/overflowing-mul.rs b/src/test/run-fail/overflowing-mul.rs new file mode 100644 index 0000000000000..bf7a9d0758651 --- /dev/null +++ b/src/test/run-fail/overflowing-mul.rs @@ -0,0 +1,15 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// error-pattern:thread '
' panicked at 'arithmatic operation overflowed' + +fn main() { + let x = 200u8 + 4u8; +} diff --git a/src/test/run-fail/overflowing-sub.rs b/src/test/run-fail/overflowing-sub.rs new file mode 100644 index 0000000000000..961b36d322cdb --- /dev/null +++ b/src/test/run-fail/overflowing-sub.rs @@ -0,0 +1,15 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// error-pattern:thread '
' panicked at 'arithmatic operation overflowed' + +fn main() { + let x = 42u8 - 43u8; +} From 1246d4067fdc034d064dfb78f88c2c3c079c3f4f Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 9 Jan 2015 16:10:57 +1300 Subject: [PATCH 04/26] Add `core::num::wrapping` and fix overflow errors. Many of the core rust libraries have places that rely on integer wrapping behaviour. These places have been altered to use the wrapping_* methods: * core::hash::sip - A number of macros * core::str - The `maximal_suffix` method in `TwoWaySearcher` * rustc::util::nodemap - Implementation of FnvHash * rustc_back::sha2 - A number of macros and other places * rand::isaac - Isaac64Rng, changed to use the Wrapping helper type Some places had "benign" underflow. This is when underflow or overflow occurs, but the unspecified value is not used due to other conditions. * collections::bit::Bitv - underflow when `self.nbits` is zero. * collections::hash::{map,table} - Underflow when searching an empty table. Did cause undefined behaviour in this case due to an out-of-bounds ptr::offset based on the underflowed index. However the resulting pointers would never be read from. * syntax::ext::deriving::encodable - Underflow when calculating the index of the last field in a variant with no fields. These cases were altered to avoid the underflow, often by moving the underflowing operation to a place where underflow could not happen. There was one case that relied on the fact that unsigned arithmetic and two's complement arithmetic are identical with wrapping semantics. This was changed to use the wrapping_* methods. Finally, the calculation of variant discriminants could overflow if the preceeding discriminant was `U64_MAX`. The logic in `rustc::middle::ty` for this was altered to avoid the overflow completely, while the remaining places were changed to use wrapping methods. This is because `rustc::middle::ty::enum_variants` now throws an error when the calculated discriminant value overflows a `u64`. This behaviour can be triggered by the following code: ``` enum Foo { A = U64_MAX, B } ``` This commit also implements the remaining integer operators for Wrapped. --- src/libcollections/bit.rs | 6 +- src/libcore/hash/sip.rs | 12 +- src/libcore/num/mod.rs | 3 + src/libcore/num/wrapping.rs | 153 ++++++++++++++++++++++++ src/libcore/str/mod.rs | 9 +- src/librand/distributions/range.rs | 3 +- src/librand/isaac.rs | 74 ++++++------ src/librustc/metadata/decoder.rs | 2 +- src/librustc/metadata/encoder.rs | 2 +- src/librustc/middle/astencode.rs | 4 +- src/librustc/middle/ty.rs | 21 +++- src/librustc/util/nodemap.rs | 2 +- src/librustc_back/sha2.rs | 28 +++-- src/librustc_typeck/check/mod.rs | 9 +- src/libstd/collections/hash/map.rs | 7 ++ src/libstd/collections/hash/table.rs | 6 +- src/libstd/num/mod.rs | 1 + src/libstd/prelude/v1.rs | 2 + src/libsyntax/ext/deriving/encodable.rs | 37 +++--- 19 files changed, 286 insertions(+), 95 deletions(-) create mode 100644 src/libcore/num/wrapping.rs diff --git a/src/libcollections/bit.rs b/src/libcollections/bit.rs index a92eccce142b1..05bf5f8e42cef 100644 --- a/src/libcollections/bit.rs +++ b/src/libcollections/bit.rs @@ -818,11 +818,11 @@ impl BitVec { let full_value = if value { !0 } else { 0 }; // Correct the old tail word, setting or clearing formerly unused bits - let old_last_word = blocks_for_bits(self.nbits) - 1; + let num_cur_blocks = blocks_for_bits(self.nbits); if self.nbits % u32::BITS as usize > 0 { let mask = mask_for_bits(self.nbits); if value { - self.storage[old_last_word] |= !mask; + self.storage[num_cur_blocks - 1] |= !mask; } else { // Extra bits are already zero by invariant. } @@ -830,7 +830,7 @@ impl BitVec { // Fill in words after the old tail word let stop_idx = cmp::min(self.storage.len(), new_nblocks); - for idx in old_last_word + 1..stop_idx { + for idx in num_cur_blocks..stop_idx { self.storage[idx] = full_value; } diff --git a/src/libcore/hash/sip.rs b/src/libcore/hash/sip.rs index 39bcbacdff182..df0008c500b8e 100644 --- a/src/libcore/hash/sip.rs +++ b/src/libcore/hash/sip.rs @@ -14,7 +14,7 @@ use prelude::*; use default::Default; - +use num::wrapping::WrappingOps; use super::Hasher; /// An implementation of SipHash 2-4. @@ -71,17 +71,17 @@ macro_rules! u8to64_le { macro_rules! rotl { ($x:expr, $b:expr) => - (($x << $b) | ($x >> (64 - $b))) + (($x << $b) | ($x >> (64.wrapping_sub($b)))) } macro_rules! compress { ($v0:expr, $v1:expr, $v2:expr, $v3:expr) => ({ - $v0 += $v1; $v1 = rotl!($v1, 13); $v1 ^= $v0; + $v0 = $v0.wrapping_add($v1); $v1 = rotl!($v1, 13); $v1 ^= $v0; $v0 = rotl!($v0, 32); - $v2 += $v3; $v3 = rotl!($v3, 16); $v3 ^= $v2; - $v0 += $v3; $v3 = rotl!($v3, 21); $v3 ^= $v0; - $v2 += $v1; $v1 = rotl!($v1, 17); $v1 ^= $v2; + $v2 = $v2.wrapping_add($v3); $v3 = rotl!($v3, 16); $v3 ^= $v2; + $v0 = $v0.wrapping_add($v3); $v3 = rotl!($v3, 21); $v3 ^= $v0; + $v2 = $v2.wrapping_add($v1); $v1 = rotl!($v1, 17); $v1 ^= $v2; $v2 = rotl!($v2, 32); }) } diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index d77a1eb4203c6..3ed059520b12f 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -30,6 +30,9 @@ use option::Option::{self, Some, None}; use result::Result::{self, Ok, Err}; use str::{FromStr, StrExt}; +#[unstable(feature = "core", reason = "may be removed or relocated")] +pub mod wrapping; + /// A built-in signed or unsigned integer. #[stable(feature = "rust1", since = "1.0.0")] pub trait Int diff --git a/src/libcore/num/wrapping.rs b/src/libcore/num/wrapping.rs new file mode 100644 index 0000000000000..30478a8f09f30 --- /dev/null +++ b/src/libcore/num/wrapping.rs @@ -0,0 +1,153 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +#![allow(missing_docs)] + +use ops::*; + +#[cfg(not(stage0))] +use intrinsics::{overflowing_add, overflowing_sub, overflowing_mul}; + +pub trait WrappingOps { + fn wrapping_add(self, rhs: Self) -> Self; + fn wrapping_sub(self, rhs: Self) -> Self; + fn wrapping_mul(self, rhs: Self) -> Self; +} + +#[cfg(not(stage0))] +macro_rules! wrapping_impl { + ($($t:ty)*) => ($( + impl WrappingOps for $t { + #[inline(always)] + fn wrapping_add(self, rhs: $t) -> $t { + unsafe { + overflowing_add(self, rhs) + } + } + #[inline(always)] + fn wrapping_sub(self, rhs: $t) -> $t { + unsafe { + overflowing_sub(self, rhs) + } + } + #[inline(always)] + fn wrapping_mul(self, rhs: $t) -> $t { + unsafe { + overflowing_mul(self, rhs) + } + } + } + )*) +} + +#[cfg(stage0)] +macro_rules! wrapping_impl { + ($($t:ty)*) => ($( + impl WrappingOps for $t { + #[inline(always)] + fn wrapping_add(self, rhs: $t) -> $t { + self + rhs + } + #[inline(always)] + fn wrapping_sub(self, rhs: $t) -> $t { + self - rhs + } + #[inline(always)] + fn wrapping_mul(self, rhs: $t) -> $t { + self * rhs + } + } + )*) +} + +wrapping_impl! { uint u8 u16 u32 u64 int i8 i16 i32 i64 } + +#[unstable(feature = "core", reason = "may be removed, renamed, or relocated")] +#[derive(PartialEq,Eq,PartialOrd,Ord,Clone,Copy)] +pub struct Wrapping(pub T); + +impl Add for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn add(self, other: Wrapping) -> Wrapping { + Wrapping(self.0.wrapping_add(other.0)) + } +} + +impl Sub for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn sub(self, other: Wrapping) -> Wrapping { + Wrapping(self.0.wrapping_sub(other.0)) + } +} + +impl Mul for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn mul(self, other: Wrapping) -> Wrapping { + Wrapping(self.0.wrapping_mul(other.0)) + } +} + +impl> Not for Wrapping { + type Output = Wrapping; + + fn not(self) -> Wrapping { + Wrapping(!self.0) + } +} + +impl> BitXor for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn bitxor(self, other: Wrapping) -> Wrapping { + Wrapping(self.0 ^ other.0) + } +} + +impl> BitOr for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn bitor(self, other: Wrapping) -> Wrapping { + Wrapping(self.0 | other.0) + } +} + +impl> BitAnd for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn bitand(self, other: Wrapping) -> Wrapping { + Wrapping(self.0 & other.0) + } +} + +impl> Shl for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn shl(self, other: uint) -> Wrapping { + Wrapping(self.0 << other) + } +} + +impl> Shr for Wrapping { + type Output = Wrapping; + + #[inline(always)] + fn shr(self, other: uint) -> Wrapping { + Wrapping(self.0 >> other) + } +} diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index b354116993c23..a7a0690640921 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -830,6 +830,7 @@ impl TwoWaySearcher { #[inline] #[allow(dead_code)] fn maximal_suffix(arr: &[u8], reversed: bool) -> (usize, usize) { + use num::wrapping::WrappingOps; let mut left = -1; // Corresponds to i in the paper let mut right = 0; // Corresponds to j in the paper let mut offset = 1; // Corresponds to k in the paper @@ -839,17 +840,17 @@ impl TwoWaySearcher { let a; let b; if reversed { - a = arr[left + offset]; + a = arr[left.wrapping_add(offset)]; b = arr[right + offset]; } else { a = arr[right + offset]; - b = arr[left + offset]; + b = arr[left.wrapping_add(offset)]; } if a < b { // Suffix is smaller, period is entire prefix so far. right += offset; offset = 1; - period = right - left; + period = right.wrapping_sub(left); } else if a == b { // Advance through repetition of the current period. if offset == period { @@ -866,7 +867,7 @@ impl TwoWaySearcher { period = 1; } } - (left + 1, period) + (left.wrapping_add(1), period) } } diff --git a/src/librand/distributions/range.rs b/src/librand/distributions/range.rs index 6eb1d68a081aa..04c67b16a7c59 100644 --- a/src/librand/distributions/range.rs +++ b/src/librand/distributions/range.rs @@ -14,6 +14,7 @@ use core::prelude::{PartialOrd}; use core::num::Int; +use core::num::wrapping::WrappingOps; use Rng; use distributions::{Sample, IndependentSample}; @@ -97,7 +98,7 @@ macro_rules! integer_impl { // bijection. fn construct_range(low: $ty, high: $ty) -> Range<$ty> { - let range = high as $unsigned - low as $unsigned; + let range = (high as $unsigned).wrapping_sub(low as $unsigned); let unsigned_max: $unsigned = Int::max_value(); // this is the largest number that fits into $unsigned diff --git a/src/librand/isaac.rs b/src/librand/isaac.rs index 701749ff3443f..63eb8417655ee 100644 --- a/src/librand/isaac.rs +++ b/src/librand/isaac.rs @@ -304,7 +304,7 @@ impl Isaac64Rng { fn init(&mut self, use_rsl: bool) { macro_rules! init { ($var:ident) => ( - let mut $var = 0x9e3779b97f4a7c13; + let mut $var = Wrapping(0x9e3779b97f4a7c13); ) } init!(a); init!(b); init!(c); init!(d); @@ -312,14 +312,14 @@ impl Isaac64Rng { macro_rules! mix { () => {{ - a-=e; f^=h>>9; h+=a; - b-=f; g^=a<<9; a+=b; - c-=g; h^=b>>23; b+=c; - d-=h; a^=c<<15; c+=d; - e-=a; b^=d>>14; d+=e; - f-=b; c^=e<<20; e+=f; - g-=c; d^=f>>17; f+=g; - h-=d; e^=g<<14; g+=h; + a=a-e; f=f^h>>9; h=h+a; + b=b-f; g=g^a<<9; a=a+b; + c=c-g; h=h^b>>23; b=b+c; + d=d-h; a=a^c<<15; c=c+d; + e=e-a; b=b^d>>14; d=d+e; + f=f-b; c=c^e<<20; e=e+f; + g=g-c; d=d^f>>17; f=f+g; + h=h-d; e=e^g<<14; g=g+h; }} } @@ -331,15 +331,15 @@ impl Isaac64Rng { macro_rules! memloop { ($arr:expr) => {{ for i in (0..RAND_SIZE_64 / 8).map(|i| i * 8) { - a+=$arr[i ]; b+=$arr[i+1]; - c+=$arr[i+2]; d+=$arr[i+3]; - e+=$arr[i+4]; f+=$arr[i+5]; - g+=$arr[i+6]; h+=$arr[i+7]; + a=a+Wrapping($arr[i ]); b=b+Wrapping($arr[i+1]); + c=c+Wrapping($arr[i+2]); d=d+Wrapping($arr[i+3]); + e=e+Wrapping($arr[i+4]); f=f+Wrapping($arr[i+5]); + g=g+Wrapping($arr[i+6]); h=h+Wrapping($arr[i+7]); mix!(); - self.mem[i ]=a; self.mem[i+1]=b; - self.mem[i+2]=c; self.mem[i+3]=d; - self.mem[i+4]=e; self.mem[i+5]=f; - self.mem[i+6]=g; self.mem[i+7]=h; + self.mem[i ]=a.0; self.mem[i+1]=b.0; + self.mem[i+2]=c.0; self.mem[i+3]=d.0; + self.mem[i+4]=e.0; self.mem[i+5]=f.0; + self.mem[i+6]=g.0; self.mem[i+7]=h.0; } }} } @@ -349,10 +349,10 @@ impl Isaac64Rng { } else { for i in (0..RAND_SIZE_64 / 8).map(|i| i * 8) { mix!(); - self.mem[i ]=a; self.mem[i+1]=b; - self.mem[i+2]=c; self.mem[i+3]=d; - self.mem[i+4]=e; self.mem[i+5]=f; - self.mem[i+6]=g; self.mem[i+7]=h; + self.mem[i ]=a.0; self.mem[i+1]=b.0; + self.mem[i+2]=c.0; self.mem[i+3]=d.0; + self.mem[i+4]=e.0; self.mem[i+5]=f.0; + self.mem[i+6]=g.0; self.mem[i+7]=h.0; } } @@ -363,8 +363,8 @@ impl Isaac64Rng { fn isaac64(&mut self) { self.c += 1; // abbreviations - let mut a = self.a; - let mut b = self.b + self.c; + let mut a = Wrapping(self.a); + let mut b = Wrapping(self.b) + Wrapping(self.c); const MIDPOINT: uint = RAND_SIZE_64 / 2; const MP_VEC: [(uint, uint); 2] = [(0,MIDPOINT), (MIDPOINT, 0)]; macro_rules! ind { @@ -383,13 +383,13 @@ impl Isaac64Rng { let mix = if $j == 0 {!mix} else {mix}; unsafe { - let x = *self.mem.get_unchecked(base + mr_offset); - a = mix + *self.mem.get_unchecked(base + m2_offset); - let y = ind!(x) + a + b; - *self.mem.get_unchecked_mut(base + mr_offset) = y; + let x = Wrapping(*self.mem.get_unchecked(base + mr_offset)); + a = mix + Wrapping(*self.mem.get_unchecked(base + m2_offset)); + let y = Wrapping(ind!(x.0)) + a + b; + *self.mem.get_unchecked_mut(base + mr_offset) = y.0; - b = ind!(y >> RAND_SIZE_64_LEN) + x; - *self.rsl.get_unchecked_mut(base + mr_offset) = b; + b = Wrapping(ind!(y.0 >> RAND_SIZE_64_LEN)) + x; + *self.rsl.get_unchecked_mut(base + mr_offset) = b.0; } }} } @@ -401,13 +401,13 @@ impl Isaac64Rng { let mix = if $j == 0 {!mix} else {mix}; unsafe { - let x = *self.mem.get_unchecked(base + mr_offset); - a = mix + *self.mem.get_unchecked(base + m2_offset); - let y = ind!(x) + a + b; - *self.mem.get_unchecked_mut(base + mr_offset) = y; + let x = Wrapping(*self.mem.get_unchecked(base + mr_offset)); + a = mix + Wrapping(*self.mem.get_unchecked(base + m2_offset)); + let y = Wrapping(ind!(x.0)) + a + b; + *self.mem.get_unchecked_mut(base + mr_offset) = y.0; - b = ind!(y >> RAND_SIZE_64_LEN) + x; - *self.rsl.get_unchecked_mut(base + mr_offset) = b; + b = Wrapping(ind!(y.0 >> RAND_SIZE_64_LEN)) + x; + *self.rsl.get_unchecked_mut(base + mr_offset) = b.0; } }} } @@ -419,8 +419,8 @@ impl Isaac64Rng { } } - self.a = a; - self.b = b; + self.a = a.0; + self.b = b.0; self.cnt = RAND_SIZE_64; } } diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs index 251c5e6eac70e..e32fcaec04734 100644 --- a/src/librustc/metadata/decoder.rs +++ b/src/librustc/metadata/decoder.rs @@ -783,7 +783,7 @@ pub fn get_enum_variants<'tcx>(intr: Rc, cdata: Cmd, id: ast::Nod _ => { /* empty */ } } let old_disr_val = disr_val; - disr_val += 1; + disr_val = disr_val.wrapping_add(1); Rc::new(ty::VariantInfo { args: arg_tys, arg_names: arg_names, diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index 131a299cc500f..8152a2bf16dd1 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -347,7 +347,7 @@ fn encode_enum_variant_info(ecx: &EncodeContext, ecx.tcx.map.with_path(variant.node.id, |path| encode_path(rbml_w, path)); rbml_w.end_tag(); - disr_val += 1; + disr_val = disr_val.wrapping_add(1); i += 1; } } diff --git a/src/librustc/middle/astencode.rs b/src/librustc/middle/astencode.rs index 599dde4b70155..33c0fb8b031ef 100644 --- a/src/librustc/middle/astencode.rs +++ b/src/librustc/middle/astencode.rs @@ -204,7 +204,9 @@ impl<'a, 'b, 'tcx> DecodeContext<'a, 'b, 'tcx> { pub fn tr_id(&self, id: ast::NodeId) -> ast::NodeId { // from_id_range should be non-empty assert!(!self.from_id_range.empty()); - (id - self.from_id_range.min + self.to_id_range.min) + // Use wrapping arithmetic because otherwise it introduces control flow. + // Maybe we should just have the control flow? -- aatch + (id.wrapping_sub(self.from_id_range.min).wrapping_add(self.to_id_range.min)) } /// Translates an EXTERNAL def-id, converting the crate number from the one used in the encoded diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index aaba840825e69..b6d1fc5a36948 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -5333,6 +5333,7 @@ pub fn type_is_empty(cx: &ctxt, ty: Ty) -> bool { pub fn enum_variants<'tcx>(cx: &ctxt<'tcx>, id: ast::DefId) -> Rc>>> { + use std::num::Int; // For checked_add memoized(&cx.enum_var_cache, id, |id: ast::DefId| { if ast::LOCAL_CRATE != id.krate { Rc::new(csearch::get_enum_variants(cx, id)) @@ -5349,11 +5350,7 @@ pub fn enum_variants<'tcx>(cx: &ctxt<'tcx>, id: ast::DefId) let mut last_discriminant: Option = None; Rc::new(enum_definition.variants.iter().map(|variant| { - let mut discriminant = match last_discriminant { - Some(val) => val + 1, - None => INITIAL_DISCRIMINANT_VALUE - }; - + let mut discriminant = INITIAL_DISCRIMINANT_VALUE; if let Some(ref e) = variant.node.disr_expr { // Preserve all values, and prefer signed. let ty = Some(cx.types.i64); @@ -5373,7 +5370,19 @@ pub fn enum_variants<'tcx>(cx: &ctxt<'tcx>, id: ast::DefId) "expected constant: {}", err); } } - }; + } else { + if let Some(val) = last_discriminant { + if let Some(v) = val.checked_add(1) { + discriminant = v + } else { + cx.sess.span_err( + variant.span, + &format!("Discriminant overflowed!")); + } + } else { + discriminant = INITIAL_DISCRIMINANT_VALUE; + } + } last_discriminant = Some(discriminant); Rc::new(VariantInfo::from_ast_variant(cx, &**variant, diff --git a/src/librustc/util/nodemap.rs b/src/librustc/util/nodemap.rs index b15da7dab3ee6..0f69aa941a31e 100644 --- a/src/librustc/util/nodemap.rs +++ b/src/librustc/util/nodemap.rs @@ -57,7 +57,7 @@ impl Hasher for FnvHasher { let FnvHasher(mut hash) = *self; for byte in bytes { hash = hash ^ (*byte as u64); - hash = hash * 0x100000001b3; + hash = hash.wrapping_mul(0x100000001b3); } *self = FnvHasher(hash); } diff --git a/src/librustc_back/sha2.rs b/src/librustc_back/sha2.rs index 6654a46f7c31b..8acb6851f111d 100644 --- a/src/librustc_back/sha2.rs +++ b/src/librustc_back/sha2.rs @@ -347,17 +347,19 @@ impl Engine256State { // Sha-512 and Sha-256 use basically the same calculations which are implemented // by these macros. Inlining the calculations seems to result in better generated code. macro_rules! schedule_round { ($t:expr) => ( - w[$t] = sigma1(w[$t - 2]) + w[$t - 7] + sigma0(w[$t - 15]) + w[$t - 16]; - ) + w[$t] = sigma1(w[$t - 2]).wrapping_add(w[$t - 7]) + .wrapping_add(sigma0(w[$t - 15])).wrapping_add(w[$t - 16]); + ) } macro_rules! sha2_round { ($A:ident, $B:ident, $C:ident, $D:ident, $E:ident, $F:ident, $G:ident, $H:ident, $K:ident, $t:expr) => ( { - $H += sum1($E) + ch($E, $F, $G) + $K[$t] + w[$t]; - $D += $H; - $H += sum0($A) + maj($A, $B, $C); + $H = $H.wrapping_add(sum1($E)).wrapping_add(ch($E, $F, $G)) + .wrapping_add($K[$t]).wrapping_add(w[$t]); + $D = $D.wrapping_add($H); + $H = $H.wrapping_add(sum0($A)).wrapping_add(maj($A, $B, $C)); } ) } @@ -397,14 +399,14 @@ impl Engine256State { sha2_round!(b, c, d, e, f, g, h, a, K32, t + 7); } - self.h0 += a; - self.h1 += b; - self.h2 += c; - self.h3 += d; - self.h4 += e; - self.h5 += f; - self.h6 += g; - self.h7 += h; + self.h0 = self.h0.wrapping_add(a); + self.h1 = self.h1.wrapping_add(b); + self.h2 = self.h2.wrapping_add(c); + self.h3 = self.h3.wrapping_add(d); + self.h4 = self.h4.wrapping_add(e); + self.h5 = self.h5.wrapping_add(f); + self.h6 = self.h6.wrapping_add(g); + self.h7 = self.h7.wrapping_add(h); } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 273aadc383c69..10771935e22a4 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4554,6 +4554,7 @@ pub fn check_enum_variants<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, id: ast::NodeId, hint: attr::ReprAttr) -> Vec>> { + use std::num::Int; let rty = ty::node_id_to_type(ccx.tcx, id); let mut variants: Vec> = Vec::new(); @@ -4565,7 +4566,13 @@ pub fn check_enum_variants<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, // If the discriminant value is specified explicitly in the enum check whether the // initialization expression is valid, otherwise use the last value plus one. let mut current_disr_val = match prev_disr_val { - Some(prev_disr_val) => prev_disr_val + 1, + Some(prev_disr_val) => { + if let Some(v) = prev_disr_val.checked_add(1) { + v + } else { + ty::INITIAL_DISCRIMINANT_VALUE + } + } None => ty::INITIAL_DISCRIMINANT_VALUE }; diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs index faddbba50590f..8eb29a8327a52 100644 --- a/src/libstd/collections/hash/map.rs +++ b/src/libstd/collections/hash/map.rs @@ -314,6 +314,13 @@ fn search_hashed(table: M, M: Deref>, F: FnMut(&K) -> bool, { + // This is the only function where capacity can be zero. To avoid + // undefined behaviour when Bucket::new gets the raw bucket in this + // case, immediately return the appropriate search result. + if table.capacity() == 0 { + return TableRef(table); + } + let size = table.size(); let mut probe = Bucket::new(table, hash); let ib = probe.index(); diff --git a/src/libstd/collections/hash/table.rs b/src/libstd/collections/hash/table.rs index 4c03d8915eb8a..908b5267b69d6 100644 --- a/src/libstd/collections/hash/table.rs +++ b/src/libstd/collections/hash/table.rs @@ -224,6 +224,9 @@ impl>> Bucket { } pub fn at_index(table: M, ib_index: usize) -> Bucket { + // if capacity is 0, then the RawBucket will be populated with bogus pointers. + // This is an uncommon case though, so avoid it in release builds. + debug_assert!(table.capacity() > 0, "Table should have capacity at this point"); let ib_index = ib_index & (table.capacity() - 1); Bucket { raw: unsafe { @@ -368,10 +371,11 @@ impl>> FullBucket { /// In the cited blog posts above, this is called the "distance to /// initial bucket", or DIB. Also known as "probe count". pub fn distance(&self) -> usize { + use core::num::wrapping::WrappingOps; // Calculates the distance one has to travel when going from // `hash mod capacity` onwards to `idx mod capacity`, wrapping around // if the destination is not reached before the end of the table. - (self.idx - self.hash().inspect() as usize) & (self.table.capacity() - 1) + (self.idx.wrapping_sub(self.hash().inspect() as usize)) & (self.table.capacity() - 1) } #[inline] diff --git a/src/libstd/num/mod.rs b/src/libstd/num/mod.rs index d776079efaeec..d4428282b148e 100644 --- a/src/libstd/num/mod.rs +++ b/src/libstd/num/mod.rs @@ -30,6 +30,7 @@ pub use core::num::{from_uint, from_u8, from_u16, from_u32, from_u64}; pub use core::num::{from_f32, from_f64}; pub use core::num::{FromStrRadix, from_str_radix}; pub use core::num::{FpCategory, ParseIntError, ParseFloatError}; +pub use core::num::wrapping; use option::Option; diff --git a/src/libstd/prelude/v1.rs b/src/libstd/prelude/v1.rs index dad0ff0a15e29..60e1354482c8f 100644 --- a/src/libstd/prelude/v1.rs +++ b/src/libstd/prelude/v1.rs @@ -58,3 +58,5 @@ #[doc(no_inline)] pub use old_io::{Buffer, Writer, Reader, Seek, BufferPrelude}; // NB: remove when range syntax lands #[doc(no_inline)] pub use iter::range; + +#[doc(no_inline)] pub use num::wrapping::{Wrapping, WrappingOps}; diff --git a/src/libsyntax/ext/deriving/encodable.rs b/src/libsyntax/ext/deriving/encodable.rs index 8038074cee14f..94b7d1b4d8cb0 100644 --- a/src/libsyntax/ext/deriving/encodable.rs +++ b/src/libsyntax/ext/deriving/encodable.rs @@ -240,25 +240,24 @@ fn encodable_substructure(cx: &mut ExtCtxt, trait_span: Span, let encoder = cx.expr_ident(trait_span, blkarg); let emit_variant_arg = cx.ident_of("emit_enum_variant_arg"); let mut stmts = Vec::new(); - let last = fields.len() - 1; - for (i, &FieldInfo { ref self_, span, .. }) in fields.iter().enumerate() { - let enc = cx.expr_method_call(span, self_.clone(), - encode, vec!(blkencoder.clone())); - let lambda = cx.lambda_expr_1(span, enc, blkarg); - let call = cx.expr_method_call(span, blkencoder.clone(), - emit_variant_arg, - vec!(cx.expr_usize(span, i), - lambda)); - let call = if i != last { - cx.expr_try(span, call) - } else { - cx.expr(span, ExprRet(Some(call))) - }; - stmts.push(cx.stmt_expr(call)); - } - - // enums with no fields need to return Ok() - if stmts.len() == 0 { + if fields.len() > 0 { + let last = fields.len() - 1; + for (i, &FieldInfo { ref self_, span, .. }) in fields.iter().enumerate() { + let enc = cx.expr_method_call(span, self_.clone(), + encode, vec!(blkencoder.clone())); + let lambda = cx.lambda_expr_1(span, enc, blkarg); + let call = cx.expr_method_call(span, blkencoder.clone(), + emit_variant_arg, + vec!(cx.expr_usize(span, i), + lambda)); + let call = if i != last { + cx.expr_try(span, call) + } else { + cx.expr(span, ExprRet(Some(call))) + }; + stmts.push(cx.stmt_expr(call)); + } + } else { let ret_ok = cx.expr(trait_span, ExprRet(Some(cx.expr_ok(trait_span, cx.expr_tuple(trait_span, vec![]))))); From 280dea743b5227d0d162217cbb89db881242c94e Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 9 Jan 2015 18:06:45 +1300 Subject: [PATCH 05/26] Implement parse_opt_bool better During my clean-up of rebase errors, I took the opportunity to implement parse_opt_bool so that it isn't identical to parse_bool wrapped in `Some`. parse_opt_bool considers no value to be true, a value of 'y', 'yes' or 'on' to be true and 'n', 'no' or 'off' to be false. All other values are an error. --- src/librustc/session/config.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 53c1f4e4a4089..536caece21f84 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -348,7 +348,8 @@ macro_rules! options { #[allow(non_upper_case_globals, dead_code)] mod $mod_desc { pub const parse_bool: Option<&'static str> = None; - pub const parse_opt_bool: Option<&'static str> = None; + pub const parse_opt_bool: Option<&'static str> = + Some("one of: `y`, `yes`, `on`, `n`, `no`, or `off`"); pub const parse_string: Option<&'static str> = Some("a string"); pub const parse_opt_string: Option<&'static str> = Some("a string"); pub const parse_list: Option<&'static str> = Some("a space-separated list of strings"); @@ -379,7 +380,19 @@ macro_rules! options { fn parse_opt_bool(slot: &mut Option, v: Option<&str>) -> bool { match v { - Some(..) => false, + Some(s) => { + match s { + "n" | "no" | "off" => { + *slot = Some(false); + } + "y" | "yes" | "on" => { + *slot = Some(true); + } + _ => { return false; } + } + + true + }, None => { *slot = Some(true); true } } } From 6d6038a1944394fc5e64048aea6fdbad4f59ee15 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 19 Feb 2015 12:32:27 +0100 Subject: [PATCH 06/26] Added `OverflowingOps` trait to core::num::wrapping. These return the result of the operation *plus* an overflow/underflow bit. This can make it easier to write operations where you want to chain some arithmetic together, but also want to return a flag signalling if overflow every occurred. --- src/libcore/lib.rs | 1 + src/libcore/num/wrapping.rs | 147 ++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+) diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index 7cc963bed358f..94d37cee5b37d 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -69,6 +69,7 @@ #![feature(unboxed_closures)] #![feature(rustc_attrs)] #![feature(optin_builtin_traits)] +#![feature(concat_idents)] #[macro_use] mod macros; diff --git a/src/libcore/num/wrapping.rs b/src/libcore/num/wrapping.rs index 30478a8f09f30..707e41a948be0 100644 --- a/src/libcore/num/wrapping.rs +++ b/src/libcore/num/wrapping.rs @@ -14,12 +14,32 @@ use ops::*; #[cfg(not(stage0))] use intrinsics::{overflowing_add, overflowing_sub, overflowing_mul}; +use intrinsics::{i8_add_with_overflow, u8_add_with_overflow}; +use intrinsics::{i16_add_with_overflow, u16_add_with_overflow}; +use intrinsics::{i32_add_with_overflow, u32_add_with_overflow}; +use intrinsics::{i64_add_with_overflow, u64_add_with_overflow}; +use intrinsics::{i8_sub_with_overflow, u8_sub_with_overflow}; +use intrinsics::{i16_sub_with_overflow, u16_sub_with_overflow}; +use intrinsics::{i32_sub_with_overflow, u32_sub_with_overflow}; +use intrinsics::{i64_sub_with_overflow, u64_sub_with_overflow}; +use intrinsics::{i8_mul_with_overflow, u8_mul_with_overflow}; +use intrinsics::{i16_mul_with_overflow, u16_mul_with_overflow}; +use intrinsics::{i32_mul_with_overflow, u32_mul_with_overflow}; +use intrinsics::{i64_mul_with_overflow, u64_mul_with_overflow}; + pub trait WrappingOps { fn wrapping_add(self, rhs: Self) -> Self; fn wrapping_sub(self, rhs: Self) -> Self; fn wrapping_mul(self, rhs: Self) -> Self; } +#[unstable(feature = "core", reason = "may be removed, renamed, or relocated")] +pub trait OverflowingOps { + fn overflowing_add(self, rhs: Self) -> (Self, bool); + fn overflowing_sub(self, rhs: Self) -> (Self, bool); + fn overflowing_mul(self, rhs: Self) -> (Self, bool); +} + #[cfg(not(stage0))] macro_rules! wrapping_impl { ($($t:ty)*) => ($( @@ -151,3 +171,130 @@ impl> Shr for Wrapping { Wrapping(self.0 >> other) } } + +macro_rules! overflowing_impl { + ($($t:ident)*) => ($( + impl OverflowingOps for $t { + #[inline(always)] + fn overflowing_add(self, rhs: $t) -> ($t, bool) { + unsafe { + concat_idents!($t, _add_with_overflow)(self, rhs) + } + } + #[inline(always)] + fn overflowing_sub(self, rhs: $t) -> ($t, bool) { + unsafe { + concat_idents!($t, _sub_with_overflow)(self, rhs) + } + } + #[inline(always)] + fn overflowing_mul(self, rhs: $t) -> ($t, bool) { + unsafe { + concat_idents!($t, _mul_with_overflow)(self, rhs) + } + } + } + )*) +} + +overflowing_impl! { u8 u16 u32 u64 i8 i16 i32 i64 } + +#[cfg(target_pointer_width = "64")] +impl OverflowingOps for usize { + #[inline(always)] + fn overflowing_add(self, rhs: usize) -> (usize, bool) { + unsafe { + let res = u64_add_with_overflow(self as u64, rhs as u64); + (res.0 as usize, res.1) + } + } + #[inline(always)] + fn overflowing_sub(self, rhs: usize) -> (usize, bool) { + unsafe { + let res = u64_sub_with_overflow(self as u64, rhs as u64); + (res.0 as usize, res.1) + } + } + #[inline(always)] + fn overflowing_mul(self, rhs: usize) -> (usize, bool) { + unsafe { + let res = u64_mul_with_overflow(self as u64, rhs as u64); + (res.0 as usize, res.1) + } + } +} + +#[cfg(target_pointer_width = "32")] +impl OverflowingOps for usize { + #[inline(always)] + fn overflowing_add(self, rhs: usize) -> (usize, bool) { + unsafe { + let res = u32_add_with_overflow(self as u32, rhs as u32); + (res.0 as usize, res.1) + } + } + #[inline(always)] + fn overflowing_sub(self, rhs: usize) -> (usize, bool) { + unsafe { + let res = u32_sub_with_overflow(self as u32, rhs as u32); + (res.0 as usize, res.1) + } + } + #[inline(always)] + fn overflowing_mul(self, rhs: usize) -> (usize, bool) { + unsafe { + let res = u32_mul_with_overflow(self as u32, rhs as u32); + (res.0 as usize, res.1) + } + } +} + +#[cfg(target_pointer_width = "64")] +impl OverflowingOps for isize { + #[inline(always)] + fn overflowing_add(self, rhs: isize) -> (isize, bool) { + unsafe { + let res = i64_add_with_overflow(self as i64, rhs as i64); + (res.0 as isize, res.1) + } + } + #[inline(always)] + fn overflowing_sub(self, rhs: isize) -> (isize, bool) { + unsafe { + let res = i64_sub_with_overflow(self as i64, rhs as i64); + (res.0 as isize, res.1) + } + } + #[inline(always)] + fn overflowing_mul(self, rhs: isize) -> (isize, bool) { + unsafe { + let res = i64_mul_with_overflow(self as i64, rhs as i64); + (res.0 as isize, res.1) + } + } +} + +#[cfg(target_pointer_width = "32")] +impl OverflowingOps for isize { + #[inline(always)] + fn overflowing_add(self, rhs: isize) -> (isize, bool) { + unsafe { + let res = i32_add_with_overflow(self as i32, rhs as i32); + (res.0 as isize, res.1) + } + } + #[inline(always)] + fn overflowing_sub(self, rhs: isize) -> (isize, bool) { + unsafe { + let res = i32_sub_with_overflow(self as i32, rhs as i32); + (res.0 as isize, res.1) + } + } + #[inline(always)] + fn overflowing_mul(self, rhs: isize) -> (isize, bool) { + unsafe { + let res = i32_mul_with_overflow(self as i32, rhs as i32); + (res.0 as isize, res.1) + } + } +} From eadc8a7b45b6897640c44b14e81c2c272d364bb7 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 18 Feb 2015 23:01:48 +0100 Subject: [PATCH 07/26] fix Iter::rposition for new arith-overflow checking. --- src/libcore/iter.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index 518ec05f5b980..edc67fbf8c9c5 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -728,10 +728,11 @@ pub trait IteratorExt: Iterator + Sized { P: FnMut(Self::Item) -> bool, Self: ExactSizeIterator + DoubleEndedIterator { - let mut i = self.len() - 1; + let mut i = self.len(); + while let Some(v) = self.next_back() { if predicate(v) { - return Some(i); + return Some(i - 1); } i -= 1; } From cf18e9c331e697faa11e395f6ccf3b0230af7aae Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 18 Feb 2015 18:56:26 -0500 Subject: [PATCH 08/26] fix test generic-extern-mangle.rs --- src/test/run-pass/generic-extern-mangle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/run-pass/generic-extern-mangle.rs b/src/test/run-pass/generic-extern-mangle.rs index 6e3d19b05d4b1..062ee507864bb 100644 --- a/src/test/run-pass/generic-extern-mangle.rs +++ b/src/test/run-pass/generic-extern-mangle.rs @@ -10,7 +10,7 @@ use std::num::Int; -extern "C" fn foo(a: T, b: T) -> T { a + b } +extern "C" fn foo(a: T, b: T) -> T { a.wrapping_add(b) } fn main() { assert_eq!(99u8, foo(255u8, 100u8)); From e7c986105f8af0b4ec4a91a63fbd1706282d401c Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 19 Feb 2015 08:33:32 +0100 Subject: [PATCH 09/26] Fixes to collections to accommodate arith-overflow changes. * `collections::btree::node`: accommodate (transient) underflow. * `collections::btree::map`: avoid underflow during `fn next` for `BTreeMap::range` methods. * `collections::slice`: note that pnkfelix deliberately used `new_pos_wrapping` only once; the other cases of arithmetic do not over- nor underflow, which is a useful property to leave implicitly checked/documented via the remaining calls to `fn new_pos(..)`. * `collections::vec_deque` applied wrapping ops (somewhat blindly) to two implementation methods, and many tests. * `std::collections::hash::table` : Use `OverflowingOps` trait to track overflow during `calculate_offsets` and `calculate_allocation` functions. --- src/libcollections/btree/map.rs | 4 +- src/libcollections/btree/node.rs | 3 +- src/libcollections/slice.rs | 11 +++-- src/libcollections/vec_deque.rs | 67 +++++++++++++++++----------- src/libstd/collections/hash/table.rs | 54 ++++++++++++---------- 5 files changed, 83 insertions(+), 56 deletions(-) diff --git a/src/libcollections/btree/map.rs b/src/libcollections/btree/map.rs index 8a3a1fcb9f387..1fa592ac477a2 100644 --- a/src/libcollections/btree/map.rs +++ b/src/libcollections/btree/map.rs @@ -25,7 +25,7 @@ use core::fmt::Debug; use core::hash::{Hash, Hasher}; use core::iter::{Map, FromIterator, IntoIterator}; use core::ops::{Index, IndexMut}; -use core::{iter, fmt, mem}; +use core::{iter, fmt, mem, usize}; use Bound::{self, Included, Excluded, Unbounded}; use borrow::Borrow; @@ -1467,7 +1467,7 @@ macro_rules! range_impl { $Range { inner: AbsIter { traversals: traversals, - size: 0, // unused + size: usize::MAX, // unused } } } diff --git a/src/libcollections/btree/node.rs b/src/libcollections/btree/node.rs index 4a80d75575e6d..f2a6910a30252 100644 --- a/src/libcollections/btree/node.rs +++ b/src/libcollections/btree/node.rs @@ -1215,7 +1215,8 @@ impl Node { ptr::copy( self.edges_mut().as_mut_ptr().offset(index as isize), self.edges().as_ptr().offset(index as isize + 1), - self.len() - index + 1 + // index can be == len+1, so do the +1 first to avoid underflow. + (self.len() + 1) - index ); edge diff --git a/src/libcollections/slice.rs b/src/libcollections/slice.rs index a2924f8fe5308..2660161c50646 100644 --- a/src/libcollections/slice.rs +++ b/src/libcollections/slice.rs @@ -96,6 +96,7 @@ use core::iter::{range_step, MultiplicativeIterator}; use core::marker::Sized; use core::mem::size_of; use core::mem; +use core::num::wrapping::WrappingOps; use core::ops::FnMut; use core::option::Option::{self, Some, None}; use core::ptr::PtrExt; @@ -1209,10 +1210,14 @@ struct SizeDirection { impl Iterator for ElementSwaps { type Item = (usize, usize); - #[inline] + // #[inline] fn next(&mut self) -> Option<(usize, usize)> { + fn new_pos_wrapping(i: usize, s: Direction) -> usize { + i.wrapping_add(match s { Pos => 1, Neg => -1 }) + } + fn new_pos(i: usize, s: Direction) -> usize { - i + match s { Pos => 1, Neg => -1 } + match s { Pos => i + 1, Neg => i - 1 } } // Find the index of the largest mobile element: @@ -1220,7 +1225,7 @@ impl Iterator for ElementSwaps { // swap should be with a smaller `size` element. let max = self.sdir.iter().cloned().enumerate() .filter(|&(i, sd)| - new_pos(i, sd.dir) < self.sdir.len() && + new_pos_wrapping(i, sd.dir) < self.sdir.len() && self.sdir[new_pos(i, sd.dir)].size < sd.size) .max_by(|&(_, sd)| sd.size); match max { diff --git a/src/libcollections/vec_deque.rs b/src/libcollections/vec_deque.rs index abcc0cef9f1fe..3f086dd62474a 100644 --- a/src/libcollections/vec_deque.rs +++ b/src/libcollections/vec_deque.rs @@ -26,6 +26,7 @@ use core::fmt; use core::iter::{self, repeat, FromIterator, IntoIterator, RandomAccessIterator}; use core::mem; use core::num::{Int, UnsignedInt}; +use core::num::wrapping::WrappingOps; use core::ops::{Index, IndexMut}; use core::ptr::{self, Unique}; use core::raw::Slice as RawSlice; @@ -120,6 +121,20 @@ impl VecDeque { #[inline] fn wrap_index(&self, idx: usize) -> usize { wrap_index(idx, self.cap) } + /// Returns the index in the underlying buffer for a given logical element + /// index + addend. + #[inline] + fn wrap_add(&self, idx: usize, addend: usize) -> usize { + wrap_index(idx.wrapping_add(addend), self.cap) + } + + /// Returns the index in the underlying buffer for a given logical element + /// index - subtrahend. + #[inline] + fn wrap_sub(&self, idx: usize, subtrahend: usize) -> usize { + wrap_index(idx.wrapping_sub(subtrahend), self.cap) + } + /// Copies a contiguous block of memory len long from src to dst #[inline] unsafe fn copy(&self, dst: usize, src: usize, len: usize) { @@ -197,7 +212,7 @@ impl VecDeque { #[stable(feature = "rust1", since = "1.0.0")] pub fn get(&self, i: usize) -> Option<&T> { if i < self.len() { - let idx = self.wrap_index(self.tail + i); + let idx = self.wrap_add(self.tail, i); unsafe { Some(&*self.ptr.offset(idx as isize)) } } else { None @@ -227,7 +242,7 @@ impl VecDeque { #[stable(feature = "rust1", since = "1.0.0")] pub fn get_mut(&mut self, i: usize) -> Option<&mut T> { if i < self.len() { - let idx = self.wrap_index(self.tail + i); + let idx = self.wrap_add(self.tail, i); unsafe { Some(&mut *self.ptr.offset(idx as isize)) } } else { None @@ -257,8 +272,8 @@ impl VecDeque { pub fn swap(&mut self, i: usize, j: usize) { assert!(i < self.len()); assert!(j < self.len()); - let ri = self.wrap_index(self.tail + i); - let rj = self.wrap_index(self.tail + j); + let ri = self.wrap_add(self.tail, i); + let rj = self.wrap_add(self.tail, j); unsafe { ptr::swap(self.ptr.offset(ri as isize), self.ptr.offset(rj as isize)) } @@ -427,7 +442,7 @@ impl VecDeque { // [. . . o o o o o o o . . . . . . ] // H T // [o o . o o o o o ] - let len = self.wrap_index(self.head - target_cap); + let len = self.wrap_sub(self.head, target_cap); unsafe { self.copy_nonoverlapping(0, target_cap, len); } @@ -438,7 +453,7 @@ impl VecDeque { // [o o o o o . . . . . . . . . o o ] // H T // [o o o o o . o o ] - debug_assert!(self.wrap_index(self.head - 1) < target_cap); + debug_assert!(self.wrap_sub(self.head, 1) < target_cap); let len = self.cap - self.tail; let new_tail = target_cap - len; unsafe { @@ -775,7 +790,7 @@ impl VecDeque { None } else { let tail = self.tail; - self.tail = self.wrap_index(self.tail + 1); + self.tail = self.wrap_add(self.tail, 1); unsafe { Some(self.buffer_read(tail)) } } } @@ -799,7 +814,7 @@ impl VecDeque { debug_assert!(!self.is_full()); } - self.tail = self.wrap_index(self.tail - 1); + self.tail = self.wrap_sub(self.tail, 1); let tail = self.tail; unsafe { self.buffer_write(tail, t); } } @@ -824,7 +839,7 @@ impl VecDeque { } let head = self.head; - self.head = self.wrap_index(self.head + 1); + self.head = self.wrap_add(self.head, 1); unsafe { self.buffer_write(head, t) } } @@ -847,7 +862,7 @@ impl VecDeque { if self.is_empty() { None } else { - self.head = self.wrap_index(self.head - 1); + self.head = self.wrap_sub(self.head, 1); let head = self.head; unsafe { Some(self.buffer_read(head)) } } @@ -971,7 +986,7 @@ impl VecDeque { // A - The element that should be after the insertion point // M - Indicates element was moved - let idx = self.wrap_index(self.tail + i); + let idx = self.wrap_add(self.tail, i); let distance_to_tail = i; let distance_to_head = self.len() - i; @@ -990,7 +1005,7 @@ impl VecDeque { // [A o o o o o o o . . . . . I] // - self.tail = self.wrap_index(self.tail - 1); + self.tail = self.wrap_sub(self.tail, 1); }, (true, true, _) => unsafe { // contiguous, insert closer to tail: @@ -1012,7 +1027,7 @@ impl VecDeque { // [o I A o o o o o . . . . . . . o] // M M - let new_tail = self.wrap_index(self.tail - 1); + let new_tail = self.wrap_sub(self.tail, 1); self.copy(new_tail, self.tail, 1); // Already moved the tail, so we only copy `i - 1` elements. @@ -1031,7 +1046,7 @@ impl VecDeque { // M M M self.copy(idx + 1, idx, self.head - idx); - self.head = self.wrap_index(self.head + 1); + self.head = self.wrap_add(self.head, 1); }, (false, true, true) => unsafe { // discontiguous, insert closer to tail, tail section: @@ -1123,7 +1138,7 @@ impl VecDeque { } // tail might've been changed so we need to recalculate - let new_idx = self.wrap_index(self.tail + i); + let new_idx = self.wrap_add(self.tail, i); unsafe { self.buffer_write(new_idx, t); } @@ -1170,7 +1185,7 @@ impl VecDeque { // R - Indicates element that is being removed // M - Indicates element was moved - let idx = self.wrap_index(self.tail + i); + let idx = self.wrap_add(self.tail, i); let elem = unsafe { Some(self.buffer_read(idx)) @@ -1219,7 +1234,7 @@ impl VecDeque { // M M self.copy(self.tail + 1, self.tail, i); - self.tail = self.wrap_index(self.tail + 1); + self.tail = self.wrap_add(self.tail, 1); }, (false, false, false) => unsafe { // discontiguous, remove closer to head, head section: @@ -1265,7 +1280,7 @@ impl VecDeque { self.copy(0, 1, self.head - 1); } - self.head = self.wrap_index(self.head - 1); + self.head = self.wrap_sub(self.head, 1); }, (false, true, false) => unsafe { // discontiguous, remove closer to tail, head section: @@ -1286,7 +1301,7 @@ impl VecDeque { // move elements from tail to end forward, excluding the last one self.copy(self.tail + 1, self.tail, self.cap - self.tail - 1); - self.tail = self.wrap_index(self.tail + 1); + self.tail = self.wrap_add(self.tail, 1); } } @@ -1354,7 +1369,7 @@ impl VecDeque { } // Cleanup where the ends of the buffers are - self.head = self.wrap_index(self.head - other_len); + self.head = self.wrap_sub(self.head, other_len); other.head = other.wrap_index(other_len); other @@ -1429,7 +1444,7 @@ fn wrap_index(index: usize, size: usize) -> usize { #[inline] fn count(tail: usize, head: usize, size: usize) -> usize { // size is always a power of 2 - (head - tail) & (size - 1) + (head.wrapping_sub(tail)) & (size - 1) } /// `VecDeque` iterator. @@ -1461,7 +1476,7 @@ impl<'a, T> Iterator for Iter<'a, T> { return None; } let tail = self.tail; - self.tail = wrap_index(self.tail + 1, self.ring.len()); + self.tail = wrap_index(self.tail.wrapping_add(1), self.ring.len()); unsafe { Some(self.ring.get_unchecked(tail)) } } @@ -1479,7 +1494,7 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> { if self.tail == self.head { return None; } - self.head = wrap_index(self.head - 1, self.ring.len()); + self.head = wrap_index(self.head.wrapping_sub(1), self.ring.len()); unsafe { Some(self.ring.get_unchecked(self.head)) } } } @@ -1500,7 +1515,7 @@ impl<'a, T> RandomAccessIterator for Iter<'a, T> { if j >= self.indexable() { None } else { - let idx = wrap_index(self.tail + j, self.ring.len()); + let idx = wrap_index(self.tail.wrapping_add(j), self.ring.len()); unsafe { Some(self.ring.get_unchecked(idx)) } } } @@ -1524,7 +1539,7 @@ impl<'a, T> Iterator for IterMut<'a, T> { return None; } let tail = self.tail; - self.tail = wrap_index(self.tail + 1, self.ring.len()); + self.tail = wrap_index(self.tail.wrapping_add(1), self.ring.len()); unsafe { let elem = self.ring.get_unchecked_mut(tail); @@ -1546,7 +1561,7 @@ impl<'a, T> DoubleEndedIterator for IterMut<'a, T> { if self.tail == self.head { return None; } - self.head = wrap_index(self.head - 1, self.ring.len()); + self.head = wrap_index(self.head.wrapping_sub(1), self.ring.len()); unsafe { let elem = self.ring.get_unchecked_mut(self.head); diff --git a/src/libstd/collections/hash/table.rs b/src/libstd/collections/hash/table.rs index 908b5267b69d6..2670cd0c003b5 100644 --- a/src/libstd/collections/hash/table.rs +++ b/src/libstd/collections/hash/table.rs @@ -20,6 +20,7 @@ use marker::{Copy, Send, Sync, Sized, self}; use mem::{min_align_of, size_of}; use mem; use num::{Int, UnsignedInt}; +use num::wrapping::{OverflowingOps, WrappingOps}; use ops::{Deref, DerefMut, Drop}; use option::Option; use option::Option::{Some, None}; @@ -371,7 +372,6 @@ impl>> FullBucket { /// In the cited blog posts above, this is called the "distance to /// initial bucket", or DIB. Also known as "probe count". pub fn distance(&self) -> usize { - use core::num::wrapping::WrappingOps; // Calculates the distance one has to travel when going from // `hash mod capacity` onwards to `idx mod capacity`, wrapping around // if the destination is not reached before the end of the table. @@ -528,13 +528,13 @@ fn test_rounding() { fn calculate_offsets(hashes_size: usize, keys_size: usize, keys_align: usize, vals_align: usize) - -> (usize, usize) { + -> (usize, usize, bool) { let keys_offset = round_up_to_next(hashes_size, keys_align); - let end_of_keys = keys_offset + keys_size; + let (end_of_keys, oflo) = keys_offset.overflowing_add(keys_size); let vals_offset = round_up_to_next(end_of_keys, vals_align); - (keys_offset, vals_offset) + (keys_offset, vals_offset, oflo) } // Returns a tuple of (minimum required malloc alignment, hash_offset, @@ -542,26 +542,26 @@ fn calculate_offsets(hashes_size: usize, fn calculate_allocation(hash_size: usize, hash_align: usize, keys_size: usize, keys_align: usize, vals_size: usize, vals_align: usize) - -> (usize, usize, usize) { + -> (usize, usize, usize, bool) { let hash_offset = 0; - let (_, vals_offset) = calculate_offsets(hash_size, - keys_size, keys_align, - vals_align); - let end_of_vals = vals_offset + vals_size; + let (_, vals_offset, oflo) = calculate_offsets(hash_size, + keys_size, keys_align, + vals_align); + let (end_of_vals, oflo2) = vals_offset.overflowing_add(vals_size); let min_align = cmp::max(hash_align, cmp::max(keys_align, vals_align)); - (min_align, hash_offset, end_of_vals) + (min_align, hash_offset, end_of_vals, oflo || oflo2) } #[test] fn test_offset_calculation() { - assert_eq!(calculate_allocation(128, 8, 15, 1, 4, 4), (8, 0, 148)); - assert_eq!(calculate_allocation(3, 1, 2, 1, 1, 1), (1, 0, 6)); - assert_eq!(calculate_allocation(6, 2, 12, 4, 24, 8), (8, 0, 48)); - assert_eq!(calculate_offsets(128, 15, 1, 4), (128, 144)); - assert_eq!(calculate_offsets(3, 2, 1, 1), (3, 5)); - assert_eq!(calculate_offsets(6, 12, 4, 8), (8, 24)); + assert_eq!(calculate_allocation(128, 8, 15, 1, 4, 4), (8, 0, 148, false)); + assert_eq!(calculate_allocation(3, 1, 2, 1, 1, 1), (1, 0, 6, false)); + assert_eq!(calculate_allocation(6, 2, 12, 4, 24, 8), (8, 0, 48, false)); + assert_eq!(calculate_offsets(128, 15, 1, 4), (128, 144, false)); + assert_eq!(calculate_offsets(3, 2, 1, 1), (3, 5, false)); + assert_eq!(calculate_offsets(6, 12, 4, 8), (8, 24, false)); } impl RawTable { @@ -591,12 +591,14 @@ impl RawTable { // This is great in theory, but in practice getting the alignment // right is a little subtle. Therefore, calculating offsets has been // factored out into a different function. - let (malloc_alignment, hash_offset, size) = + let (malloc_alignment, hash_offset, size, oflo) = calculate_allocation( hashes_size, min_align_of::(), keys_size, min_align_of::< K >(), vals_size, min_align_of::< V >()); + assert!(!oflo, "capacity overflow"); + // One check for overflow that covers calculation and rounding of size. let size_of_bucket = size_of::().checked_add(size_of::()).unwrap() .checked_add(size_of::()).unwrap(); @@ -622,10 +624,11 @@ impl RawTable { let keys_size = self.capacity * size_of::(); let buffer = *self.hashes as *mut u8; - let (keys_offset, vals_offset) = calculate_offsets(hashes_size, - keys_size, min_align_of::(), - min_align_of::()); - + let (keys_offset, vals_offset, oflo) = + calculate_offsets(hashes_size, + keys_size, min_align_of::(), + min_align_of::()); + debug_assert!(!oflo, "capacity overflow"); unsafe { RawBucket { hash: *self.hashes, @@ -999,9 +1002,12 @@ impl Drop for RawTable { let hashes_size = self.capacity * size_of::(); let keys_size = self.capacity * size_of::(); let vals_size = self.capacity * size_of::(); - let (align, _, size) = calculate_allocation(hashes_size, min_align_of::(), - keys_size, min_align_of::(), - vals_size, min_align_of::()); + let (align, _, size, oflo) = + calculate_allocation(hashes_size, min_align_of::(), + keys_size, min_align_of::(), + vals_size, min_align_of::()); + + debug_assert!(!oflo, "should be impossible"); unsafe { deallocate(*self.hashes as *mut u8, size, align); From 4394720dae12c119a9b1aed492b1b24ee089d808 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 19 Feb 2015 12:37:27 +0100 Subject: [PATCH 10/26] Fix the overflowing tests in run-fail. * The error patterns had a typo. * Our current constant evaluation would silently allow the overflow (filed as Issue 22531). * The overflowing-mul test was accidentally doing addition instead of multiplication. --- src/test/run-fail/overflowing-add.rs | 7 +++++-- src/test/run-fail/overflowing-mul.rs | 7 +++++-- src/test/run-fail/overflowing-sub.rs | 7 +++++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/test/run-fail/overflowing-add.rs b/src/test/run-fail/overflowing-add.rs index c3e41110d20da..34a03e5f0080d 100644 --- a/src/test/run-fail/overflowing-add.rs +++ b/src/test/run-fail/overflowing-add.rs @@ -8,8 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// error-pattern:thread '
' panicked at 'arithmatic operation overflowed' +// error-pattern:thread '
' panicked at 'arithmetic operation overflowed' + +// (Work around constant-evaluation) +fn value() -> u8 { 200 } fn main() { - let x = 200u8 + 200u8 + 200u8; + let _x = value() + value() + value(); } diff --git a/src/test/run-fail/overflowing-mul.rs b/src/test/run-fail/overflowing-mul.rs index bf7a9d0758651..b18d99cd232e8 100644 --- a/src/test/run-fail/overflowing-mul.rs +++ b/src/test/run-fail/overflowing-mul.rs @@ -8,8 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// error-pattern:thread '
' panicked at 'arithmatic operation overflowed' +// error-pattern:thread '
' panicked at 'arithmetic operation overflowed' + +// (Work around constant-evaluation) +fn value() -> u8 { 200 } fn main() { - let x = 200u8 + 4u8; + let x = value() * 4; } diff --git a/src/test/run-fail/overflowing-sub.rs b/src/test/run-fail/overflowing-sub.rs index 961b36d322cdb..ee32291eca6b2 100644 --- a/src/test/run-fail/overflowing-sub.rs +++ b/src/test/run-fail/overflowing-sub.rs @@ -8,8 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// error-pattern:thread '
' panicked at 'arithmatic operation overflowed' +// error-pattern:thread '
' panicked at 'arithmetic operation overflowed' + +// (Work around constant-evaluation) +fn value() -> u8 { 42 } fn main() { - let x = 42u8 - 43u8; + let _x = value() - (value() + 1); } From c8db89aa82573b89481fde598da6e54371f266cb Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 19 Feb 2015 15:14:48 +0100 Subject: [PATCH 11/26] Accommodate arith-overflow in `core::num`, `std::num`, `coretest::num`. * `core::num`: adjust `UnsignedInt::is_power_of_two`, `UnsignedInt::next_power_of_two`, `Int::pow`. In particular for `Int::pow`: (1.) do not panic when `base` overflows if `acc` never observes the overflowed `base`, and (2.) if `acc` does observe the overflowed `base`, make sure we only panic if we would have otherwise (e.g. during a computation of `base * base`). * also in `core::num`: avoid underflow during computation of `uint::MAX`. * `std::num`: adjust tests `uint::test_uint_from_str_overflow`, `uint::test_uint_to_str_overflow`, `strconv` * `coretest::num`: adjust `test::test_int_from_str_overflow`. --- src/libcore/num/mod.rs | 26 +++++++++++++++++++++----- src/libcore/num/uint_macros.rs | 2 +- src/libcoretest/num/mod.rs | 8 ++++---- src/libstd/num/mod.rs | 16 ++++++++-------- src/libstd/num/strconv.rs | 9 +++++---- 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 3ed059520b12f..92cdd84160b7a 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -15,6 +15,8 @@ #![stable(feature = "rust1", since = "1.0.0")] #![allow(missing_docs)] +use self::wrapping::{OverflowingOps, WrappingOps}; + use char::CharExt; use clone::Clone; use cmp::{PartialEq, Eq, PartialOrd, Ord}; @@ -51,6 +53,8 @@ pub trait Int + BitXor + Shl + Shr + + WrappingOps + + OverflowingOps { /// Returns the `0` value of this integer type. // FIXME (#5527): Should be an associated constant @@ -379,11 +383,23 @@ pub trait Int let mut base = self; let mut acc: Self = Int::one(); + let mut prev_base = self; + let mut base_oflo = false; while exp > 0 { if (exp & 1) == 1 { - acc = acc * base; + if base_oflo { + // ensure overflow occurs in the same manner it + // would have otherwise (i.e. signal any exception + // it would have otherwise). + acc = acc * (prev_base * prev_base); + } else { + acc = acc * base; + } } - base = base * base; + prev_base = base; + let (new_base, new_base_oflo) = base.overflowing_mul(base); + base = new_base; + base_oflo = new_base_oflo; exp /= 2; } acc @@ -694,12 +710,12 @@ signed_int_impl! { int } /// A built-in unsigned integer. #[stable(feature = "rust1", since = "1.0.0")] -pub trait UnsignedInt: Int { +pub trait UnsignedInt: Int + WrappingOps { /// Returns `true` iff `self == 2^k` for some `k`. #[stable(feature = "rust1", since = "1.0.0")] #[inline] fn is_power_of_two(self) -> bool { - (self - Int::one()) & self == Int::zero() && !(self == Int::zero()) + (self.wrapping_sub(Int::one())) & self == Int::zero() && !(self == Int::zero()) } /// Returns the smallest power of two greater than or equal to `self`. @@ -709,7 +725,7 @@ pub trait UnsignedInt: Int { fn next_power_of_two(self) -> Self { let bits = size_of::() * 8; let one: Self = Int::one(); - one << ((bits - (self - one).leading_zeros() as usize) % bits) + one << ((bits - self.wrapping_sub(one).leading_zeros() as usize) % bits) } /// Returns the smallest power of two greater than or equal to `n`. If the diff --git a/src/libcore/num/uint_macros.rs b/src/libcore/num/uint_macros.rs index 330f0b91bf183..d0c4885ad00b7 100644 --- a/src/libcore/num/uint_macros.rs +++ b/src/libcore/num/uint_macros.rs @@ -20,6 +20,6 @@ pub const BYTES : u32 = ($bits / 8); #[stable(feature = "rust1", since = "1.0.0")] pub const MIN: $T = 0 as $T; #[stable(feature = "rust1", since = "1.0.0")] -pub const MAX: $T = 0 as $T - 1 as $T; +pub const MAX: $T = !0 as $T; ) } diff --git a/src/libcoretest/num/mod.rs b/src/libcoretest/num/mod.rs index 1cd1989c11dc3..721354b6a44c2 100644 --- a/src/libcoretest/num/mod.rs +++ b/src/libcoretest/num/mod.rs @@ -92,7 +92,7 @@ mod test { assert_eq!("127".parse::().ok(), Some(i8_val)); assert_eq!("128".parse::().ok(), None); - i8_val += 1 as i8; + i8_val = i8_val.wrapping_add(1); assert_eq!("-128".parse::().ok(), Some(i8_val)); assert_eq!("-129".parse::().ok(), None); @@ -100,7 +100,7 @@ mod test { assert_eq!("32767".parse::().ok(), Some(i16_val)); assert_eq!("32768".parse::().ok(), None); - i16_val += 1 as i16; + i16_val = i16_val.wrapping_add(1); assert_eq!("-32768".parse::().ok(), Some(i16_val)); assert_eq!("-32769".parse::().ok(), None); @@ -108,7 +108,7 @@ mod test { assert_eq!("2147483647".parse::().ok(), Some(i32_val)); assert_eq!("2147483648".parse::().ok(), None); - i32_val += 1 as i32; + i32_val = i32_val.wrapping_add(1); assert_eq!("-2147483648".parse::().ok(), Some(i32_val)); assert_eq!("-2147483649".parse::().ok(), None); @@ -116,7 +116,7 @@ mod test { assert_eq!("9223372036854775807".parse::().ok(), Some(i64_val)); assert_eq!("9223372036854775808".parse::().ok(), None); - i64_val += 1 as i64; + i64_val = i64_val.wrapping_add(1); assert_eq!("-9223372036854775808".parse::().ok(), Some(i64_val)); assert_eq!("-9223372036854775809".parse::().ok(), None); } diff --git a/src/libstd/num/mod.rs b/src/libstd/num/mod.rs index d4428282b148e..0bca60ed1a0ae 100644 --- a/src/libstd/num/mod.rs +++ b/src/libstd/num/mod.rs @@ -1758,25 +1758,25 @@ mod tests { let mut u8_val: u8 = 255_u8; assert_eq!(u8_val.to_string(), "255"); - u8_val += 1 as u8; + u8_val = u8_val.wrapping_add(1); assert_eq!(u8_val.to_string(), "0"); let mut u16_val: u16 = 65_535_u16; assert_eq!(u16_val.to_string(), "65535"); - u16_val += 1 as u16; + u16_val = u16_val.wrapping_add(1); assert_eq!(u16_val.to_string(), "0"); let mut u32_val: u32 = 4_294_967_295_u32; assert_eq!(u32_val.to_string(), "4294967295"); - u32_val += 1 as u32; + u32_val = u32_val.wrapping_add(1); assert_eq!(u32_val.to_string(), "0"); let mut u64_val: u64 = 18_446_744_073_709_551_615_u64; assert_eq!(u64_val.to_string(), "18446744073709551615"); - u64_val += 1 as u64; + u64_val = u64_val.wrapping_add(1); assert_eq!(u64_val.to_string(), "0"); } @@ -1790,7 +1790,7 @@ mod tests { assert_eq!(from_str::("255"), Some(u8_val)); assert_eq!(from_str::("256"), None); - u8_val += 1 as u8; + u8_val = u8_val.wrapping_add(1); assert_eq!(from_str::("0"), Some(u8_val)); assert_eq!(from_str::("-1"), None); @@ -1798,7 +1798,7 @@ mod tests { assert_eq!(from_str::("65535"), Some(u16_val)); assert_eq!(from_str::("65536"), None); - u16_val += 1 as u16; + u16_val = u16_val.wrapping_add(1); assert_eq!(from_str::("0"), Some(u16_val)); assert_eq!(from_str::("-1"), None); @@ -1806,7 +1806,7 @@ mod tests { assert_eq!(from_str::("4294967295"), Some(u32_val)); assert_eq!(from_str::("4294967296"), None); - u32_val += 1 as u32; + u32_val = u32_val.wrapping_add(1); assert_eq!(from_str::("0"), Some(u32_val)); assert_eq!(from_str::("-1"), None); @@ -1814,7 +1814,7 @@ mod tests { assert_eq!(from_str::("18446744073709551615"), Some(u64_val)); assert_eq!(from_str::("18446744073709551616"), None); - u64_val += 1 as u64; + u64_val = u64_val.wrapping_add(1); assert_eq!(from_str::("0"), Some(u64_val)); assert_eq!(from_str::("-1"), None); } diff --git a/src/libstd/num/strconv.rs b/src/libstd/num/strconv.rs index ca2e6ba5d5dfb..8ec19c01098ed 100644 --- a/src/libstd/num/strconv.rs +++ b/src/libstd/num/strconv.rs @@ -427,6 +427,7 @@ static DIGIT_E_RADIX: u32 = ('e' as u32) - ('a' as u32) + 11; #[cfg(test)] mod tests { + use core::num::wrapping::WrappingOps; use string::ToString; #[test] @@ -434,25 +435,25 @@ mod tests { let mut i8_val: i8 = 127_i8; assert_eq!(i8_val.to_string(), "127"); - i8_val += 1 as i8; + i8_val = i8_val.wrapping_add(1); assert_eq!(i8_val.to_string(), "-128"); let mut i16_val: i16 = 32_767_i16; assert_eq!(i16_val.to_string(), "32767"); - i16_val += 1 as i16; + i16_val = i16_val.wrapping_add(1); assert_eq!(i16_val.to_string(), "-32768"); let mut i32_val: i32 = 2_147_483_647_i32; assert_eq!(i32_val.to_string(), "2147483647"); - i32_val += 1 as i32; + i32_val = i32_val.wrapping_add(1); assert_eq!(i32_val.to_string(), "-2147483648"); let mut i64_val: i64 = 9_223_372_036_854_775_807_i64; assert_eq!(i64_val.to_string(), "9223372036854775807"); - i64_val += 1 as i64; + i64_val = i64_val.wrapping_add(1); assert_eq!(i64_val.to_string(), "-9223372036854775808"); } } From 7c8edabac8030889f4a885b944c86190772953fd Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 19 Feb 2015 19:21:53 +0100 Subject: [PATCH 12/26] Accommodate arith-overflow in serialize::json numeric parsing. --- src/libserialize/json.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libserialize/json.rs b/src/libserialize/json.rs index 14930f91c9152..bf4d006fcfaac 100644 --- a/src/libserialize/json.rs +++ b/src/libserialize/json.rs @@ -1569,8 +1569,8 @@ impl> Parser { while !self.eof() { match self.ch_or_null() { c @ '0' ... '9' => { - accum *= 10; - accum += (c as u64) - ('0' as u64); + accum = accum.wrapping_mul(10); + accum = accum.wrapping_add((c as u64) - ('0' as u64)); // Detect overflow by comparing to the last value. if accum <= last_accum { return self.error(InvalidNumber); } From 6189e99c8605578aae841be6fba9e27bfbad97fc Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 20 Feb 2015 00:10:08 +0100 Subject: [PATCH 13/26] Accommodate arith-overflow in `rand` and `std::rand`. Regarding the `rand` changes: It is unfortunate that Wrapping(T) does not support the `+=` operator. We may want to try to fix that before 1.0 to make porting code like this palatable. Regarding `std::rand`, just arith-overflow in first example from `std::rand::random()` doc. --- src/librand/chacha.rs | 11 +++--- src/librand/distributions/range.rs | 2 +- src/librand/isaac.rs | 62 +++++++++++++++--------------- src/libstd/rand/mod.rs | 4 +- 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/librand/chacha.rs b/src/librand/chacha.rs index 2673649f3440c..71ace016d6b16 100644 --- a/src/librand/chacha.rs +++ b/src/librand/chacha.rs @@ -12,6 +12,7 @@ use core::prelude::*; use core::num::Int; +use core::num::wrapping::WrappingOps; use {Rng, SeedableRng, Rand}; const KEY_WORDS : uint = 8; // 8 words for the 256-bit key @@ -43,10 +44,10 @@ static EMPTY: ChaChaRng = ChaChaRng { macro_rules! quarter_round{ ($a: expr, $b: expr, $c: expr, $d: expr) => {{ - $a += $b; $d ^= $a; $d = $d.rotate_left(16); - $c += $d; $b ^= $c; $b = $b.rotate_left(12); - $a += $b; $d ^= $a; $d = $d.rotate_left( 8); - $c += $d; $b ^= $c; $b = $b.rotate_left( 7); + $a = $a.wrapping_add($b); $d = $d ^ $a; $d = $d.rotate_left(16); + $c = $c.wrapping_add($d); $b = $b ^ $c; $b = $b.rotate_left(12); + $a = $a.wrapping_add($b); $d = $d ^ $a; $d = $d.rotate_left( 8); + $c = $c.wrapping_add($d); $b = $b ^ $c; $b = $b.rotate_left( 7); }} } @@ -74,7 +75,7 @@ fn core(output: &mut [u32; STATE_WORDS], input: &[u32; STATE_WORDS]) { } for i in 0..STATE_WORDS { - output[i] += input[i]; + output[i] = output[i].wrapping_add(input[i]); } } diff --git a/src/librand/distributions/range.rs b/src/librand/distributions/range.rs index 04c67b16a7c59..fb73a44c2b945 100644 --- a/src/librand/distributions/range.rs +++ b/src/librand/distributions/range.rs @@ -123,7 +123,7 @@ macro_rules! integer_impl { // be uniformly distributed) if v < r.accept_zone as $unsigned { // and return it, with some adjustments - return r.low + (v % r.range as $unsigned) as $ty; + return r.low.wrapping_add((v % r.range as $unsigned) as $ty); } } } diff --git a/src/librand/isaac.rs b/src/librand/isaac.rs index 63eb8417655ee..6bc00abd85f10 100644 --- a/src/librand/isaac.rs +++ b/src/librand/isaac.rs @@ -13,6 +13,7 @@ use core::prelude::*; use core::slice; use core::iter::{range_step, repeat}; +use core::num::wrapping::Wrapping; use {Rng, SeedableRng, Rand}; @@ -60,7 +61,7 @@ impl IsaacRng { /// of `rsl` as a seed, otherwise construct one algorithmically (not /// randomly). fn init(&mut self, use_rsl: bool) { - let mut a = 0x9e3779b9; + let mut a = Wrapping(0x9e3779b9); let mut b = a; let mut c = a; let mut d = a; @@ -71,14 +72,14 @@ impl IsaacRng { macro_rules! mix { () => {{ - a^=b<<11; d+=a; b+=c; - b^=c>>2; e+=b; c+=d; - c^=d<<8; f+=c; d+=e; - d^=e>>16; g+=d; e+=f; - e^=f<<10; h+=e; f+=g; - f^=g>>4; a+=f; g+=h; - g^=h<<8; b+=g; h+=a; - h^=a>>9; c+=h; a+=b; + a=a^(b<<11); d=d+a; b=b+c; + b=b^(c>>2); e=e+b; c=c+d; + c=c^(d<<8); f=f+c; d=d+e; + d=d^(e>>16); g=g+d; e=e+f; + e=e^(f<<10); h=h+e; f=f+g; + f=f^(g>>4); a=a+f; g=g+h; + g=g^(h<<8); b=b+g; h=h+a; + h=h^(a>>9); c=c+h; a=a+b; }} } @@ -90,15 +91,15 @@ impl IsaacRng { macro_rules! memloop { ($arr:expr) => {{ for i in range_step(0, RAND_SIZE as uint, 8) { - a+=$arr[i ]; b+=$arr[i+1]; - c+=$arr[i+2]; d+=$arr[i+3]; - e+=$arr[i+4]; f+=$arr[i+5]; - g+=$arr[i+6]; h+=$arr[i+7]; + a=a+Wrapping($arr[i ]); b=b+Wrapping($arr[i+1]); + c=c+Wrapping($arr[i+2]); d=d+Wrapping($arr[i+3]); + e=e+Wrapping($arr[i+4]); f=f+Wrapping($arr[i+5]); + g=g+Wrapping($arr[i+6]); h=h+Wrapping($arr[i+7]); mix!(); - self.mem[i ]=a; self.mem[i+1]=b; - self.mem[i+2]=c; self.mem[i+3]=d; - self.mem[i+4]=e; self.mem[i+5]=f; - self.mem[i+6]=g; self.mem[i+7]=h; + self.mem[i ]=a.0; self.mem[i+1]=b.0; + self.mem[i+2]=c.0; self.mem[i+3]=d.0; + self.mem[i+4]=e.0; self.mem[i+5]=f.0; + self.mem[i+6]=g.0; self.mem[i+7]=h.0; } }} } @@ -108,10 +109,10 @@ impl IsaacRng { } else { for i in range_step(0, RAND_SIZE as uint, 8) { mix!(); - self.mem[i ]=a; self.mem[i+1]=b; - self.mem[i+2]=c; self.mem[i+3]=d; - self.mem[i+4]=e; self.mem[i+5]=f; - self.mem[i+6]=g; self.mem[i+7]=h; + self.mem[i ]=a.0; self.mem[i+1]=b.0; + self.mem[i+2]=c.0; self.mem[i+3]=d.0; + self.mem[i+4]=e.0; self.mem[i+5]=f.0; + self.mem[i+6]=g.0; self.mem[i+7]=h.0; } } @@ -130,7 +131,8 @@ impl IsaacRng { static MIDPOINT: uint = (RAND_SIZE / 2) as uint; macro_rules! ind { - ($x:expr) => ( self.mem[(($x >> 2) as uint & ((RAND_SIZE - 1) as uint))] ) + ($x:expr) => (Wrapping( self.mem[(($x >> 2) as uint & + ((RAND_SIZE - 1) as uint))] )) } let r = [(0, MIDPOINT), (MIDPOINT, 0)]; @@ -142,11 +144,11 @@ impl IsaacRng { let mix = a << $shift as uint; let x = self.mem[base + mr_offset]; - a = (a ^ mix) + self.mem[base + m2_offset]; - let y = ind!(x) + a + b; - self.mem[base + mr_offset] = y; + a = (Wrapping(a ^ mix) + Wrapping(self.mem[base + m2_offset])).0; + let y = ind!(x) + Wrapping(a) + Wrapping(b); + self.mem[base + mr_offset] = y.0; - b = ind!(y >> RAND_SIZE_LEN as uint) + x; + b = (ind!(y.0 >> RAND_SIZE_LEN as uint) + Wrapping(x)).0; self.rsl[base + mr_offset] = b; }} } @@ -157,11 +159,11 @@ impl IsaacRng { let mix = a >> $shift as uint; let x = self.mem[base + mr_offset]; - a = (a ^ mix) + self.mem[base + m2_offset]; - let y = ind!(x) + a + b; - self.mem[base + mr_offset] = y; + a = (Wrapping(a ^ mix) + Wrapping(self.mem[base + m2_offset])).0; + let y = ind!(x) + Wrapping(a) + Wrapping(b); + self.mem[base + mr_offset] = y.0; - b = ind!(y >> RAND_SIZE_LEN as uint) + x; + b = (ind!(y.0 >> RAND_SIZE_LEN as uint) + Wrapping(x)).0; self.rsl[base + mr_offset] = b; }} } diff --git a/src/libstd/rand/mod.rs b/src/libstd/rand/mod.rs index 5c89144119803..ac7622fc7f724 100644 --- a/src/libstd/rand/mod.rs +++ b/src/libstd/rand/mod.rs @@ -386,8 +386,8 @@ impl Rng for ThreadRng { /// ``` /// use std::rand; /// -/// let x = rand::random(); -/// println!("{}", 2u8 * x); +/// let x: u8 = rand::random(); +/// println!("{}", 2 * x as u16); /// /// let y = rand::random::(); /// println!("{}", y); From f0404c39f272868c1dedc7cda7b0b6dffcb5713d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 20 Feb 2015 09:22:40 +0100 Subject: [PATCH 14/26] `core::iter`: fix bug uncovered by arith-overflow. (The bug was in `impl RandomAccessIterator for Rev`; it may or may not have been innocuous, depending on what guarantees one has about the behavior of `idx` for the underlying iterator.) --- src/libcore/iter.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index edc67fbf8c9c5..55bd7c17245c5 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -1130,7 +1130,11 @@ impl RandomAccessIterator for Rev where I: DoubleEndedIterator + RandomAcc #[inline] fn idx(&mut self, index: usize) -> Option<::Item> { let amt = self.indexable(); - self.iter.idx(amt - index - 1) + if amt > index { + self.iter.idx(amt - index - 1) + } else { + None + } } } From faf3bcd72c85774805ae0e84d0458aa3e67b20e4 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 20 Feb 2015 13:10:54 +0100 Subject: [PATCH 15/26] Accommodate simple cases of arith-overflow in `rustc` related crates. --- src/librustc_trans/trans/adt.rs | 6 ++++-- src/librustc_typeck/astconv.rs | 4 ++-- src/libsyntax/ext/deriving/encodable.rs | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/librustc_trans/trans/adt.rs b/src/librustc_trans/trans/adt.rs index 2fb0488cd704e..1525055f13fde 100644 --- a/src/librustc_trans/trans/adt.rs +++ b/src/librustc_trans/trans/adt.rs @@ -778,7 +778,9 @@ fn load_discr(bcx: Block, ity: IntType, ptr: ValueRef, min: Disr, max: Disr) assert!(bits <= 64); let bits = bits as uint; let mask = (-1u64 >> (64 - bits)) as Disr; - if (max + 1) & mask == min & mask { + // For a (max) discr of -1, max will be `-1 as usize`, which overflows. + // However, that is fine here (it would still represent the full range), + if (max.wrapping_add(1)) & mask == min & mask { // i.e., if the range is everything. The lo==hi case would be // rejected by the LLVM verifier (it would mean either an // empty set, which is impossible, or the entire range of the @@ -787,7 +789,7 @@ fn load_discr(bcx: Block, ity: IntType, ptr: ValueRef, min: Disr, max: Disr) } else { // llvm::ConstantRange can deal with ranges that wrap around, // so an overflow on (max + 1) is fine. - LoadRangeAssert(bcx, ptr, min, (max+1), /* signed: */ True) + LoadRangeAssert(bcx, ptr, min, (max.wrapping_add(1)), /* signed: */ True) } } diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index d9dc050aebf10..87c17c7d9ad9b 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -205,9 +205,9 @@ pub fn opt_ast_region_to_region<'tcx>( if len == 2 && i == 0 { m.push_str(" or "); - } else if i == len - 2 { + } else if i + 2 == len { m.push_str(", or "); - } else if i != len - 1 { + } else if i + 1 != len { m.push_str(", "); } } diff --git a/src/libsyntax/ext/deriving/encodable.rs b/src/libsyntax/ext/deriving/encodable.rs index 94b7d1b4d8cb0..17687534d750a 100644 --- a/src/libsyntax/ext/deriving/encodable.rs +++ b/src/libsyntax/ext/deriving/encodable.rs @@ -181,7 +181,6 @@ fn encodable_substructure(cx: &mut ExtCtxt, trait_span: Span, Struct(ref fields) => { let emit_struct_field = cx.ident_of("emit_struct_field"); let mut stmts = Vec::new(); - let last = fields.len() - 1; for (i, &FieldInfo { name, ref self_, @@ -204,6 +203,7 @@ fn encodable_substructure(cx: &mut ExtCtxt, trait_span: Span, lambda)); // last call doesn't need a try! + let last = fields.len() - 1; let call = if i != last { cx.expr_try(span, call) } else { From f1ea2b3094b1c28e64af30e187e31aa82f5ff004 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 21 Feb 2015 00:35:20 +0100 Subject: [PATCH 16/26] Catch arith-overflow explicitly during `rustc::middle::const_eval`. This only replaces the conditional arith-overflow asserts with unconditional errors from the guts of const-eval; it does *not* attempt to sanely handle such errors e.g. with a nice error message from `rustc`. So the same test that led me to add this commit are still failing, and must be addressed. --- src/librustc/middle/const_eval.rs | 40 ++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index 0c9f9d2a5301a..23844739a6beb 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -25,6 +25,7 @@ use syntax::parse::token::InternedString; use syntax::ptr::P; use syntax::{ast_map, ast_util, codemap}; +use std::num::wrapping::OverflowingOps; use std::cmp::Ordering; use std::collections::hash_map::Entry::Vacant; use std::{i8, i16, i32, i64}; @@ -206,6 +207,33 @@ pub fn eval_const_expr(tcx: &ty::ctxt, e: &Expr) -> const_val { } } +fn checked_add_int(a: i64, b: i64) -> Result { + let (ret, oflo) = a.overflowing_add(b); + if !oflo { Ok(const_int(ret)) } else { Err(format!("constant arithmetic overflow")) } +} +fn checked_sub_int(a: i64, b: i64) -> Result { + let (ret, oflo) = a.overflowing_sub(b); + if !oflo { Ok(const_int(ret)) } else { Err(format!("constant arithmetic overflow")) } +} +fn checked_mul_int(a: i64, b: i64) -> Result { + let (ret, oflo) = a.overflowing_mul(b); + if !oflo { Ok(const_int(ret)) } else { Err(format!("constant arithmetic overflow")) } +} + +fn checked_add_uint(a: u64, b: u64) -> Result { + let (ret, oflo) = a.overflowing_add(b); + if !oflo { Ok(const_uint(ret)) } else { Err(format!("constant arithmetic overflow")) } +} +fn checked_sub_uint(a: u64, b: u64) -> Result { + let (ret, oflo) = a.overflowing_sub(b); + if !oflo { Ok(const_uint(ret)) } else { Err(format!("constant arithmetic overflow")) } +} +fn checked_mul_uint(a: u64, b: u64) -> Result { + let (ret, oflo) = a.overflowing_mul(b); + if !oflo { Ok(const_uint(ret)) } else { Err(format!("constant arithmetic overflow")) } +} + + pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, e: &Expr, ty_hint: Option>) @@ -276,9 +304,9 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, } }; match op.node { - ast::BiAdd => Ok(const_int(a + b)), - ast::BiSub => Ok(const_int(a - b)), - ast::BiMul => Ok(const_int(a * b)), + ast::BiAdd => checked_add_int(a, b), + ast::BiSub => checked_sub_int(a, b), + ast::BiMul => checked_mul_int(a, b), ast::BiDiv => { if b == 0 { Err("attempted to divide by zero".to_string()) @@ -312,9 +340,9 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, } (Ok(const_uint(a)), Ok(const_uint(b))) => { match op.node { - ast::BiAdd => Ok(const_uint(a + b)), - ast::BiSub => Ok(const_uint(a - b)), - ast::BiMul => Ok(const_uint(a * b)), + ast::BiAdd => checked_add_uint(a, b), + ast::BiSub => checked_sub_uint(a, b), + ast::BiMul => checked_mul_uint(a, b), ast::BiDiv if b == 0 => { Err("attempted to divide by zero".to_string()) } From f9bbef7f448ba843052eb88733c79aa36c35d5ab Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sun, 22 Feb 2015 16:24:39 +0100 Subject: [PATCH 17/26] Avoid fatal errors in astconv; just err and return `ty_err` instead. This allows computation to proceed and find further errors. (However, this is also annoying at times when the subsequent errors are just reporting that a ty_err occurred. I have thoughts on ways to fix this that I will experiment with separately.) --- src/librustc_typeck/astconv.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 87c17c7d9ad9b..87604f4a4f80e 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1233,17 +1233,18 @@ pub fn finish_resolving_def_to_ty<'tcx>(this: &AstConv<'tcx>, if segments.is_empty() { opt_self_ty.expect("missing T in ::a::b::c") } else { - tcx.sess.span_bug(span, - &format!("found module name used as a type: {}", - tcx.map.node_to_string(id.node))); + span_err!(tcx.sess, span, E0247, "found module name used as a type: {}", + tcx.map.node_to_string(id.node)); + return this.tcx().types.err; } } def::DefPrimTy(prim_ty) => { prim_ty_to_ty(tcx, segments, prim_ty) } _ => { - span_fatal!(tcx.sess, span, E0248, - "found value name used as a type: {:?}", *def); + span_err!(tcx.sess, span, E0248, + "found value name used as a type: {:?}", *def); + return this.tcx().types.err; } }; @@ -1278,10 +1279,11 @@ pub fn ast_ty_to_ty<'tcx>(this: &AstConv<'tcx>, match ast_ty_to_ty_cache.get(&ast_ty.id) { Some(&ty::atttce_resolved(ty)) => return ty, Some(&ty::atttce_unresolved) => { - span_fatal!(tcx.sess, ast_ty.span, E0246, + span_err!(tcx.sess, ast_ty.span, E0246, "illegal recursive type; insert an enum \ or struct in the cycle, if this is \ desired"); + return this.tcx().types.err; } None => { /* go on */ } } From e919f82da1fb9fa0409f8febede311de4a8f1703 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sun, 22 Feb 2015 16:34:26 +0100 Subject: [PATCH 18/26] Address arith-overflow and error-handling in `const_eval.rs`. 1. Detect and report arithmetic overflow during const-expr eval. 2. Instead `eval_const_expr_partial` returning `Err(String)`, it now has a dedicated enum of different cases. The main benefit of this is the ability to pass along an interpretable payload, namely the two inputs that caused an overlfow. I attempted to minimize fallout to error output in tests, but some was unavoidable. Those changes are in a follow-on commit. --- src/librustc/middle/check_const.rs | 5 +- src/librustc/middle/const_eval.rs | 266 +++++++++++++++++++++++------ src/librustc/middle/ty.rs | 5 +- src/librustc_typeck/astconv.rs | 18 +- src/librustc_typeck/check/mod.rs | 5 +- src/test/compile-fail/eval-enum.rs | 4 +- 6 files changed, 234 insertions(+), 69 deletions(-) diff --git a/src/librustc/middle/check_const.rs b/src/librustc/middle/check_const.rs index 8401d25024d35..497022ac6ac49 100644 --- a/src/librustc/middle/check_const.rs +++ b/src/librustc/middle/check_const.rs @@ -307,8 +307,9 @@ impl<'a, 'tcx, 'v> Visitor<'v> for CheckCrateVisitor<'a, 'tcx> { match const_eval::eval_const_expr_partial(self.tcx, ex, None) { Ok(_) => {} Err(msg) => { - span_err!(self.tcx.sess, ex.span, E0020, - "{} in a constant expression", msg) + span_err!(self.tcx.sess, msg.span, E0020, + "{} in a constant expression", + msg.description()) } } } diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index 23844739a6beb..51bf5a6db5bbe 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -25,6 +25,7 @@ use syntax::parse::token::InternedString; use syntax::ptr::P; use syntax::{ast_map, ast_util, codemap}; +use std::borrow::{Cow, IntoCow}; use std::num::wrapping::OverflowingOps; use std::cmp::Ordering; use std::collections::hash_map::Entry::Vacant; @@ -203,42 +204,193 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr, span: Span) -> P pub fn eval_const_expr(tcx: &ty::ctxt, e: &Expr) -> const_val { match eval_const_expr_partial(tcx, e, None) { Ok(r) => r, - Err(s) => tcx.sess.span_fatal(e.span, &s[..]) + Err(s) => tcx.sess.span_fatal(s.span, s.description().as_slice()) } } -fn checked_add_int(a: i64, b: i64) -> Result { + +#[derive(Clone)] +pub struct ConstEvalErr { + pub span: Span, + pub kind: ErrKind, +} + +#[derive(Clone)] +pub enum ErrKind { + CannotCast, + CannotCastTo(&'static str), + InvalidOpForBools(ast::BinOp_), + InvalidOpForFloats(ast::BinOp_), + InvalidOpForIntUint(ast::BinOp_), + InvalidOpForUintInt(ast::BinOp_), + NegateOnString, + NegateOnBoolean, + NotOnFloat, + NotOnString, + + AddiWithOverflow(i64, i64), + SubiWithOverflow(i64, i64), + MuliWithOverflow(i64, i64), + AdduWithOverflow(u64, u64), + SubuWithOverflow(u64, u64), + MuluWithOverflow(u64, u64), + DivideByZero, + DivideWithOverflow, + ModuloByZero, + ModuloWithOverflow, + MissingStructField, + NonConstPath, + NonConstStruct, + TupleIndexOutOfBounds, + + MiscBinaryOp, + MiscCatchAll, +} + +impl ConstEvalErr { + pub fn description(&self) -> Cow { + use self::ErrKind::*; + match self.kind { + CannotCast => "can't cast this type".into_cow(), + CannotCastTo(s) => format!("can't cast this type to {}", s).into_cow(), + InvalidOpForBools(_) => "can't do this op on bools".into_cow(), + InvalidOpForFloats(_) => "can't do this op on floats".into_cow(), + InvalidOpForIntUint(..) => "can't do this op on an int and uint".into_cow(), + InvalidOpForUintInt(..) => "can't do this op on a uint and int".into_cow(), + NegateOnString => "negate on string".into_cow(), + NegateOnBoolean => "negate on boolean".into_cow(), + NotOnFloat => "not on float or string".into_cow(), + NotOnString => "not on float or string".into_cow(), + + AddiWithOverflow(..) => "attempted to add with overflow".into_cow(), + SubiWithOverflow(..) => "attempted to sub with overflow".into_cow(), + MuliWithOverflow(..) => "attempted to mul with overflow".into_cow(), + AdduWithOverflow(..) => "attempted to add with overflow".into_cow(), + SubuWithOverflow(..) => "attempted to sub with overflow".into_cow(), + MuluWithOverflow(..) => "attempted to mul with overflow".into_cow(), + DivideByZero => "attempted to divide by zero".into_cow(), + DivideWithOverflow => "attempted to divide with overflow".into_cow(), + ModuloByZero => "attempted remainder with a divisor of zero".into_cow(), + ModuloWithOverflow => "attempted remainder with overflow".into_cow(), + MissingStructField => "nonexistent struct field".into_cow(), + NonConstPath => "non-constant path in constant expr".into_cow(), + NonConstStruct => "non-constant struct in constant expr".into_cow(), + TupleIndexOutOfBounds => "tuple index out of bounds".into_cow(), + + MiscBinaryOp => "bad operands for binary".into_cow(), + MiscCatchAll => "unsupported constant expr".into_cow(), + } + } +} + +fn invalid_op_for_bools(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForBools(op) } +} +fn invalid_op_for_floats(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForFloats(op) } +} +fn invalid_op_for_int_uint(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForIntUint(op) } +} +fn invalid_op_for_uint_int(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForUintInt(op) } +} +fn negate_on_string(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::NegateOnString } +} +fn negate_on_boolean(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::NegateOnBoolean } +} +fn not_on_float(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::NotOnFloat } +} +fn not_on_string(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::NotOnString } +} + +fn addi_with_overflow(e: &Expr, a: i64, b: i64) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::AddiWithOverflow(a, b) } +} +fn subi_with_overflow(e: &Expr, a: i64, b: i64) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::SubiWithOverflow(a, b) } +} +fn muli_with_overflow(e: &Expr, a: i64, b: i64) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::MuliWithOverflow(a, b) } +} +fn addu_with_overflow(e: &Expr, a: u64, b: u64) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::AdduWithOverflow(a, b) } +} +fn subu_with_overflow(e: &Expr, a: u64, b: u64) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::SubuWithOverflow(a, b) } +} +fn mulu_with_overflow(e: &Expr, a: u64, b: u64) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::MuluWithOverflow(a, b) } +} +fn divide_by_zero(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::DivideByZero } +} +fn divide_with_overflow(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::DivideWithOverflow } +} +fn modulo_by_zero(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::ModuloByZero } +} +fn modulo_with_overflow(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::ModuloWithOverflow } +} +fn missing_struct_field(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::MissingStructField } +} +fn non_const_path(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::NonConstPath } +} +fn non_const_struct(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::NonConstStruct } +} +fn tuple_index_out_of_bounds(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::TupleIndexOutOfBounds } +} + +fn misc_binary_op(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::MiscBinaryOp } +} +fn misc_catch_all(e: &Expr) -> ConstEvalErr { + ConstEvalErr { span: e.span, kind: ErrKind::MiscCatchAll } +} + + +fn checked_add_int(e: &Expr, a: i64, b: i64) -> Result { let (ret, oflo) = a.overflowing_add(b); - if !oflo { Ok(const_int(ret)) } else { Err(format!("constant arithmetic overflow")) } + if !oflo { Ok(const_int(ret)) } else { Err(addi_with_overflow(e, a, b)) } } -fn checked_sub_int(a: i64, b: i64) -> Result { +fn checked_sub_int(e: &Expr, a: i64, b: i64) -> Result { let (ret, oflo) = a.overflowing_sub(b); - if !oflo { Ok(const_int(ret)) } else { Err(format!("constant arithmetic overflow")) } + if !oflo { Ok(const_int(ret)) } else { Err(subi_with_overflow(e, a, b)) } } -fn checked_mul_int(a: i64, b: i64) -> Result { +fn checked_mul_int(e: &Expr, a: i64, b: i64) -> Result { let (ret, oflo) = a.overflowing_mul(b); - if !oflo { Ok(const_int(ret)) } else { Err(format!("constant arithmetic overflow")) } + if !oflo { Ok(const_int(ret)) } else { Err(muli_with_overflow(e, a, b)) } } -fn checked_add_uint(a: u64, b: u64) -> Result { +fn checked_add_uint(e: &Expr, a: u64, b: u64) -> Result { let (ret, oflo) = a.overflowing_add(b); - if !oflo { Ok(const_uint(ret)) } else { Err(format!("constant arithmetic overflow")) } + if !oflo { Ok(const_uint(ret)) } else { Err(addu_with_overflow(e, a, b)) } } -fn checked_sub_uint(a: u64, b: u64) -> Result { +fn checked_sub_uint(e: &Expr, a: u64, b: u64) -> Result { let (ret, oflo) = a.overflowing_sub(b); - if !oflo { Ok(const_uint(ret)) } else { Err(format!("constant arithmetic overflow")) } + if !oflo { Ok(const_uint(ret)) } else { Err(subu_with_overflow(e, a, b)) } } -fn checked_mul_uint(a: u64, b: u64) -> Result { +fn checked_mul_uint(e: &Expr, a: u64, b: u64) -> Result { let (ret, oflo) = a.overflowing_mul(b); - if !oflo { Ok(const_uint(ret)) } else { Err(format!("constant arithmetic overflow")) } + if !oflo { Ok(const_uint(ret)) } else { Err(mulu_with_overflow(e, a, b)) } } pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, e: &Expr, ty_hint: Option>) - -> Result { - fn fromb(b: bool) -> Result { Ok(const_int(b as i64)) } + -> Result { + fn fromb(b: bool) -> Result { Ok(const_int(b as i64)) } let ety = ty_hint.or_else(|| ty::expr_ty_opt(tcx, e)); @@ -248,9 +400,9 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, Ok(const_float(f)) => Ok(const_float(-f)), Ok(const_int(i)) => Ok(const_int(-i)), Ok(const_uint(i)) => Ok(const_uint(-i)), - Ok(const_str(_)) => Err("negate on string".to_string()), - Ok(const_bool(_)) => Err("negate on boolean".to_string()), - ref err => ((*err).clone()) + Ok(const_str(_)) => Err(negate_on_string(e)), + Ok(const_bool(_)) => Err(negate_on_boolean(e)), + err => err } } ast::ExprUnary(ast::UnNot, ref inner) => { @@ -258,7 +410,9 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, Ok(const_int(i)) => Ok(const_int(!i)), Ok(const_uint(i)) => Ok(const_uint(!i)), Ok(const_bool(b)) => Ok(const_bool(!b)), - _ => Err("not on float or string".to_string()) + Ok(const_str(_)) => Err(not_on_string(e)), + Ok(const_float(_)) => Err(not_on_float(e)), + err => err } } ast::ExprBinary(op, ref a, ref b) => { @@ -281,7 +435,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiNe => fromb(a != b), ast::BiGe => fromb(a >= b), ast::BiGt => fromb(a > b), - _ => Err("can't do this op on floats".to_string()) + _ => Err(invalid_op_for_floats(e, op.node)), } } (Ok(const_int(a)), Ok(const_int(b))) => { @@ -304,23 +458,23 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, } }; match op.node { - ast::BiAdd => checked_add_int(a, b), - ast::BiSub => checked_sub_int(a, b), - ast::BiMul => checked_mul_int(a, b), + ast::BiAdd => checked_add_int(e, a, b), + ast::BiSub => checked_sub_int(e, a, b), + ast::BiMul => checked_mul_int(e, a, b), ast::BiDiv => { if b == 0 { - Err("attempted to divide by zero".to_string()) + Err(divide_by_zero(e)) } else if b == -1 && is_a_min_value() { - Err("attempted to divide with overflow".to_string()) + Err(divide_with_overflow(e)) } else { Ok(const_int(a / b)) } } ast::BiRem => { if b == 0 { - Err("attempted remainder with a divisor of zero".to_string()) + Err(modulo_by_zero(e)) } else if b == -1 && is_a_min_value() { - Err("attempted remainder with overflow".to_string()) + Err(modulo_with_overflow(e)) } else { Ok(const_int(a % b)) } @@ -340,17 +494,12 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, } (Ok(const_uint(a)), Ok(const_uint(b))) => { match op.node { - ast::BiAdd => checked_add_uint(a, b), - ast::BiSub => checked_sub_uint(a, b), - ast::BiMul => checked_mul_uint(a, b), - ast::BiDiv if b == 0 => { - Err("attempted to divide by zero".to_string()) - } + ast::BiAdd => checked_add_uint(e, a, b), + ast::BiSub => checked_sub_uint(e, a, b), + ast::BiMul => checked_mul_uint(e, a, b), + ast::BiDiv if b == 0 => Err(divide_by_zero(e)), ast::BiDiv => Ok(const_uint(a / b)), - ast::BiRem if b == 0 => { - Err("attempted remainder with a divisor of \ - zero".to_string()) - } + ast::BiRem if b == 0 => Err(modulo_by_zero(e)), ast::BiRem => Ok(const_uint(a % b)), ast::BiAnd | ast::BiBitAnd => Ok(const_uint(a & b)), ast::BiOr | ast::BiBitOr => Ok(const_uint(a | b)), @@ -370,14 +519,14 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, match op.node { ast::BiShl => Ok(const_int(a << b as uint)), ast::BiShr => Ok(const_int(a >> b as uint)), - _ => Err("can't do this op on an int and uint".to_string()) + _ => Err(invalid_op_for_int_uint(e, op.node)), } } (Ok(const_uint(a)), Ok(const_int(b))) => { match op.node { ast::BiShl => Ok(const_uint(a << b as uint)), ast::BiShr => Ok(const_uint(a >> b as uint)), - _ => Err("can't do this op on a uint and int".to_string()) + _ => Err(invalid_op_for_uint_int(e, op.node)), } } (Ok(const_bool(a)), Ok(const_bool(b))) => { @@ -389,10 +538,13 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiBitOr => a | b, ast::BiEq => a == b, ast::BiNe => a != b, - _ => return Err("can't do this op on bools".to_string()) + _ => return Err(invalid_op_for_bools(e, op.node)), })) } - _ => Err("bad operands for binary".to_string()) + (err @ Err(..), _) | + (_, err @ Err(..)) => err, + + _ => Err(misc_binary_op(e)), } } ast::ExprCast(ref base, ref target_ty) => { @@ -407,7 +559,10 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, // Prefer known type to noop, but always have a type hint. let base_hint = ty::expr_ty_opt(tcx, &**base).unwrap_or(ety); let val = try!(eval_const_expr_partial(tcx, &**base, Some(base_hint))); - cast_const(val, ety) + match cast_const(val, ety) { + Ok(val) => Ok(val), + Err(kind) => Err(ConstEvalErr { span: e.span, kind: kind }), + } } ast::ExprPath(..) => { let opt_def = tcx.def_map.borrow().get(&e.id).map(|d| d.full_def()); @@ -434,7 +589,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, }; let const_expr = match const_expr { Some(actual_e) => actual_e, - None => return Err("non-constant path in constant expr".to_string()) + None => return Err(non_const_path(e)), }; let ety = ety.or_else(|| const_ty.and_then(|ty| ast_ty_to_prim_ty(tcx, ty))); eval_const_expr_partial(tcx, const_expr, ety) @@ -456,11 +611,11 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, if fields.len() > index.node { return eval_const_expr_partial(tcx, &*fields[index.node], None) } else { - return Err("tuple index out of bounds".to_string()) + return Err(tuple_index_out_of_bounds(e)) } } - Err("non-constant struct in constant expr".to_string()) + Err(non_const_struct(e)) } ast::ExprField(ref base, field_name) => { // Get the base expression if it is a struct and it is constant @@ -471,17 +626,17 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, f.ident.node.as_str() == field_name.node.as_str()) { return eval_const_expr_partial(tcx, &*f.expr, None) } else { - return Err("nonexistent struct field".to_string()) + return Err(missing_struct_field(e)); } } - Err("non-constant struct in constant expr".to_string()) + Err(non_const_struct(e)) } - _ => Err("unsupported constant expr".to_string()) + _ => Err(misc_catch_all(e)) } } -fn cast_const(val: const_val, ty: Ty) -> Result { +fn cast_const(val: const_val, ty: Ty) -> Result { macro_rules! define_casts { ($($ty_pat:pat => ( $intermediate_ty:ty, @@ -494,11 +649,10 @@ fn cast_const(val: const_val, ty: Ty) -> Result { const_uint(u) => Ok($const_type(u as $intermediate_ty as $target_ty)), const_int(i) => Ok($const_type(i as $intermediate_ty as $target_ty)), const_float(f) => Ok($const_type(f as $intermediate_ty as $target_ty)), - _ => Err(concat!("can't cast this type to ", - stringify!($const_type)).to_string()) + _ => Err(ErrKind::CannotCastTo(stringify!($const_type))), } },)* - _ => Err("can't cast this type".to_string()) + _ => Err(ErrKind::CannotCast), }) } @@ -572,15 +726,15 @@ pub fn compare_lit_exprs<'tcx>(tcx: &ty::ctxt<'tcx>, -> Option { let a = match eval_const_expr_partial(tcx, a, ty_hint) { Ok(a) => a, - Err(s) => { - tcx.sess.span_err(a.span, &s[..]); + Err(e) => { + tcx.sess.span_err(a.span, e.description().as_slice()); return None; } }; let b = match eval_const_expr_partial(tcx, b, ty_hint) { Ok(b) => b, - Err(s) => { - tcx.sess.span_err(b.span, &s[..]); + Err(e) => { + tcx.sess.span_err(b.span, e.description().as_slice()); return None; } }; diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index b6d1fc5a36948..62947d796b7ee 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -5366,8 +5366,9 @@ pub fn enum_variants<'tcx>(cx: &ctxt<'tcx>, id: ast::DefId) "expected signed integer constant"); } Err(err) => { - span_err!(cx.sess, e.span, E0305, - "expected constant: {}", err); + span_err!(cx.sess, err.span, E0305, + "constant evaluation error: {}", + err.description().as_slice()); } } } else { diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 87604f4a4f80e..832de5251b502 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1390,14 +1390,22 @@ pub fn ast_ty_to_ty<'tcx>(this: &AstConv<'tcx>, ty::mk_vec(tcx, ast_ty_to_ty(this, rscope, &**ty), Some(i as uint)), _ => { - span_fatal!(tcx.sess, ast_ty.span, E0249, - "expected constant expr for array length"); + span_err!(tcx.sess, ast_ty.span, E0249, + "expected constant expr for array length"); + this.tcx().types.err } } } - Err(r) => { - span_fatal!(tcx.sess, ast_ty.span, E0250, - "expected constant expr for array length: {}", r); + Err(ref r) => { + let subspan = + ast_ty.span.lo <= r.span.lo && r.span.hi <= ast_ty.span.hi; + span_err!(tcx.sess, ast_ty.span, E0250, + "array length constant evaluation error: {}", + r.description().as_slice()); + if !subspan { + span_note!(tcx.sess, ast_ty.span, "for array length here") + } + this.tcx().types.err } } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 10771935e22a4..a12ff04912c49 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4604,8 +4604,9 @@ pub fn check_enum_variants<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, "expected signed integer constant"); } Err(ref err) => { - span_err!(ccx.tcx.sess, e.span, E0080, - "expected constant: {}", *err); + span_err!(ccx.tcx.sess, err.span, E0080, + "constant evaluation error: {}", + err.description().as_slice()); } } }, diff --git a/src/test/compile-fail/eval-enum.rs b/src/test/compile-fail/eval-enum.rs index 92b7b601e4dc3..ed1327f31185e 100644 --- a/src/test/compile-fail/eval-enum.rs +++ b/src/test/compile-fail/eval-enum.rs @@ -9,8 +9,8 @@ // except according to those terms. enum test { - div_zero = 1/0, //~ERROR expected constant: attempted to divide by zero - rem_zero = 1%0 //~ERROR expected constant: attempted remainder with a divisor of zero + div_zero = 1/0, //~ERROR constant evaluation error: attempted to divide by zero + rem_zero = 1%0 //~ERROR constant evaluation error: attempted remainder with a divisor of zero } fn main() {} From 5d950bd37d9f4ef46798b79d38d9d91927bb0cf4 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sun, 22 Feb 2015 16:35:11 +0100 Subject: [PATCH 19/26] Switch to `eval_const_expr_partial` when `check_match.rs` checks for `NaN`. --- src/librustc/middle/check_match.rs | 35 ++++++++++++++++++------------ 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index c409c8fb13f14..f8a2c507e4204 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -13,7 +13,8 @@ use self::Usefulness::*; use self::WitnessPreference::*; use middle::const_eval::{compare_const_vals, const_bool, const_float, const_val}; -use middle::const_eval::{const_expr_to_pat, eval_const_expr, lookup_const_by_id}; +use middle::const_eval::{eval_const_expr, eval_const_expr_partial}; +use middle::const_eval::{const_expr_to_pat, lookup_const_by_id}; use middle::def::*; use middle::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, Init}; use middle::expr_use_visitor::{JustWrite, LoanCause, MutateMode}; @@ -229,13 +230,6 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) { } } -fn is_expr_const_nan(tcx: &ty::ctxt, expr: &ast::Expr) -> bool { - match eval_const_expr(tcx, expr) { - const_float(f) => f.is_nan(), - _ => false - } -} - fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat) { ast_util::walk_pat(pat, |p| { match p.node { @@ -269,13 +263,26 @@ fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat) // Check that we do not match against a static NaN (#6804) fn check_for_static_nan(cx: &MatchCheckCtxt, pat: &Pat) { ast_util::walk_pat(pat, |p| { - match p.node { - ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => { - span_warn!(cx.tcx.sess, p.span, E0003, - "unmatchable NaN in pattern, \ - use the is_nan method in a guard instead"); + if let ast::PatLit(ref expr) = p.node { + match eval_const_expr_partial(cx.tcx, &**expr, None) { + Ok(const_float(f)) if f.is_nan() => { + span_warn!(cx.tcx.sess, p.span, E0003, + "unmatchable NaN in pattern, \ + use the is_nan method in a guard instead"); + } + Ok(_) => {} + + Err(err) => { + let subspan = p.span.lo <= err.span.lo && err.span.hi <= p.span.hi; + cx.tcx.sess.span_err(err.span, + &format!("constant evaluation error: {}", + err.description().as_slice())); + if !subspan { + cx.tcx.sess.span_note(p.span, + "in pattern here") + } + } } - _ => () } true }); From 46de12ad0064e7879b1d8ce07e96e1e31274b881 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sun, 22 Feb 2015 16:36:57 +0100 Subject: [PATCH 20/26] Switch to eval_const_expr_partial in `trans::consts`. However, I left the ICEs rather than switching to nicer error reporting there; these cases *should* be detected prior to hitting trans, and thus ICE'ing here is appropriate. (So why switch to `eval_const_expr_partial`? Because I want to try to eliminate all uses of `eval_const_expr` entirely.) --- src/librustc_trans/trans/consts.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_trans/trans/consts.rs b/src/librustc_trans/trans/consts.rs index c1d22cc973c24..a39f5d42b555f 100644 --- a/src/librustc_trans/trans/consts.rs +++ b/src/librustc_trans/trans/consts.rs @@ -462,9 +462,9 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, ast::ExprIndex(ref base, ref index) => { let (bv, bt) = const_expr(cx, &**base, param_substs); - let iv = match const_eval::eval_const_expr(cx.tcx(), &**index) { - const_eval::const_int(i) => i as u64, - const_eval::const_uint(u) => u, + let iv = match const_eval::eval_const_expr_partial(cx.tcx(), &**index, None) { + Ok(const_eval::const_int(i)) => i as u64, + Ok(const_eval::const_uint(u)) => u, _ => cx.sess().span_bug(index.span, "index is not an integer-constant expression") }; @@ -650,9 +650,9 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, ast::ExprRepeat(ref elem, ref count) => { let unit_ty = ty::sequence_element_type(cx.tcx(), ety); let llunitty = type_of::type_of(cx, unit_ty); - let n = match const_eval::eval_const_expr(cx.tcx(), &**count) { - const_eval::const_int(i) => i as uint, - const_eval::const_uint(i) => i as uint, + let n = match const_eval::eval_const_expr_partial(cx.tcx(), &**count, None) { + Ok(const_eval::const_int(i)) => i as uint, + Ok(const_eval::const_uint(i)) => i as uint, _ => cx.sess().span_bug(count.span, "count must be integral const expression.") }; let unit_val = const_expr(cx, &**elem, param_substs).0; From 8491c82c905d9b6d72742fd6dd0ec82c021bb800 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sun, 22 Feb 2015 16:38:22 +0100 Subject: [PATCH 21/26] Update tests for const-eval error handling changes. --- src/test/compile-fail/issue-18389.rs | 1 + src/test/compile-fail/issue-19244-1.rs | 4 +++- src/test/compile-fail/issue-19244-2.rs | 4 +++- src/test/compile-fail/non-constant-expr-for-fixed-len-vec.rs | 3 ++- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/compile-fail/issue-18389.rs b/src/test/compile-fail/issue-18389.rs index 20323e9900333..9065a5b9605b7 100644 --- a/src/test/compile-fail/issue-18389.rs +++ b/src/test/compile-fail/issue-18389.rs @@ -23,6 +23,7 @@ pub trait Public: Private< ::P, //~^ ERROR illegal recursive type; insert an enum or struct in the cycle, if this is desired ::R +//~^ ERROR unsupported cyclic reference between types/traits detected > { type P; type R; diff --git a/src/test/compile-fail/issue-19244-1.rs b/src/test/compile-fail/issue-19244-1.rs index 0850705aee6cb..5c11787d46780 100644 --- a/src/test/compile-fail/issue-19244-1.rs +++ b/src/test/compile-fail/issue-19244-1.rs @@ -12,5 +12,7 @@ const TUP: (usize,) = (42,); fn main() { let a: [isize; TUP.1]; - //~^ ERROR expected constant expr for array length: tuple index out of bounds + //~^ ERROR array length constant evaluation error: tuple index out of bounds + //~| ERROR attempted out-of-bounds tuple index + //~| ERROR attempted out-of-bounds tuple index } diff --git a/src/test/compile-fail/issue-19244-2.rs b/src/test/compile-fail/issue-19244-2.rs index 93a3fc87eb010..d896f76865910 100644 --- a/src/test/compile-fail/issue-19244-2.rs +++ b/src/test/compile-fail/issue-19244-2.rs @@ -13,5 +13,7 @@ const STRUCT: MyStruct = MyStruct { field: 42 }; fn main() { let a: [isize; STRUCT.nonexistent_field]; - //~^ ERROR expected constant expr for array length: nonexistent struct field + //~^ ERROR array length constant evaluation error: nonexistent struct field + //~| ERROR attempted access of field `nonexistent_field` + //~| ERROR attempted access of field `nonexistent_field` } diff --git a/src/test/compile-fail/non-constant-expr-for-fixed-len-vec.rs b/src/test/compile-fail/non-constant-expr-for-fixed-len-vec.rs index 14d2c8d0326a1..59e910ec6afda 100644 --- a/src/test/compile-fail/non-constant-expr-for-fixed-len-vec.rs +++ b/src/test/compile-fail/non-constant-expr-for-fixed-len-vec.rs @@ -13,6 +13,7 @@ fn main() { fn bar(n: isize) { let _x: [isize; n]; - //~^ ERROR expected constant expr for array length: non-constant path in constant expr + //~^ ERROR no type for local variable + //~| ERROR array length constant evaluation error: non-constant path in constant expr } } From b85b3c9b2b018e8dd8685db3217faa72ab82b965 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 27 Feb 2015 17:24:22 +0100 Subject: [PATCH 22/26] Fix span typo in the arithmetic overflow for array length reporting. Added pair of regression tests that I should have made when I thought of doing this in the first place. --- src/librustc_typeck/astconv.rs | 2 +- .../const-len-underflow-separate-spans.rs | 23 +++++++++++++++++++ .../const-len-underflow-subspans.rs | 20 ++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/const-len-underflow-separate-spans.rs create mode 100644 src/test/compile-fail/const-len-underflow-subspans.rs diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 832de5251b502..1e7b90d5a1892 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1399,7 +1399,7 @@ pub fn ast_ty_to_ty<'tcx>(this: &AstConv<'tcx>, Err(ref r) => { let subspan = ast_ty.span.lo <= r.span.lo && r.span.hi <= ast_ty.span.hi; - span_err!(tcx.sess, ast_ty.span, E0250, + span_err!(tcx.sess, r.span, E0250, "array length constant evaluation error: {}", r.description().as_slice()); if !subspan { diff --git a/src/test/compile-fail/const-len-underflow-separate-spans.rs b/src/test/compile-fail/const-len-underflow-separate-spans.rs new file mode 100644 index 0000000000000..cd021a0d3b1cb --- /dev/null +++ b/src/test/compile-fail/const-len-underflow-separate-spans.rs @@ -0,0 +1,23 @@ +// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Check that an constant-evaluation underflow highlights the correct +// spot (where the underflow occurred), while also providing the +// overall context for what caused the evaluation. + +const ONE: usize = 1; +const TWO: usize = 2; +const LEN: usize = ONE - TWO; +//~^ ERROR array length constant evaluation error: attempted to sub with overflow [E0250] + +fn main() { + let a: [i8; LEN] = unimplemented!(); + //~^ NOTE for array length here +} diff --git a/src/test/compile-fail/const-len-underflow-subspans.rs b/src/test/compile-fail/const-len-underflow-subspans.rs new file mode 100644 index 0000000000000..a31da11467924 --- /dev/null +++ b/src/test/compile-fail/const-len-underflow-subspans.rs @@ -0,0 +1,20 @@ +// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Check that an constant-evaluation underflow highlights the correct +// spot (where the underflow occurred). + +const ONE: usize = 1; +const TWO: usize = 2; + +fn main() { + let a: [i8; ONE - TWO] = unimplemented!(); + //~^ ERROR array length constant evaluation error: attempted to sub with overflow [E0250] +} From 11057fee08a6de23edbb8cb188336c104b4d25f9 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 27 Feb 2015 18:16:30 +0100 Subject: [PATCH 23/26] Incorporated first review sugestion from eddyb. Suggestion was here: https://github.com/rust-lang/rust/pull/22532#discussion_r25505609 --- src/librustc/middle/const_eval.rs | 134 ++++++++---------------------- 1 file changed, 34 insertions(+), 100 deletions(-) diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index 51bf5a6db5bbe..f5f0357976634 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -283,106 +283,40 @@ impl ConstEvalErr { } } -fn invalid_op_for_bools(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForBools(op) } -} -fn invalid_op_for_floats(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForFloats(op) } -} -fn invalid_op_for_int_uint(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForIntUint(op) } -} -fn invalid_op_for_uint_int(e: &Expr, op: ast::BinOp_) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::InvalidOpForUintInt(op) } -} -fn negate_on_string(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::NegateOnString } -} -fn negate_on_boolean(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::NegateOnBoolean } -} -fn not_on_float(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::NotOnFloat } -} -fn not_on_string(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::NotOnString } -} - -fn addi_with_overflow(e: &Expr, a: i64, b: i64) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::AddiWithOverflow(a, b) } -} -fn subi_with_overflow(e: &Expr, a: i64, b: i64) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::SubiWithOverflow(a, b) } -} -fn muli_with_overflow(e: &Expr, a: i64, b: i64) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::MuliWithOverflow(a, b) } -} -fn addu_with_overflow(e: &Expr, a: u64, b: u64) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::AdduWithOverflow(a, b) } -} -fn subu_with_overflow(e: &Expr, a: u64, b: u64) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::SubuWithOverflow(a, b) } -} -fn mulu_with_overflow(e: &Expr, a: u64, b: u64) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::MuluWithOverflow(a, b) } -} -fn divide_by_zero(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::DivideByZero } -} -fn divide_with_overflow(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::DivideWithOverflow } -} -fn modulo_by_zero(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::ModuloByZero } -} -fn modulo_with_overflow(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::ModuloWithOverflow } -} -fn missing_struct_field(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::MissingStructField } -} -fn non_const_path(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::NonConstPath } -} -fn non_const_struct(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::NonConstStruct } -} -fn tuple_index_out_of_bounds(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::TupleIndexOutOfBounds } -} +macro_rules! signal { + ($e:expr, $ctor:ident) => { + return Err(ConstEvalErr { span: $e.span, kind: ErrKind::$ctor }) + }; -fn misc_binary_op(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::MiscBinaryOp } -} -fn misc_catch_all(e: &Expr) -> ConstEvalErr { - ConstEvalErr { span: e.span, kind: ErrKind::MiscCatchAll } + ($e:expr, $ctor:ident($($arg:expr),*)) => { + return Err(ConstEvalErr { span: $e.span, kind: ErrKind::$ctor($($arg),*) }) + } } - fn checked_add_int(e: &Expr, a: i64, b: i64) -> Result { let (ret, oflo) = a.overflowing_add(b); - if !oflo { Ok(const_int(ret)) } else { Err(addi_with_overflow(e, a, b)) } + if !oflo { Ok(const_int(ret)) } else { signal!(e, AddiWithOverflow(a, b)) } } fn checked_sub_int(e: &Expr, a: i64, b: i64) -> Result { let (ret, oflo) = a.overflowing_sub(b); - if !oflo { Ok(const_int(ret)) } else { Err(subi_with_overflow(e, a, b)) } + if !oflo { Ok(const_int(ret)) } else { signal!(e, SubiWithOverflow(a, b)) } } fn checked_mul_int(e: &Expr, a: i64, b: i64) -> Result { let (ret, oflo) = a.overflowing_mul(b); - if !oflo { Ok(const_int(ret)) } else { Err(muli_with_overflow(e, a, b)) } + if !oflo { Ok(const_int(ret)) } else { signal!(e, MuliWithOverflow(a, b)) } } fn checked_add_uint(e: &Expr, a: u64, b: u64) -> Result { let (ret, oflo) = a.overflowing_add(b); - if !oflo { Ok(const_uint(ret)) } else { Err(addu_with_overflow(e, a, b)) } + if !oflo { Ok(const_uint(ret)) } else { signal!(e, AdduWithOverflow(a, b)) } } fn checked_sub_uint(e: &Expr, a: u64, b: u64) -> Result { let (ret, oflo) = a.overflowing_sub(b); - if !oflo { Ok(const_uint(ret)) } else { Err(subu_with_overflow(e, a, b)) } + if !oflo { Ok(const_uint(ret)) } else { signal!(e, SubuWithOverflow(a, b)) } } fn checked_mul_uint(e: &Expr, a: u64, b: u64) -> Result { let (ret, oflo) = a.overflowing_mul(b); - if !oflo { Ok(const_uint(ret)) } else { Err(mulu_with_overflow(e, a, b)) } + if !oflo { Ok(const_uint(ret)) } else { signal!(e, MuluWithOverflow(a, b)) } } @@ -400,8 +334,8 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, Ok(const_float(f)) => Ok(const_float(-f)), Ok(const_int(i)) => Ok(const_int(-i)), Ok(const_uint(i)) => Ok(const_uint(-i)), - Ok(const_str(_)) => Err(negate_on_string(e)), - Ok(const_bool(_)) => Err(negate_on_boolean(e)), + Ok(const_str(_)) => signal!(e, NegateOnString), + Ok(const_bool(_)) => signal!(e, NegateOnBoolean), err => err } } @@ -410,8 +344,8 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, Ok(const_int(i)) => Ok(const_int(!i)), Ok(const_uint(i)) => Ok(const_uint(!i)), Ok(const_bool(b)) => Ok(const_bool(!b)), - Ok(const_str(_)) => Err(not_on_string(e)), - Ok(const_float(_)) => Err(not_on_float(e)), + Ok(const_str(_)) => signal!(e, NotOnString), + Ok(const_float(_)) => signal!(e, NotOnFloat), err => err } } @@ -435,7 +369,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiNe => fromb(a != b), ast::BiGe => fromb(a >= b), ast::BiGt => fromb(a > b), - _ => Err(invalid_op_for_floats(e, op.node)), + _ => signal!(e, InvalidOpForFloats(op.node)) } } (Ok(const_int(a)), Ok(const_int(b))) => { @@ -463,18 +397,18 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiMul => checked_mul_int(e, a, b), ast::BiDiv => { if b == 0 { - Err(divide_by_zero(e)) + signal!(e, DivideByZero); } else if b == -1 && is_a_min_value() { - Err(divide_with_overflow(e)) + signal!(e, DivideWithOverflow); } else { Ok(const_int(a / b)) } } ast::BiRem => { if b == 0 { - Err(modulo_by_zero(e)) + signal!(e, ModuloByZero) } else if b == -1 && is_a_min_value() { - Err(modulo_with_overflow(e)) + signal!(e, ModuloWithOverflow) } else { Ok(const_int(a % b)) } @@ -497,9 +431,9 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiAdd => checked_add_uint(e, a, b), ast::BiSub => checked_sub_uint(e, a, b), ast::BiMul => checked_mul_uint(e, a, b), - ast::BiDiv if b == 0 => Err(divide_by_zero(e)), + ast::BiDiv if b == 0 => signal!(e, DivideByZero), ast::BiDiv => Ok(const_uint(a / b)), - ast::BiRem if b == 0 => Err(modulo_by_zero(e)), + ast::BiRem if b == 0 => signal!(e, ModuloByZero), ast::BiRem => Ok(const_uint(a % b)), ast::BiAnd | ast::BiBitAnd => Ok(const_uint(a & b)), ast::BiOr | ast::BiBitOr => Ok(const_uint(a | b)), @@ -519,14 +453,14 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, match op.node { ast::BiShl => Ok(const_int(a << b as uint)), ast::BiShr => Ok(const_int(a >> b as uint)), - _ => Err(invalid_op_for_int_uint(e, op.node)), + _ => signal!(e, InvalidOpForIntUint(op.node)), } } (Ok(const_uint(a)), Ok(const_int(b))) => { match op.node { ast::BiShl => Ok(const_uint(a << b as uint)), ast::BiShr => Ok(const_uint(a >> b as uint)), - _ => Err(invalid_op_for_uint_int(e, op.node)), + _ => signal!(e, InvalidOpForUintInt(op.node)), } } (Ok(const_bool(a)), Ok(const_bool(b))) => { @@ -538,13 +472,13 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiBitOr => a | b, ast::BiEq => a == b, ast::BiNe => a != b, - _ => return Err(invalid_op_for_bools(e, op.node)), + _ => signal!(e, InvalidOpForBools(op.node)), })) } (err @ Err(..), _) | (_, err @ Err(..)) => err, - _ => Err(misc_binary_op(e)), + _ => signal!(e, MiscBinaryOp), } } ast::ExprCast(ref base, ref target_ty) => { @@ -589,7 +523,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, }; let const_expr = match const_expr { Some(actual_e) => actual_e, - None => return Err(non_const_path(e)), + None => signal!(e, NonConstPath) }; let ety = ety.or_else(|| const_ty.and_then(|ty| ast_ty_to_prim_ty(tcx, ty))); eval_const_expr_partial(tcx, const_expr, ety) @@ -611,11 +545,11 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, if fields.len() > index.node { return eval_const_expr_partial(tcx, &*fields[index.node], None) } else { - return Err(tuple_index_out_of_bounds(e)) + signal!(e, TupleIndexOutOfBounds); } } - Err(non_const_struct(e)) + signal!(e, NonConstStruct); } ast::ExprField(ref base, field_name) => { // Get the base expression if it is a struct and it is constant @@ -626,13 +560,13 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, f.ident.node.as_str() == field_name.node.as_str()) { return eval_const_expr_partial(tcx, &*f.expr, None) } else { - return Err(missing_struct_field(e)); + signal!(e, MissingStructField); } } - Err(non_const_struct(e)) + signal!(e, NonConstStruct); } - _ => Err(misc_catch_all(e)) + _ => signal!(e, MiscCatchAll) } } From 4e23179c85b3706e2ff78ecb0c014e42c56b096d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sun, 1 Mar 2015 12:29:46 +0100 Subject: [PATCH 24/26] Incorporated second review suggestion from eddyb. --- src/librustc/middle/const_eval.rs | 138 +++++++++++++++--------------- 1 file changed, 71 insertions(+), 67 deletions(-) diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index f5f0357976634..9291f175777fa 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -225,8 +225,10 @@ pub enum ErrKind { InvalidOpForUintInt(ast::BinOp_), NegateOnString, NegateOnBoolean, + NegateOnBinary, NotOnFloat, NotOnString, + NotOnBinary, AddiWithOverflow(i64, i64), SubiWithOverflow(i64, i64), @@ -259,8 +261,10 @@ impl ConstEvalErr { InvalidOpForUintInt(..) => "can't do this op on a uint and int".into_cow(), NegateOnString => "negate on string".into_cow(), NegateOnBoolean => "negate on boolean".into_cow(), + NegateOnBinary => "negate on binary literal".into_cow(), NotOnFloat => "not on float or string".into_cow(), NotOnString => "not on float or string".into_cow(), + NotOnBinary => "not on binary literal".into_cow(), AddiWithOverflow(..) => "attempted to add with overflow".into_cow(), SubiWithOverflow(..) => "attempted to sub with overflow".into_cow(), @@ -324,29 +328,29 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, e: &Expr, ty_hint: Option>) -> Result { - fn fromb(b: bool) -> Result { Ok(const_int(b as i64)) } + fn fromb(b: bool) -> const_val { const_int(b as i64) } let ety = ty_hint.or_else(|| ty::expr_ty_opt(tcx, e)); - match e.node { + let result = match e.node { ast::ExprUnary(ast::UnNeg, ref inner) => { - match eval_const_expr_partial(tcx, &**inner, ety) { - Ok(const_float(f)) => Ok(const_float(-f)), - Ok(const_int(i)) => Ok(const_int(-i)), - Ok(const_uint(i)) => Ok(const_uint(-i)), - Ok(const_str(_)) => signal!(e, NegateOnString), - Ok(const_bool(_)) => signal!(e, NegateOnBoolean), - err => err + match try!(eval_const_expr_partial(tcx, &**inner, ety)) { + const_float(f) => const_float(-f), + const_int(i) => const_int(-i), + const_uint(i) => const_uint(-i), + const_str(_) => signal!(e, NegateOnString), + const_bool(_) => signal!(e, NegateOnBoolean), + const_binary(_) => signal!(e, NegateOnBinary), } } ast::ExprUnary(ast::UnNot, ref inner) => { - match eval_const_expr_partial(tcx, &**inner, ety) { - Ok(const_int(i)) => Ok(const_int(!i)), - Ok(const_uint(i)) => Ok(const_uint(!i)), - Ok(const_bool(b)) => Ok(const_bool(!b)), - Ok(const_str(_)) => signal!(e, NotOnString), - Ok(const_float(_)) => signal!(e, NotOnFloat), - err => err + match try!(eval_const_expr_partial(tcx, &**inner, ety)) { + const_int(i) => const_int(!i), + const_uint(i) => const_uint(!i), + const_bool(b) => const_bool(!b), + const_str(_) => signal!(e, NotOnString), + const_float(_) => signal!(e, NotOnFloat), + const_binary(_) => signal!(e, NotOnBinary), } } ast::ExprBinary(op, ref a, ref b) => { @@ -354,15 +358,15 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiShl | ast::BiShr => Some(tcx.types.uint), _ => ety }; - match (eval_const_expr_partial(tcx, &**a, ety), - eval_const_expr_partial(tcx, &**b, b_ty)) { - (Ok(const_float(a)), Ok(const_float(b))) => { + match (try!(eval_const_expr_partial(tcx, &**a, ety)), + try!(eval_const_expr_partial(tcx, &**b, b_ty))) { + (const_float(a), const_float(b)) => { match op.node { - ast::BiAdd => Ok(const_float(a + b)), - ast::BiSub => Ok(const_float(a - b)), - ast::BiMul => Ok(const_float(a * b)), - ast::BiDiv => Ok(const_float(a / b)), - ast::BiRem => Ok(const_float(a % b)), + ast::BiAdd => const_float(a + b), + ast::BiSub => const_float(a - b), + ast::BiMul => const_float(a * b), + ast::BiDiv => const_float(a / b), + ast::BiRem => const_float(a % b), ast::BiEq => fromb(a == b), ast::BiLt => fromb(a < b), ast::BiLe => fromb(a <= b), @@ -372,7 +376,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, _ => signal!(e, InvalidOpForFloats(op.node)) } } - (Ok(const_int(a)), Ok(const_int(b))) => { + (const_int(a), const_int(b)) => { let is_a_min_value = || { let int_ty = match ty::expr_ty_opt(tcx, e).map(|ty| &ty.sty) { Some(&ty::ty_int(int_ty)) => int_ty, @@ -392,16 +396,16 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, } }; match op.node { - ast::BiAdd => checked_add_int(e, a, b), - ast::BiSub => checked_sub_int(e, a, b), - ast::BiMul => checked_mul_int(e, a, b), + ast::BiAdd => try!(checked_add_int(e, a, b)), + ast::BiSub => try!(checked_sub_int(e, a, b)), + ast::BiMul => try!(checked_mul_int(e, a, b)), ast::BiDiv => { if b == 0 { signal!(e, DivideByZero); } else if b == -1 && is_a_min_value() { signal!(e, DivideWithOverflow); } else { - Ok(const_int(a / b)) + const_int(a / b) } } ast::BiRem => { @@ -410,14 +414,14 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, } else if b == -1 && is_a_min_value() { signal!(e, ModuloWithOverflow) } else { - Ok(const_int(a % b)) + const_int(a % b) } } - ast::BiAnd | ast::BiBitAnd => Ok(const_int(a & b)), - ast::BiOr | ast::BiBitOr => Ok(const_int(a | b)), - ast::BiBitXor => Ok(const_int(a ^ b)), - ast::BiShl => Ok(const_int(a << b as uint)), - ast::BiShr => Ok(const_int(a >> b as uint)), + ast::BiAnd | ast::BiBitAnd => const_int(a & b), + ast::BiOr | ast::BiBitOr => const_int(a | b), + ast::BiBitXor => const_int(a ^ b), + ast::BiShl => const_int(a << b as uint), + ast::BiShr => const_int(a >> b as uint), ast::BiEq => fromb(a == b), ast::BiLt => fromb(a < b), ast::BiLe => fromb(a <= b), @@ -426,20 +430,20 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiGt => fromb(a > b) } } - (Ok(const_uint(a)), Ok(const_uint(b))) => { + (const_uint(a), const_uint(b)) => { match op.node { - ast::BiAdd => checked_add_uint(e, a, b), - ast::BiSub => checked_sub_uint(e, a, b), - ast::BiMul => checked_mul_uint(e, a, b), + ast::BiAdd => try!(checked_add_uint(e, a, b)), + ast::BiSub => try!(checked_sub_uint(e, a, b)), + ast::BiMul => try!(checked_mul_uint(e, a, b)), ast::BiDiv if b == 0 => signal!(e, DivideByZero), - ast::BiDiv => Ok(const_uint(a / b)), + ast::BiDiv => const_uint(a / b), ast::BiRem if b == 0 => signal!(e, ModuloByZero), - ast::BiRem => Ok(const_uint(a % b)), - ast::BiAnd | ast::BiBitAnd => Ok(const_uint(a & b)), - ast::BiOr | ast::BiBitOr => Ok(const_uint(a | b)), - ast::BiBitXor => Ok(const_uint(a ^ b)), - ast::BiShl => Ok(const_uint(a << b as uint)), - ast::BiShr => Ok(const_uint(a >> b as uint)), + ast::BiRem => const_uint(a % b), + ast::BiAnd | ast::BiBitAnd => const_uint(a & b), + ast::BiOr | ast::BiBitOr => const_uint(a | b), + ast::BiBitXor => const_uint(a ^ b), + ast::BiShl => const_uint(a << b as uint), + ast::BiShr => const_uint(a >> b as uint), ast::BiEq => fromb(a == b), ast::BiLt => fromb(a < b), ast::BiLe => fromb(a <= b), @@ -449,22 +453,22 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, } } // shifts can have any integral type as their rhs - (Ok(const_int(a)), Ok(const_uint(b))) => { + (const_int(a), const_uint(b)) => { match op.node { - ast::BiShl => Ok(const_int(a << b as uint)), - ast::BiShr => Ok(const_int(a >> b as uint)), + ast::BiShl => const_int(a << b as uint), + ast::BiShr => const_int(a >> b as uint), _ => signal!(e, InvalidOpForIntUint(op.node)), } } - (Ok(const_uint(a)), Ok(const_int(b))) => { + (const_uint(a), const_int(b)) => { match op.node { - ast::BiShl => Ok(const_uint(a << b as uint)), - ast::BiShr => Ok(const_uint(a >> b as uint)), + ast::BiShl => const_uint(a << b as uint), + ast::BiShr => const_uint(a >> b as uint), _ => signal!(e, InvalidOpForUintInt(op.node)), } } - (Ok(const_bool(a)), Ok(const_bool(b))) => { - Ok(const_bool(match op.node { + (const_bool(a), const_bool(b)) => { + const_bool(match op.node { ast::BiAnd => a && b, ast::BiOr => a || b, ast::BiBitXor => a ^ b, @@ -473,10 +477,8 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, ast::BiEq => a == b, ast::BiNe => a != b, _ => signal!(e, InvalidOpForBools(op.node)), - })) + }) } - (err @ Err(..), _) | - (_, err @ Err(..)) => err, _ => signal!(e, MiscBinaryOp), } @@ -494,8 +496,8 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, let base_hint = ty::expr_ty_opt(tcx, &**base).unwrap_or(ety); let val = try!(eval_const_expr_partial(tcx, &**base, Some(base_hint))); match cast_const(val, ety) { - Ok(val) => Ok(val), - Err(kind) => Err(ConstEvalErr { span: e.span, kind: kind }), + Ok(val) => val, + Err(kind) => return Err(ConstEvalErr { span: e.span, kind: kind }), } } ast::ExprPath(..) => { @@ -526,16 +528,16 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, None => signal!(e, NonConstPath) }; let ety = ety.or_else(|| const_ty.and_then(|ty| ast_ty_to_prim_ty(tcx, ty))); - eval_const_expr_partial(tcx, const_expr, ety) + try!(eval_const_expr_partial(tcx, const_expr, ety)) } ast::ExprLit(ref lit) => { - Ok(lit_to_const(&**lit, ety)) + lit_to_const(&**lit, ety) } - ast::ExprParen(ref e) => eval_const_expr_partial(tcx, &**e, ety), + ast::ExprParen(ref e) => try!(eval_const_expr_partial(tcx, &**e, ety)), ast::ExprBlock(ref block) => { match block.expr { - Some(ref expr) => eval_const_expr_partial(tcx, &**expr, ety), - None => Ok(const_int(0i64)) + Some(ref expr) => try!(eval_const_expr_partial(tcx, &**expr, ety)), + None => const_int(0i64) } } ast::ExprTupField(ref base, index) => { @@ -543,7 +545,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, if let Some(&ast::ExprTup(ref fields)) = lookup_const(tcx, &**base).map(|s| &s.node) { // Check that the given index is within bounds and evaluate its value if fields.len() > index.node { - return eval_const_expr_partial(tcx, &*fields[index.node], None) + return eval_const_expr_partial(tcx, &*fields[index.node], None); } else { signal!(e, TupleIndexOutOfBounds); } @@ -558,7 +560,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, // Check that the given field exists and evaluate it if let Some(f) = fields.iter().find(|f| f.ident.node.as_str() == field_name.node.as_str()) { - return eval_const_expr_partial(tcx, &*f.expr, None) + return eval_const_expr_partial(tcx, &*f.expr, None); } else { signal!(e, MissingStructField); } @@ -567,7 +569,9 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, signal!(e, NonConstStruct); } _ => signal!(e, MiscCatchAll) - } + }; + + Ok(result) } fn cast_const(val: const_val, ty: Ty) -> Result { From 185c074798ce87429118868c292d2c2c7dc46cfc Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 2 Mar 2015 16:11:01 +0530 Subject: [PATCH 25/26] Fix backtrace tests for Linux --- src/libstd/sys/unix/backtrace.rs | 2 +- src/test/run-pass/backtrace-debuginfo.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstd/sys/unix/backtrace.rs b/src/libstd/sys/unix/backtrace.rs index 3695b615f62b8..0b8b43413e9b6 100644 --- a/src/libstd/sys/unix/backtrace.rs +++ b/src/libstd/sys/unix/backtrace.rs @@ -176,7 +176,7 @@ pub fn write(w: &mut Writer) -> IoResult<()> { let mut ip = unsafe { uw::_Unwind_GetIPInfo(ctx, &mut ip_before_insn) as *mut libc::c_void }; - if ip_before_insn == 0 { + if !ip.is_null() && ip_before_insn == 0 { // this is a non-signaling frame, so `ip` refers to the address // after the calling instruction. account for that. ip = (ip as usize - 1) as *mut _; diff --git a/src/test/run-pass/backtrace-debuginfo.rs b/src/test/run-pass/backtrace-debuginfo.rs index a2a63d44a7845..23aadbc70537e 100644 --- a/src/test/run-pass/backtrace-debuginfo.rs +++ b/src/test/run-pass/backtrace-debuginfo.rs @@ -68,7 +68,7 @@ fn dump_filelines(filelines: &[Pos]) { } #[inline(never)] -fn inner(counter: &mut u32, main_pos: Pos, outer_pos: Pos) { +fn inner(counter: &mut i32, main_pos: Pos, outer_pos: Pos) { check!(counter; main_pos, outer_pos); check!(counter; main_pos, outer_pos); let inner_pos = pos!(); aux::callback(|aux_pos| { @@ -80,12 +80,12 @@ fn inner(counter: &mut u32, main_pos: Pos, outer_pos: Pos) { } #[inline(always)] -fn inner_inlined(counter: &mut u32, main_pos: Pos, outer_pos: Pos) { +fn inner_inlined(counter: &mut i32, main_pos: Pos, outer_pos: Pos) { check!(counter; main_pos, outer_pos); check!(counter; main_pos, outer_pos); #[inline(always)] - fn inner_further_inlined(counter: &mut u32, main_pos: Pos, outer_pos: Pos, inner_pos: Pos) { + fn inner_further_inlined(counter: &mut i32, main_pos: Pos, outer_pos: Pos, inner_pos: Pos) { check!(counter; main_pos, outer_pos, inner_pos); } inner_further_inlined(counter, main_pos, outer_pos, pos!()); @@ -103,7 +103,7 @@ fn inner_inlined(counter: &mut u32, main_pos: Pos, outer_pos: Pos) { } #[inline(never)] -fn outer(mut counter: u32, main_pos: Pos) { +fn outer(mut counter: i32, main_pos: Pos) { inner(&mut counter, main_pos, pos!()); inner_inlined(&mut counter, main_pos, pos!()); } From 243c5164ea32b38c4ac44fdd5e0ceb2da45c283f Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 3 Mar 2015 12:07:48 +0100 Subject: [PATCH 26/26] sidestep potential over- and underflow in estimated stack bounds. See buildlog here for evidence of such occurring: http://buildbot.rust-lang.org/builders/auto-linux-32-opt/builds/3910/steps/test/logs/stdio --- src/libstd/rt/mod.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libstd/rt/mod.rs b/src/libstd/rt/mod.rs index 42cca73e5e241..fe32a51e81c9a 100644 --- a/src/libstd/rt/mod.rs +++ b/src/libstd/rt/mod.rs @@ -27,6 +27,7 @@ use marker::Send; use ops::FnOnce; use sys; use thunk::Thunk; +use usize; // Reexport some of our utilities which are expected by other crates. pub use self::util::{default_sched_threads, min_stack, running_on_valgrind}; @@ -78,7 +79,20 @@ fn lang_start(main: *const u8, argc: int, argv: *const *const u8) -> int { // FIXME #11359 we just assume that this thread has a stack of a // certain size, and estimate that there's at most 20KB of stack // frames above our current position. - let my_stack_bottom = my_stack_top + 20000 - OS_DEFAULT_STACK_ESTIMATE; + const TWENTY_KB: uint = 20000; + + // saturating-add to sidestep overflow + let top_plus_spill = if usize::MAX - TWENTY_KB < my_stack_top { + usize::MAX + } else { + my_stack_top + TWENTY_KB + }; + // saturating-sub to sidestep underflow + let my_stack_bottom = if top_plus_spill < OS_DEFAULT_STACK_ESTIMATE { + 0 + } else { + top_plus_spill - OS_DEFAULT_STACK_ESTIMATE + }; let failed = unsafe { // First, make sure we don't trigger any __morestack overflow checks,