From 416985d24691a99186406e8482a1c2cab200f51a Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Sat, 27 Jan 2018 15:41:06 -0500 Subject: [PATCH 1/7] Check overflow when casting floats to integers. This change adds some new macro rules used when converting from floats to integers. There are two macro rule variants, one for signed ints, one for unsigned ints. Among other things, this change specifically addresses the overflow case documented in https://github.com/rust-num/num-traits/issues/12 --- src/cast.rs | 96 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 2d7fe19a..b7f3f671 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -236,35 +236,93 @@ macro_rules! impl_to_primitive_float_to_float { ) } +macro_rules! impl_to_primitive_float_to_signed_int { + ($SrcT:ident, $DstT:ident, $slf:expr) => ( + if size_of::<$SrcT>() <= size_of::<$DstT>() { + Some($slf as $DstT) + } else { + // Make sure the value is in range for the cast. + let n = $slf as $DstT; + let max_value: $DstT = ::std::$DstT::MAX; + if -max_value as $DstT <= n && n <= max_value as $DstT { + Some($slf as $DstT) + } else { + None + } + } + ) +} + +macro_rules! impl_to_primitive_float_to_unsigned_int { + ($SrcT:ident, $DstT:ident, $slf:expr) => ( + if size_of::<$SrcT>() <= size_of::<$DstT>() { + Some($slf as $DstT) + } else { + // Make sure the value is in range for the cast. + let n = $slf as $DstT; + let max_value: $DstT = ::std::$DstT::MAX; + if n <= max_value as $DstT { + Some($slf as $DstT) + } else { + None + } + } + ) +} + macro_rules! impl_to_primitive_float { ($T:ident) => ( impl ToPrimitive for $T { #[inline] - fn to_isize(&self) -> Option { Some(*self as isize) } + fn to_isize(&self) -> Option { + impl_to_primitive_float_to_signed_int!($T, isize, *self) + } #[inline] - fn to_i8(&self) -> Option { Some(*self as i8) } + fn to_i8(&self) -> Option { + impl_to_primitive_float_to_signed_int!($T, i8, *self) + } #[inline] - fn to_i16(&self) -> Option { Some(*self as i16) } + fn to_i16(&self) -> Option { + impl_to_primitive_float_to_signed_int!($T, i16, *self) + } #[inline] - fn to_i32(&self) -> Option { Some(*self as i32) } + fn to_i32(&self) -> Option { + impl_to_primitive_float_to_signed_int!($T, i32, *self) + } #[inline] - fn to_i64(&self) -> Option { Some(*self as i64) } + fn to_i64(&self) -> Option { + impl_to_primitive_float_to_signed_int!($T, i64, *self) + } #[inline] - fn to_usize(&self) -> Option { Some(*self as usize) } + fn to_usize(&self) -> Option { + impl_to_primitive_float_to_unsigned_int!($T, usize, *self) + } #[inline] - fn to_u8(&self) -> Option { Some(*self as u8) } + fn to_u8(&self) -> Option { + impl_to_primitive_float_to_unsigned_int!($T, u8, *self) + } #[inline] - fn to_u16(&self) -> Option { Some(*self as u16) } + fn to_u16(&self) -> Option { + impl_to_primitive_float_to_unsigned_int!($T, u16, *self) + } #[inline] - fn to_u32(&self) -> Option { Some(*self as u32) } + fn to_u32(&self) -> Option { + impl_to_primitive_float_to_unsigned_int!($T, u32, *self) + } #[inline] - fn to_u64(&self) -> Option { Some(*self as u64) } + fn to_u64(&self) -> Option { + impl_to_primitive_float_to_unsigned_int!($T, u64, *self) + } #[inline] - fn to_f32(&self) -> Option { impl_to_primitive_float_to_float!($T, f32, *self) } + fn to_f32(&self) -> Option { + impl_to_primitive_float_to_float!($T, f32, *self) + } #[inline] - fn to_f64(&self) -> Option { impl_to_primitive_float_to_float!($T, f64, *self) } + fn to_f64(&self) -> Option { + impl_to_primitive_float_to_float!($T, f64, *self) + } } ) } @@ -589,3 +647,17 @@ fn as_primitive() { let x: u8 = (768i16).as_(); assert_eq!(x, 0); } + +#[test] +fn float_to_integer_checks_overflow() { + // This will overflow an i32 + let source: f64 = 1.0e+123f64; + + // Expect the overflow to be caught + assert_eq!(source.to_i32(), None); + + // Specifically make sure we didn't silently let the overflow through + // (This line is mostly for humans -- the robots should catch any problem + // on the line above). + assert_ne!(source.to_i32(), Some(-2147483648)); +} From 1349db516542602382d0867c5acea3c972e587c0 Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Sat, 27 Jan 2018 15:55:17 -0500 Subject: [PATCH 2/7] Don't use assert_ne! `num` is tested against `rust 1.8.0`, which doesn't include `assert_ne!` -- so we use a plain ol' `assert` instead. --- src/cast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cast.rs b/src/cast.rs index b7f3f671..81c85e71 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -659,5 +659,5 @@ fn float_to_integer_checks_overflow() { // Specifically make sure we didn't silently let the overflow through // (This line is mostly for humans -- the robots should catch any problem // on the line above). - assert_ne!(source.to_i32(), Some(-2147483648)); + assert!(source.to_i32() != Some(-2147483648)); } From 387049aa62fc17b249a3cecaec5f32038127885e Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 18:52:02 -0500 Subject: [PATCH 3/7] Remove an unneeded assert. --- src/cast.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 81c85e71..097d6b43 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -655,9 +655,4 @@ fn float_to_integer_checks_overflow() { // Expect the overflow to be caught assert_eq!(source.to_i32(), None); - - // Specifically make sure we didn't silently let the overflow through - // (This line is mostly for humans -- the robots should catch any problem - // on the line above). - assert!(source.to_i32() != Some(-2147483648)); } From 9b88f9bcdd3c73452f7be5deff8a36fb27e761c2 Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 19:03:12 -0500 Subject: [PATCH 4/7] Change assert form. --- src/cast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cast.rs b/src/cast.rs index 097d6b43..c9885c1f 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -654,5 +654,5 @@ fn float_to_integer_checks_overflow() { let source: f64 = 1.0e+123f64; // Expect the overflow to be caught - assert_eq!(source.to_i32(), None); + assert_eq!(cast::(source), None); } From 39edcf1f6f9601dec7056043d3c3fd919652ff22 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Mon, 29 Jan 2018 19:15:56 -0500 Subject: [PATCH 5/7] Patch in apopiak@'s changes from github.com/rust-num/num/pull/135/. --- src/cast.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index c9885c1f..57c923d2 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -238,17 +238,11 @@ macro_rules! impl_to_primitive_float_to_float { macro_rules! impl_to_primitive_float_to_signed_int { ($SrcT:ident, $DstT:ident, $slf:expr) => ( - if size_of::<$SrcT>() <= size_of::<$DstT>() { + let t = $slf.trunc(); + if t >= $DstT::MIN as $SrcT && t <= $DstT::MAX as $SrcT { Some($slf as $DstT) } else { - // Make sure the value is in range for the cast. - let n = $slf as $DstT; - let max_value: $DstT = ::std::$DstT::MAX; - if -max_value as $DstT <= n && n <= max_value as $DstT { - Some($slf as $DstT) - } else { - None - } + None } ) } @@ -656,3 +650,51 @@ fn float_to_integer_checks_overflow() { // Expect the overflow to be caught assert_eq!(cast::(source), None); } + +#[test] +fn test_cast_to_int() { + let big_f: f64 = 1.0e123; + let normal_f: f64 = 1.0; + let small_f: f64 = -1.0e123; + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + + assert_eq!(Some(normal_f as isize), cast::(normal_f)); + assert_eq!(Some(normal_f as i8), cast::(normal_f)); + assert_eq!(Some(normal_f as i16), cast::(normal_f)); + assert_eq!(Some(normal_f as i32), cast::(normal_f)); + assert_eq!(Some(normal_f as i64), cast::(normal_f)); + + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); +} + +#[test] +fn test_cast_to_unsigned_int() { + let big_f: f64 = 1.0e123; + let normal_f: f64 = 1.0; + let small_f: f64 = -1.0e123; + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + assert_eq!(None, cast::(big_f)); + + assert_eq!(Some(normal_f as usize), cast::(normal_f)); + assert_eq!(Some(normal_f as u8), cast::(normal_f)); + assert_eq!(Some(normal_f as u16), cast::(normal_f)); + assert_eq!(Some(normal_f as u32), cast::(normal_f)); + assert_eq!(Some(normal_f as u64), cast::(normal_f)); + + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); + assert_eq!(None, cast::(small_f)); +} From 0eca7f99be8b6dd66d2c7f564a68442a22da094b Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 19:38:31 -0500 Subject: [PATCH 6/7] Reformat macros. --- src/cast.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 57c923d2..ccaaa41a 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -237,31 +237,25 @@ macro_rules! impl_to_primitive_float_to_float { } macro_rules! impl_to_primitive_float_to_signed_int { - ($SrcT:ident, $DstT:ident, $slf:expr) => ( + ($SrcT:ident, $DstT:ident, $slf:expr) => ({ let t = $slf.trunc(); - if t >= $DstT::MIN as $SrcT && t <= $DstT::MAX as $SrcT { + if t >= $DstT::min_value() as $SrcT && t <= $DstT::max_value() as $SrcT { Some($slf as $DstT) } else { None } - ) + }) } macro_rules! impl_to_primitive_float_to_unsigned_int { - ($SrcT:ident, $DstT:ident, $slf:expr) => ( - if size_of::<$SrcT>() <= size_of::<$DstT>() { + ($SrcT:ident, $DstT:ident, $slf:expr) => ({ + let t = $slf.trunc(); + if t >= $DstT::min_value() as $SrcT && t <= $DstT::max_value() as $SrcT { Some($slf as $DstT) } else { - // Make sure the value is in range for the cast. - let n = $slf as $DstT; - let max_value: $DstT = ::std::$DstT::MAX; - if n <= max_value as $DstT { - Some($slf as $DstT) - } else { - None - } + None } - ) + }) } macro_rules! impl_to_primitive_float { From 8dcf2d22878d601793cf10859a5e47d48929ce76 Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 19:44:43 -0500 Subject: [PATCH 7/7] Rename some tests. --- src/cast.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index ccaaa41a..ebe53902 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -646,7 +646,7 @@ fn float_to_integer_checks_overflow() { } #[test] -fn test_cast_to_int() { +fn cast_to_int_checks_overflow() { let big_f: f64 = 1.0e123; let normal_f: f64 = 1.0; let small_f: f64 = -1.0e123; @@ -670,7 +670,7 @@ fn test_cast_to_int() { } #[test] -fn test_cast_to_unsigned_int() { +fn cast_to_unsigned_int_checks_overflow() { let big_f: f64 = 1.0e123; let normal_f: f64 = 1.0; let small_f: f64 = -1.0e123;