From d968efbc76639a89ae649c377375afdfb2686dd3 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 20 Jun 2018 13:05:03 -0700 Subject: [PATCH 1/5] Avoid `as` casts in default FromPrimitive methods Particularly, the default `from_f64` used `n as i64`, which has undefined behavior on overflow, kind of defeating the purpose here. Now we use a checked `to_i64()` for this, and even try `to_u64()` as a fallback for completeness. (All of the primitive implementations already do better, at least.) --- src/cast.rs | 25 +++++++++++++++---------- tests/cast.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index a8c8484..eb1fa70 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -355,28 +355,28 @@ pub trait FromPrimitive: Sized { /// value cannot be represented by this value, the `None` is returned. #[inline] fn from_isize(n: isize) -> Option { - FromPrimitive::from_i64(n as i64) + n.to_i64().and_then(FromPrimitive::from_i64) } /// Convert an `i8` to return an optional value of this type. If the /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_i8(n: i8) -> Option { - FromPrimitive::from_i64(n as i64) + FromPrimitive::from_i64(From::from(n)) } /// Convert an `i16` to return an optional value of this type. If the /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_i16(n: i16) -> Option { - FromPrimitive::from_i64(n as i64) + FromPrimitive::from_i64(From::from(n)) } /// Convert an `i32` to return an optional value of this type. If the /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_i32(n: i32) -> Option { - FromPrimitive::from_i64(n as i64) + FromPrimitive::from_i64(From::from(n)) } /// Convert an `i64` to return an optional value of this type. If the @@ -400,28 +400,28 @@ pub trait FromPrimitive: Sized { /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_usize(n: usize) -> Option { - FromPrimitive::from_u64(n as u64) + n.to_u64().and_then(FromPrimitive::from_u64) } /// Convert an `u8` to return an optional value of this type. If the /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_u8(n: u8) -> Option { - FromPrimitive::from_u64(n as u64) + FromPrimitive::from_u64(From::from(n)) } /// Convert an `u16` to return an optional value of this type. If the /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_u16(n: u16) -> Option { - FromPrimitive::from_u64(n as u64) + FromPrimitive::from_u64(From::from(n)) } /// Convert an `u32` to return an optional value of this type. If the /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_u32(n: u32) -> Option { - FromPrimitive::from_u64(n as u64) + FromPrimitive::from_u64(From::from(n)) } /// Convert an `u64` to return an optional value of this type. If the @@ -445,14 +445,17 @@ pub trait FromPrimitive: Sized { /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_f32(n: f32) -> Option { - FromPrimitive::from_f64(n as f64) + FromPrimitive::from_f64(From::from(n)) } /// Convert a `f64` to return an optional value of this type. If the /// type cannot be represented by this value, the `None` is returned. #[inline] fn from_f64(n: f64) -> Option { - FromPrimitive::from_i64(n as i64) + match n.to_i64() { + Some(i) => FromPrimitive::from_i64(i), + None => n.to_u64().and_then(FromPrimitive::from_u64), + } } } @@ -460,6 +463,7 @@ macro_rules! impl_from_primitive { ($T:ty, $to_ty:ident) => ( #[allow(deprecated)] impl FromPrimitive for $T { + #[inline] fn from_isize(n: isize) -> Option<$T> { n.$to_ty() } #[inline] fn from_i8(n: i8) -> Option<$T> { n.$to_ty() } #[inline] fn from_i16(n: i16) -> Option<$T> { n.$to_ty() } #[inline] fn from_i32(n: i32) -> Option<$T> { n.$to_ty() } @@ -467,6 +471,7 @@ macro_rules! impl_from_primitive { #[cfg(has_i128)] #[inline] fn from_i128(n: i128) -> Option<$T> { n.$to_ty() } + #[inline] fn from_usize(n: usize) -> Option<$T> { n.$to_ty() } #[inline] fn from_u8(n: u8) -> Option<$T> { n.$to_ty() } #[inline] fn from_u16(n: u16) -> Option<$T> { n.$to_ty() } #[inline] fn from_u32(n: u32) -> Option<$T> { n.$to_ty() } diff --git a/tests/cast.rs b/tests/cast.rs index edac185..fd248de 100644 --- a/tests/cast.rs +++ b/tests/cast.rs @@ -9,6 +9,7 @@ extern crate std; extern crate num_traits; use num_traits::cast::*; +use num_traits::Bounded; use core::{i8, i16, i32, i64, isize}; use core::{u8, u16, u32, u64, usize}; @@ -16,6 +17,7 @@ use core::{f32, f64}; #[cfg(has_i128)] use core::{i128, u128}; +use core::fmt::Debug; use core::mem; use core::num::Wrapping; @@ -317,3 +319,40 @@ fn cast_int_to_128_edge_cases() { test_edge!(usize u8 u16 u32 u64 u128); } +#[test] +fn newtype_from_primitive() { + #[derive(PartialEq, Debug)] + struct New(T); + + // minimal impl + impl FromPrimitive for New { + fn from_i64(n: i64) -> Option { + T::from_i64(n).map(New) + } + + fn from_u64(n: u64) -> Option { + T::from_u64(n).map(New) + } + } + + macro_rules! assert_eq_from { + ($( $from:ident )+) => {$( + assert_eq!(T::$from(Bounded::min_value()).map(New), + New::::$from(Bounded::min_value())); + assert_eq!(T::$from(Bounded::max_value()).map(New), + New::::$from(Bounded::max_value())); + )+} + } + + fn check() { + assert_eq_from!(from_i8 from_i16 from_i32 from_i64 from_isize); + assert_eq_from!(from_u8 from_u16 from_u32 from_u64 from_usize); + assert_eq_from!(from_f32 from_f64); + } + + macro_rules! check { + ($( $ty:ty )+) => {$( check::<$ty>(); )+} + } + check!(i8 i16 i32 i64 isize); + check!(u8 u16 u32 u64 usize); +} From dd7900d62fc28e0aecb5d643f442b2cff58e5403 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 20 Jun 2018 13:10:41 -0700 Subject: [PATCH 2/5] Avoid closures in default `ToPrimitive` methods In `to_f64()`, we also try `to_u64()` if `to_i64()` failed. --- src/cast.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index eb1fa70..e10b5e5 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -13,25 +13,25 @@ pub trait ToPrimitive { /// Converts the value of `self` to an `isize`. #[inline] fn to_isize(&self) -> Option { - self.to_i64().and_then(|x| x.to_isize()) + self.to_i64().as_ref().and_then(ToPrimitive::to_isize) } /// Converts the value of `self` to an `i8`. #[inline] fn to_i8(&self) -> Option { - self.to_i64().and_then(|x| x.to_i8()) + self.to_i64().as_ref().and_then(ToPrimitive::to_i8) } /// Converts the value of `self` to an `i16`. #[inline] fn to_i16(&self) -> Option { - self.to_i64().and_then(|x| x.to_i16()) + self.to_i64().as_ref().and_then(ToPrimitive::to_i16) } /// Converts the value of `self` to an `i32`. #[inline] fn to_i32(&self) -> Option { - self.to_i64().and_then(|x| x.to_i32()) + self.to_i64().as_ref().and_then(ToPrimitive::to_i32) } /// Converts the value of `self` to an `i64`. @@ -52,25 +52,25 @@ pub trait ToPrimitive { /// Converts the value of `self` to a `usize`. #[inline] fn to_usize(&self) -> Option { - self.to_u64().and_then(|x| x.to_usize()) + self.to_u64().as_ref().and_then(ToPrimitive::to_usize) } /// Converts the value of `self` to an `u8`. #[inline] fn to_u8(&self) -> Option { - self.to_u64().and_then(|x| x.to_u8()) + self.to_u64().as_ref().and_then(ToPrimitive::to_u8) } /// Converts the value of `self` to an `u16`. #[inline] fn to_u16(&self) -> Option { - self.to_u64().and_then(|x| x.to_u16()) + self.to_u64().as_ref().and_then(ToPrimitive::to_u16) } /// Converts the value of `self` to an `u32`. #[inline] fn to_u32(&self) -> Option { - self.to_u64().and_then(|x| x.to_u32()) + self.to_u64().as_ref().and_then(ToPrimitive::to_u32) } /// Converts the value of `self` to an `u64`. @@ -92,13 +92,16 @@ pub trait ToPrimitive { /// Converts the value of `self` to an `f32`. #[inline] fn to_f32(&self) -> Option { - self.to_f64().and_then(|x| x.to_f32()) + self.to_f64().as_ref().and_then(ToPrimitive::to_f32) } /// Converts the value of `self` to an `f64`. #[inline] fn to_f64(&self) -> Option { - self.to_i64().and_then(|x| x.to_f64()) + match self.to_i64() { + Some(i) => i.to_f64(), + None => self.to_u64().as_ref().and_then(ToPrimitive::to_f64), + } } } From 21e3620999be417841aefb1c312fdaf0b7381867 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 20 Jun 2018 13:39:08 -0700 Subject: [PATCH 3/5] doc: fix a typo, s/the/then/ --- src/cast.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/cast.rs b/src/cast.rs index e10b5e5..c692df8 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -355,39 +355,39 @@ impl_to_primitive_float!(f64); /// A generic trait for converting a number to a value. pub trait FromPrimitive: Sized { /// Convert an `isize` to return an optional value of this type. If the - /// value cannot be represented by this value, the `None` is returned. + /// value cannot be represented by this value, then `None` is returned. #[inline] fn from_isize(n: isize) -> Option { n.to_i64().and_then(FromPrimitive::from_i64) } /// Convert an `i8` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_i8(n: i8) -> Option { FromPrimitive::from_i64(From::from(n)) } /// Convert an `i16` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_i16(n: i16) -> Option { FromPrimitive::from_i64(From::from(n)) } /// Convert an `i32` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_i32(n: i32) -> Option { FromPrimitive::from_i64(From::from(n)) } /// Convert an `i64` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. fn from_i64(n: i64) -> Option; /// Convert an `i128` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. /// /// This method is only available with feature `i128` enabled on Rust >= 1.26. /// @@ -400,39 +400,39 @@ pub trait FromPrimitive: Sized { } /// Convert a `usize` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_usize(n: usize) -> Option { n.to_u64().and_then(FromPrimitive::from_u64) } /// Convert an `u8` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_u8(n: u8) -> Option { FromPrimitive::from_u64(From::from(n)) } /// Convert an `u16` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_u16(n: u16) -> Option { FromPrimitive::from_u64(From::from(n)) } /// Convert an `u32` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_u32(n: u32) -> Option { FromPrimitive::from_u64(From::from(n)) } /// Convert an `u64` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. fn from_u64(n: u64) -> Option; /// Convert an `u128` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. /// /// This method is only available with feature `i128` enabled on Rust >= 1.26. /// @@ -445,14 +445,14 @@ pub trait FromPrimitive: Sized { } /// Convert a `f32` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_f32(n: f32) -> Option { FromPrimitive::from_f64(From::from(n)) } /// Convert a `f64` to return an optional value of this type. If the - /// type cannot be represented by this value, the `None` is returned. + /// type cannot be represented by this value, then `None` is returned. #[inline] fn from_f64(n: f64) -> Option { match n.to_i64() { From 60924ecc70749601995e1a8b10285d8e7267e894 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 20 Jun 2018 13:49:57 -0700 Subject: [PATCH 4/5] add test newtype_to_primitive --- tests/cast.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/cast.rs b/tests/cast.rs index fd248de..63643eb 100644 --- a/tests/cast.rs +++ b/tests/cast.rs @@ -356,3 +356,41 @@ fn newtype_from_primitive() { check!(i8 i16 i32 i64 isize); check!(u8 u16 u32 u64 usize); } + +#[test] +fn newtype_to_primitive() { + #[derive(PartialEq, Debug)] + struct New(T); + + // minimal impl + impl ToPrimitive for New { + fn to_i64(&self) -> Option { + self.0.to_i64() + } + + fn to_u64(&self) -> Option { + self.0.to_u64() + } + } + + macro_rules! assert_eq_to { + ($( $to:ident )+) => {$( + assert_eq!(T::$to(&Bounded::min_value()), + New::::$to(&New(Bounded::min_value()))); + assert_eq!(T::$to(&Bounded::max_value()), + New::::$to(&New(Bounded::max_value()))); + )+} + } + + fn check() { + assert_eq_to!(to_i8 to_i16 to_i32 to_i64 to_isize); + assert_eq_to!(to_u8 to_u16 to_u32 to_u64 to_usize); + assert_eq_to!(to_f32 to_f64); + } + + macro_rules! check { + ($( $ty:ty )+) => {$( check::<$ty>(); )+} + } + check!(i8 i16 i32 i64 isize); + check!(u8 u16 u32 u64 usize); +} From 714057979ec4819327a4d27c8e9da6c56bd81488 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 20 Jun 2018 14:24:56 -0700 Subject: [PATCH 5/5] Release 0.2.5 --- Cargo.toml | 2 +- RELEASES.md | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5a15f5f..7861af5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ categories = ["algorithms", "science", "no-std"] license = "MIT/Apache-2.0" repository = "https://github.com/rust-num/num-traits" name = "num-traits" -version = "0.2.4" +version = "0.2.5" readme = "README.md" build = "build.rs" diff --git a/RELEASES.md b/RELEASES.md index cbf64da..a7bf787 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,3 +1,13 @@ +# Release 0.2.5 + +- [Documentation for `mul_add` now clarifies that it's not always faster.][70] +- [The default methods in `FromPrimitive` and `ToPrimitive` are more robust.][73] + +**Contributors**: @cuviper, @frewsxcv + +[70]: https://github.com/rust-num/num-traits/pull/70 +[73]: https://github.com/rust-num/num-traits/pull/73 + # Release 0.2.4 - [Support for 128-bit integers is now automatically detected and enabled.][69]