From f99aa0e1812bc5de88da44fb6c28e79c5ccd858a Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Sat, 27 Jan 2018 15:41:06 -0500 Subject: [PATCH 01/14] 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 630341f..5f45bd5 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -238,35 +238,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) + } } ) } @@ -591,3 +649,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 3534a8985851f8fcaeb1782a58b923da78730f9a Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Sat, 27 Jan 2018 15:55:17 -0500 Subject: [PATCH 02/14] 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 5f45bd5..005a43b 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -661,5 +661,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 ecb0816c8302c1f47e7a8168ea839e8ee7b55ad6 Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 18:52:02 -0500 Subject: [PATCH 03/14] Remove an unneeded assert. --- src/cast.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 005a43b..6b6abb1 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -657,9 +657,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 ab8fda7654264b028225e0cea483a7188ea43dd2 Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 19:03:12 -0500 Subject: [PATCH 04/14] 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 6b6abb1..288acf4 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -656,5 +656,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 c32cb5c65b18750cfb6476cdc879fa3ca3a80f5a Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Mon, 29 Jan 2018 19:15:56 -0500 Subject: [PATCH 05/14] 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 288acf4..94230ae 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -240,17 +240,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 } ) } @@ -658,3 +652,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 aab7098acd584a115d6b0a9b2f2332830444b969 Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 19:38:31 -0500 Subject: [PATCH 06/14] Reformat macros. --- src/cast.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 94230ae..58f5a90 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -239,31 +239,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 8e27c7327dce5b5603c83dc010b827360c5a09e3 Mon Sep 17 00:00:00 2001 From: Dan Barella Date: Mon, 29 Jan 2018 19:44:43 -0500 Subject: [PATCH 07/14] 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 58f5a90..164a001 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -648,7 +648,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; @@ -672,7 +672,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; From b025c273c778ee97d9de6632c5a71808f4d2be3a Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sat, 10 Mar 2018 23:01:30 -0800 Subject: [PATCH 08/14] Rewrite range checks in float ToPrimitive macros --- src/cast.rs | 63 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 164a001..ffb4406 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -1,3 +1,6 @@ +use core::{i8, i16, i32, i64, isize}; +use core::{u8, u16, u32, u64, usize}; +use core::{f32, f64}; use core::mem::size_of; use core::num::Wrapping; @@ -220,43 +223,57 @@ impl_to_primitive_uint!(u32); impl_to_primitive_uint!(u64); macro_rules! impl_to_primitive_float_to_float { - ($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. - // NaN and +-inf are cast as they are. + ($SrcT:ident, $DstT:ident, $slf:expr) => ({ + // Only finite values that are reducing size need to worry about overflow. + if size_of::<$SrcT>() > size_of::<$DstT>() && FloatCore::is_finite($slf) { let n = $slf as f64; - let max_value: $DstT = ::core::$DstT::MAX; - if !FloatCore::is_finite(n) || (-max_value as f64 <= n && n <= max_value as f64) - { - Some($slf as $DstT) - } else { - None + if n < $DstT::MIN as f64 || n > $DstT::MAX as f64 { + return None; } } - ) + // We can safely cast NaN, +-inf, and finite values in range. + Some($slf as $DstT) + }) } macro_rules! impl_to_primitive_float_to_signed_int { ($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 { - None + let t = $slf.trunc(); // round toward zero. + // MIN is a power of two, which we can cast and compare directly. + if t >= $DstT::MIN as $SrcT { + // The mantissa might not be able to represent all digits of MAX. + let sig_bits = size_of::<$DstT>() as u32 * 8 - 1; + let max = if sig_bits > $SrcT::MANTISSA_DIGITS { + let lost_bits = sig_bits - $SrcT::MANTISSA_DIGITS; + $DstT::MAX & !((1 << lost_bits) - 1) + } else { + $DstT::MAX + }; + if t <= max as $SrcT { + return Some($slf as $DstT); + } } + None }) } macro_rules! impl_to_primitive_float_to_unsigned_int { ($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 { - None + let t = $slf.trunc(); // round toward zero. + if t >= 0.0 { + // The mantissa might not be able to represent all digits of MAX. + let sig_bits = size_of::<$DstT>() as u32 * 8; + let max = if sig_bits > $SrcT::MANTISSA_DIGITS { + let lost_bits = sig_bits - $SrcT::MANTISSA_DIGITS; + $DstT::MAX & !((1 << lost_bits) - 1) + } else { + $DstT::MAX + }; + if t <= max as $SrcT { + return Some($slf as $DstT); + } } + None }) } From f6dc4d29a434901a8fea756e1a6e03ec14097fa3 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sat, 10 Mar 2018 23:05:02 -0800 Subject: [PATCH 09/14] Add thorough tests of float to int edge cases --- src/cast.rs | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/cast.rs b/src/cast.rs index ffb4406..5b5e642 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -711,3 +711,72 @@ fn cast_to_unsigned_int_checks_overflow() { assert_eq!(None, cast::(small_f)); assert_eq!(None, cast::(small_f)); } + +#[test] +fn cast_float_to_int_edge_cases() { + use core::mem::transmute; + + trait RawOffset: Sized { + type Raw; + fn raw_offset(self, offset: Self::Raw) -> Self; + } + impl RawOffset for f32 { + type Raw = i32; + fn raw_offset(self, offset: Self::Raw) -> Self { + unsafe { + let raw: Self::Raw = transmute(self); + transmute(raw + offset) + } + } + } + impl RawOffset for f64 { + type Raw = i64; + fn raw_offset(self, offset: Self::Raw) -> Self { + unsafe { + let raw: Self::Raw = transmute(self); + transmute(raw + offset) + } + } + } + + macro_rules! test_edge { + ($f:ident -> $($t:ident)+) => { $({ + println!("testing cast edge cases for {} -> {}", stringify!($f), stringify!($t)); + + let small = if $t::MIN == 0 || size_of::<$t>() < size_of::<$f>() { + $t::MIN as $f - 1.0 + } else { + ($t::MIN as $f).raw_offset(1).floor() + }; + let fmin = small.raw_offset(-1); + println!(" testing min {}\n\tvs. {:.16}\n\tand {:.16}", $t::MIN, fmin, small); + assert_eq!(Some($t::MIN), cast::<$f, $t>($t::MIN as $f)); + assert_eq!(Some($t::MIN), cast::<$f, $t>(fmin)); + assert_eq!(None, cast::<$f, $t>(small)); + + let (max, large) = if size_of::<$t>() < size_of::<$f>() { + ($t::MAX, $t::MAX as $f + 1.0) + } else { + let large = $t::MAX as $f; // rounds up! + let max = large.raw_offset(-1) as $t; // the next smallest possible + assert_eq!(max.count_ones(), $f::MANTISSA_DIGITS); + (max, large) + }; + let fmax = large.raw_offset(-1); + println!(" testing max {}\n\tvs. {:.16}\n\tand {:.16}", max, fmax, large); + assert_eq!(Some(max), cast::<$f, $t>(max as $f)); + assert_eq!(Some(max), cast::<$f, $t>(fmax)); + assert_eq!(None, cast::<$f, $t>(large)); + + println!(" testing non-finite values"); + assert_eq!(None, cast::<$f, $t>($f::NAN)); + assert_eq!(None, cast::<$f, $t>($f::INFINITY)); + assert_eq!(None, cast::<$f, $t>($f::NEG_INFINITY)); + })+} + } + + test_edge!(f32 -> isize i8 i16 i32 i64); + test_edge!(f32 -> usize u8 u16 u32 u64); + test_edge!(f64 -> isize i8 i16 i32 i64); + test_edge!(f64 -> usize u8 u16 u32 u64); +} From d195eafbe2f33937a1d278baf6c6693bab5bfc98 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sat, 10 Mar 2018 23:33:47 -0800 Subject: [PATCH 10/14] Simplify the to_primitive_float macros --- src/cast.rs | 149 ++++++++++++++++++++++------------------------------ 1 file changed, 64 insertions(+), 85 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 5b5e642..74f996f 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -223,112 +223,91 @@ impl_to_primitive_uint!(u32); impl_to_primitive_uint!(u64); macro_rules! impl_to_primitive_float_to_float { - ($SrcT:ident, $DstT:ident, $slf:expr) => ({ - // Only finite values that are reducing size need to worry about overflow. - if size_of::<$SrcT>() > size_of::<$DstT>() && FloatCore::is_finite($slf) { - let n = $slf as f64; - if n < $DstT::MIN as f64 || n > $DstT::MAX as f64 { - return None; + ($SrcT:ident : $( fn $method:ident -> $DstT:ident ; )*) => {$( + #[inline] + fn $method(&self) -> Option<$DstT> { + // Only finite values that are reducing size need to worry about overflow. + if size_of::<$SrcT>() > size_of::<$DstT>() && FloatCore::is_finite(*self) { + let n = *self as f64; + if n < $DstT::MIN as f64 || n > $DstT::MAX as f64 { + return None; + } } + // We can safely cast NaN, +-inf, and finite values in range. + Some(*self as $DstT) } - // We can safely cast NaN, +-inf, and finite values in range. - Some($slf as $DstT) - }) + )*} } macro_rules! impl_to_primitive_float_to_signed_int { - ($SrcT:ident, $DstT:ident, $slf:expr) => ({ - let t = $slf.trunc(); // round toward zero. - // MIN is a power of two, which we can cast and compare directly. - if t >= $DstT::MIN as $SrcT { - // The mantissa might not be able to represent all digits of MAX. - let sig_bits = size_of::<$DstT>() as u32 * 8 - 1; - let max = if sig_bits > $SrcT::MANTISSA_DIGITS { - let lost_bits = sig_bits - $SrcT::MANTISSA_DIGITS; - $DstT::MAX & !((1 << lost_bits) - 1) - } else { - $DstT::MAX - }; - if t <= max as $SrcT { - return Some($slf as $DstT); + ($f:ident : $( fn $method:ident -> $i:ident ; )*) => {$( + #[inline] + fn $method(&self) -> Option<$i> { + let t = self.trunc(); // round toward zero. + // MIN is a power of two, which we can cast and compare directly. + if t >= $i::MIN as $f { + // The mantissa might not be able to represent all digits of MAX. + let sig_bits = size_of::<$i>() as u32 * 8 - 1; + let max = if sig_bits > $f::MANTISSA_DIGITS { + let lost_bits = sig_bits - $f::MANTISSA_DIGITS; + $i::MAX & !((1 << lost_bits) - 1) + } else { + $i::MAX + }; + if t <= max as $f { + return Some(*self as $i); + } } + None } - None - }) + )*} } macro_rules! impl_to_primitive_float_to_unsigned_int { - ($SrcT:ident, $DstT:ident, $slf:expr) => ({ - let t = $slf.trunc(); // round toward zero. - if t >= 0.0 { - // The mantissa might not be able to represent all digits of MAX. - let sig_bits = size_of::<$DstT>() as u32 * 8; - let max = if sig_bits > $SrcT::MANTISSA_DIGITS { - let lost_bits = sig_bits - $SrcT::MANTISSA_DIGITS; - $DstT::MAX & !((1 << lost_bits) - 1) - } else { - $DstT::MAX - }; - if t <= max as $SrcT { - return Some($slf as $DstT); + ($f:ident : $( fn $method:ident -> $u:ident ; )*) => {$( + #[inline] + fn $method(&self) -> Option<$u> { + let t = self.trunc(); // round toward zero. + if t >= 0.0 { + // The mantissa might not be able to represent all digits of MAX. + let sig_bits = size_of::<$u>() as u32 * 8; + let max = if sig_bits > $f::MANTISSA_DIGITS { + let lost_bits = sig_bits - $f::MANTISSA_DIGITS; + $u::MAX & !((1 << lost_bits) - 1) + } else { + $u::MAX + }; + if t <= max as $f { + return Some(*self as $u); + } } + None } - None - }) + )*} } macro_rules! impl_to_primitive_float { ($T:ident) => ( impl ToPrimitive for $T { - #[inline] - fn to_isize(&self) -> Option { - impl_to_primitive_float_to_signed_int!($T, isize, *self) - } - #[inline] - fn to_i8(&self) -> Option { - impl_to_primitive_float_to_signed_int!($T, i8, *self) - } - #[inline] - fn to_i16(&self) -> Option { - impl_to_primitive_float_to_signed_int!($T, i16, *self) - } - #[inline] - fn to_i32(&self) -> Option { - impl_to_primitive_float_to_signed_int!($T, i32, *self) - } - #[inline] - fn to_i64(&self) -> Option { - impl_to_primitive_float_to_signed_int!($T, i64, *self) + impl_to_primitive_float_to_signed_int! { $T: + fn to_isize -> isize; + fn to_i8 -> i8; + fn to_i16 -> i16; + fn to_i32 -> i32; + fn to_i64 -> i64; } - #[inline] - fn to_usize(&self) -> Option { - impl_to_primitive_float_to_unsigned_int!($T, usize, *self) - } - #[inline] - fn to_u8(&self) -> Option { - impl_to_primitive_float_to_unsigned_int!($T, u8, *self) - } - #[inline] - fn to_u16(&self) -> Option { - impl_to_primitive_float_to_unsigned_int!($T, u16, *self) - } - #[inline] - fn to_u32(&self) -> Option { - impl_to_primitive_float_to_unsigned_int!($T, u32, *self) - } - #[inline] - fn to_u64(&self) -> Option { - impl_to_primitive_float_to_unsigned_int!($T, u64, *self) + impl_to_primitive_float_to_unsigned_int! { $T: + fn to_usize -> usize; + fn to_u8 -> u8; + fn to_u16 -> u16; + fn to_u32 -> u32; + fn to_u64 -> u64; } - #[inline] - 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) + impl_to_primitive_float_to_float! { $T: + fn to_f32 -> f32; + fn to_f64 -> f64; } } ) From 6d7bbb1b533beef0952ee10392affd8c0ded8aa1 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sun, 11 Mar 2018 01:36:17 -0800 Subject: [PATCH 11/14] Mask debug prints no-std mode --- src/cast.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 74f996f..3a83b06 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -691,6 +691,19 @@ fn cast_to_unsigned_int_checks_overflow() { assert_eq!(None, cast::(small_f)); } +#[cfg(all(test, feature = "std"))] +fn dbg(args: ::core::fmt::Arguments) { + println!("{}", args); +} + +#[cfg(all(test, not(feature = "std")))] +fn dbg(_: ::core::fmt::Arguments) {} + +// Rust 1.8 doesn't handle cfg on macros correctly +// #[cfg(test)] +#[allow(unused)] +macro_rules! dbg { ($($tok:tt)*) => { dbg(format_args!($($tok)*)) } } + #[test] fn cast_float_to_int_edge_cases() { use core::mem::transmute; @@ -720,7 +733,7 @@ fn cast_float_to_int_edge_cases() { macro_rules! test_edge { ($f:ident -> $($t:ident)+) => { $({ - println!("testing cast edge cases for {} -> {}", stringify!($f), stringify!($t)); + dbg!("testing cast edge cases for {} -> {}", stringify!($f), stringify!($t)); let small = if $t::MIN == 0 || size_of::<$t>() < size_of::<$f>() { $t::MIN as $f - 1.0 @@ -728,7 +741,7 @@ fn cast_float_to_int_edge_cases() { ($t::MIN as $f).raw_offset(1).floor() }; let fmin = small.raw_offset(-1); - println!(" testing min {}\n\tvs. {:.16}\n\tand {:.16}", $t::MIN, fmin, small); + dbg!(" testing min {}\n\tvs. {:.16}\n\tand {:.16}", $t::MIN, fmin, small); assert_eq!(Some($t::MIN), cast::<$f, $t>($t::MIN as $f)); assert_eq!(Some($t::MIN), cast::<$f, $t>(fmin)); assert_eq!(None, cast::<$f, $t>(small)); @@ -742,12 +755,12 @@ fn cast_float_to_int_edge_cases() { (max, large) }; let fmax = large.raw_offset(-1); - println!(" testing max {}\n\tvs. {:.16}\n\tand {:.16}", max, fmax, large); + dbg!(" testing max {}\n\tvs. {:.16}\n\tand {:.16}", max, fmax, large); assert_eq!(Some(max), cast::<$f, $t>(max as $f)); assert_eq!(Some(max), cast::<$f, $t>(fmax)); assert_eq!(None, cast::<$f, $t>(large)); - println!(" testing non-finite values"); + dbg!(" testing non-finite values"); assert_eq!(None, cast::<$f, $t>($f::NAN)); assert_eq!(None, cast::<$f, $t>($f::INFINITY)); assert_eq!(None, cast::<$f, $t>($f::NEG_INFINITY)); From 50868c60d2d37cd68ed545f180faf66b96e2271b Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sun, 11 Mar 2018 01:37:00 -0800 Subject: [PATCH 12/14] Refactor to_primitive_int/uint macros --- src/cast.rs | 157 ++++++++++++++++++++++------------------------------ 1 file changed, 67 insertions(+), 90 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 3a83b06..694b0e5 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -4,8 +4,6 @@ use core::{f32, f64}; use core::mem::size_of; use core::num::Wrapping; -use identities::Zero; -use bounds::Bounded; use float::FloatCore; /// A generic trait for converting a value to a number. @@ -79,62 +77,52 @@ pub trait ToPrimitive { } macro_rules! impl_to_primitive_int_to_int { - ($SrcT:ty, $DstT:ty, $slf:expr) => ( - { - if size_of::<$SrcT>() <= size_of::<$DstT>() { - Some($slf as $DstT) - } else { - let n = $slf as i64; - let min_value: $DstT = Bounded::min_value(); - let max_value: $DstT = Bounded::max_value(); - if min_value as i64 <= n && n <= max_value as i64 { - Some($slf as $DstT) - } else { - None - } - } - } - ) -} - -macro_rules! impl_to_primitive_int_to_uint { - ($SrcT:ty, $DstT:ty, $slf:expr) => ( - { - let zero: $SrcT = Zero::zero(); - let max_value: $DstT = Bounded::max_value(); - if zero <= $slf && $slf as u64 <= max_value as u64 { - Some($slf as $DstT) + ($SrcT:ident : $( fn $method:ident -> $DstT:ident ; )*) => {$( + #[inline] + fn $method(&self) -> Option<$DstT> { + let min = $DstT::MIN as $SrcT; + let max = $DstT::MAX as $SrcT; + if size_of::<$SrcT>() <= size_of::<$DstT>() || (min <= *self && *self <= max) { + Some(*self as $DstT) } else { None } } - ) + )*} +} + +macro_rules! impl_to_primitive_int_to_uint { + ($SrcT:ident : $( fn $method:ident -> $DstT:ident ; )*) => {$( + #[inline] + fn $method(&self) -> Option<$DstT> { + let max = $DstT::MAX as u64; + if 0 <= *self && (size_of::<$SrcT>() < size_of::<$DstT>() || *self as u64 <= max) { + Some(*self as $DstT) + } else { + None + } + } + )*} } macro_rules! impl_to_primitive_int { - ($T:ty) => ( + ($T:ident) => ( impl ToPrimitive for $T { - #[inline] - fn to_isize(&self) -> Option { impl_to_primitive_int_to_int!($T, isize, *self) } - #[inline] - fn to_i8(&self) -> Option { impl_to_primitive_int_to_int!($T, i8, *self) } - #[inline] - fn to_i16(&self) -> Option { impl_to_primitive_int_to_int!($T, i16, *self) } - #[inline] - fn to_i32(&self) -> Option { impl_to_primitive_int_to_int!($T, i32, *self) } - #[inline] - fn to_i64(&self) -> Option { impl_to_primitive_int_to_int!($T, i64, *self) } + impl_to_primitive_int_to_int! { $T: + fn to_isize -> isize; + fn to_i8 -> i8; + fn to_i16 -> i16; + fn to_i32 -> i32; + fn to_i64 -> i64; + } - #[inline] - fn to_usize(&self) -> Option { impl_to_primitive_int_to_uint!($T, usize, *self) } - #[inline] - fn to_u8(&self) -> Option { impl_to_primitive_int_to_uint!($T, u8, *self) } - #[inline] - fn to_u16(&self) -> Option { impl_to_primitive_int_to_uint!($T, u16, *self) } - #[inline] - fn to_u32(&self) -> Option { impl_to_primitive_int_to_uint!($T, u32, *self) } - #[inline] - fn to_u64(&self) -> Option { impl_to_primitive_int_to_uint!($T, u64, *self) } + impl_to_primitive_int_to_uint! { $T: + fn to_usize -> usize; + fn to_u8 -> u8; + fn to_u16 -> u16; + fn to_u32 -> u32; + fn to_u64 -> u64; + } #[inline] fn to_f32(&self) -> Option { Some(*self as f32) } @@ -151,62 +139,51 @@ impl_to_primitive_int!(i32); impl_to_primitive_int!(i64); macro_rules! impl_to_primitive_uint_to_int { - ($DstT:ty, $slf:expr) => ( - { - let max_value: $DstT = Bounded::max_value(); - if $slf as u64 <= max_value as u64 { - Some($slf as $DstT) + ($SrcT:ident : $( fn $method:ident -> $DstT:ident ; )*) => {$( + #[inline] + fn $method(&self) -> Option<$DstT> { + let max = $DstT::MAX as u64; + if size_of::<$SrcT>() < size_of::<$DstT>() || *self as u64 <= max { + Some(*self as $DstT) } else { None } } - ) + )*} } macro_rules! impl_to_primitive_uint_to_uint { - ($SrcT:ty, $DstT:ty, $slf:expr) => ( - { - if size_of::<$SrcT>() <= size_of::<$DstT>() { - Some($slf as $DstT) + ($SrcT:ident : $( fn $method:ident -> $DstT:ident ; )*) => {$( + #[inline] + fn $method(&self) -> Option<$DstT> { + let max = $DstT::MAX as $SrcT; + if size_of::<$SrcT>() <= size_of::<$DstT>() || *self <= max { + Some(*self as $DstT) } else { - let zero: $SrcT = Zero::zero(); - let max_value: $DstT = Bounded::max_value(); - if zero <= $slf && $slf as u64 <= max_value as u64 { - Some($slf as $DstT) - } else { - None - } + None } } - ) + )*} } macro_rules! impl_to_primitive_uint { - ($T:ty) => ( + ($T:ident) => ( impl ToPrimitive for $T { - #[inline] - fn to_isize(&self) -> Option { impl_to_primitive_uint_to_int!(isize, *self) } - #[inline] - fn to_i8(&self) -> Option { impl_to_primitive_uint_to_int!(i8, *self) } - #[inline] - fn to_i16(&self) -> Option { impl_to_primitive_uint_to_int!(i16, *self) } - #[inline] - fn to_i32(&self) -> Option { impl_to_primitive_uint_to_int!(i32, *self) } - #[inline] - fn to_i64(&self) -> Option { impl_to_primitive_uint_to_int!(i64, *self) } - - #[inline] - fn to_usize(&self) -> Option { - impl_to_primitive_uint_to_uint!($T, usize, *self) + impl_to_primitive_uint_to_int! { $T: + fn to_isize -> isize; + fn to_i8 -> i8; + fn to_i16 -> i16; + fn to_i32 -> i32; + fn to_i64 -> i64; + } + + impl_to_primitive_uint_to_uint! { $T: + fn to_usize -> usize; + fn to_u8 -> u8; + fn to_u16 -> u16; + fn to_u32 -> u32; + fn to_u64 -> u64; } - #[inline] - fn to_u8(&self) -> Option { impl_to_primitive_uint_to_uint!($T, u8, *self) } - #[inline] - fn to_u16(&self) -> Option { impl_to_primitive_uint_to_uint!($T, u16, *self) } - #[inline] - fn to_u32(&self) -> Option { impl_to_primitive_uint_to_uint!($T, u32, *self) } - #[inline] - fn to_u64(&self) -> Option { impl_to_primitive_uint_to_uint!($T, u64, *self) } #[inline] fn to_f32(&self) -> Option { Some(*self as f32) } From f0ed42b3bc45cdbb4323ad2ece36fe164a9bd066 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sun, 11 Mar 2018 01:37:27 -0800 Subject: [PATCH 13/14] Test edge cases of ToPrimitive with ints --- src/cast.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/cast.rs b/src/cast.rs index 694b0e5..8a87008 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -749,3 +749,52 @@ fn cast_float_to_int_edge_cases() { test_edge!(f64 -> isize i8 i16 i32 i64); test_edge!(f64 -> usize u8 u16 u32 u64); } + +#[test] +fn cast_int_to_int_edge_cases() { + use core::cmp::Ordering::*; + + macro_rules! test_edge { + ($f:ident -> $($t:ident)+) => { $({ + fn test_edge() { + dbg!("testing cast edge cases for {} -> {}", stringify!($f), stringify!($t)); + + match ($f::MIN as i64).cmp(&($t::MIN as i64)) { + Greater => { + assert_eq!(Some($f::MIN as $t), cast::<$f, $t>($f::MIN)); + } + Equal => { + assert_eq!(Some($t::MIN), cast::<$f, $t>($f::MIN)); + } + Less => { + let min = $t::MIN as $f; + assert_eq!(Some($t::MIN), cast::<$f, $t>(min)); + assert_eq!(None, cast::<$f, $t>(min - 1)); + } + } + + match ($f::MAX as u64).cmp(&($t::MAX as u64)) { + Greater => { + let max = $t::MAX as $f; + assert_eq!(Some($t::MAX), cast::<$f, $t>(max)); + assert_eq!(None, cast::<$f, $t>(max + 1)); + } + Equal => { + assert_eq!(Some($t::MAX), cast::<$f, $t>($f::MAX)); + } + Less => { + assert_eq!(Some($f::MAX as $t), cast::<$f, $t>($f::MAX)); + } + } + } + test_edge(); + })+}; + ($( $from:ident )+) => { $({ + test_edge!($from -> isize i8 i16 i32 i64); + test_edge!($from -> usize u8 u16 u32 u64); + })+} + } + + test_edge!(isize i8 i16 i32 i64); + test_edge!(usize u8 u16 u32 u64); +} From a4d234c25309ad0642a4504c89361d7f51881f11 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 13 Mar 2018 13:38:17 -0700 Subject: [PATCH 14/14] Further simplify float-to-int range checks We don't actually need to compute the `trunc()` value, as long as we can figure out the right values for the exclusive range `(MIN-1, MAX+1)` to measure the same truncation effect. --- src/cast.rs | 53 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index 8a87008..6e89961 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -220,18 +220,23 @@ macro_rules! impl_to_primitive_float_to_signed_int { ($f:ident : $( fn $method:ident -> $i:ident ; )*) => {$( #[inline] fn $method(&self) -> Option<$i> { - let t = self.trunc(); // round toward zero. - // MIN is a power of two, which we can cast and compare directly. - if t >= $i::MIN as $f { - // The mantissa might not be able to represent all digits of MAX. - let sig_bits = size_of::<$i>() as u32 * 8 - 1; - let max = if sig_bits > $f::MANTISSA_DIGITS { - let lost_bits = sig_bits - $f::MANTISSA_DIGITS; - $i::MAX & !((1 << lost_bits) - 1) - } else { - $i::MAX - }; - if t <= max as $f { + // Float as int truncates toward zero, so we want to allow values + // in the exclusive range `(MIN-1, MAX+1)`. + if size_of::<$f>() > size_of::<$i>() { + // With a larger size, we can represent the range exactly. + const MIN_M1: $f = $i::MIN as $f - 1.0; + const MAX_P1: $f = $i::MAX as $f + 1.0; + if *self > MIN_M1 && *self < MAX_P1 { + return Some(*self as $i); + } + } else { + // We can't represent `MIN-1` exactly, but there's no fractional part + // at this magnitude, so we can just use a `MIN` inclusive boundary. + const MIN: $f = $i::MIN as $f; + // We can't represent `MAX` exactly, but it will round up to exactly + // `MAX+1` (a power of two) when we cast it. + const MAX_P1: $f = $i::MAX as $f; + if *self >= MIN && *self < MAX_P1 { return Some(*self as $i); } } @@ -244,17 +249,19 @@ macro_rules! impl_to_primitive_float_to_unsigned_int { ($f:ident : $( fn $method:ident -> $u:ident ; )*) => {$( #[inline] fn $method(&self) -> Option<$u> { - let t = self.trunc(); // round toward zero. - if t >= 0.0 { - // The mantissa might not be able to represent all digits of MAX. - let sig_bits = size_of::<$u>() as u32 * 8; - let max = if sig_bits > $f::MANTISSA_DIGITS { - let lost_bits = sig_bits - $f::MANTISSA_DIGITS; - $u::MAX & !((1 << lost_bits) - 1) - } else { - $u::MAX - }; - if t <= max as $f { + // Float as int truncates toward zero, so we want to allow values + // in the exclusive range `(-1, MAX+1)`. + if size_of::<$f>() > size_of::<$u>() { + // With a larger size, we can represent the range exactly. + const MAX_P1: $f = $u::MAX as $f + 1.0; + if *self > -1.0 && *self < MAX_P1 { + return Some(*self as $u); + } + } else { + // We can't represent `MAX` exactly, but it will round up to exactly + // `MAX+1` (a power of two) when we cast it. + const MAX_P1: $f = $u::MAX as $f; + if *self > -1.0 && *self < MAX_P1 { return Some(*self as $u); } }