Browse Source

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.)
Josh Stone 6 years ago
parent
commit
d968efbc76
2 changed files with 54 additions and 10 deletions
  1. 15 10
      src/cast.rs
  2. 39 0
      tests/cast.rs

+ 15 - 10
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<Self> {
-        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<Self> {
-        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<Self> {
-        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<Self> {
-        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<Self> {
-        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<Self> {
-        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<Self> {
-        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<Self> {
-        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<Self> {
-        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<Self> {
-        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() }

+ 39 - 0
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>(T);
+
+    // minimal impl
+    impl<T: FromPrimitive> FromPrimitive for New<T> {
+        fn from_i64(n: i64) -> Option<Self> {
+            T::from_i64(n).map(New)
+        }
+
+        fn from_u64(n: u64) -> Option<Self> {
+            T::from_u64(n).map(New)
+        }
+    }
+
+    macro_rules! assert_eq_from {
+        ($( $from:ident )+) => {$(
+            assert_eq!(T::$from(Bounded::min_value()).map(New),
+                       New::<T>::$from(Bounded::min_value()));
+            assert_eq!(T::$from(Bounded::max_value()).map(New),
+                       New::<T>::$from(Bounded::max_value()));
+        )+}
+    }
+
+    fn check<T: PartialEq + Debug + FromPrimitive>() {
+        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);
+}