Skip to content

Commit

Permalink
Merge #73
Browse files Browse the repository at this point in the history 
73: Avoid `as` casts in default FromPrimitive methods r=cuviper a=cuviper

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.

Also make similar changes to default `ToPrimitive`, but at least it
didn't have the same casting problem.

Co-authored-by: Josh Stone <[email protected]>
  • Loading branch information
@bors @cuviper
bors[bot] and cuviper committed Jun 20, 2018
2 parents 15dc0e7 + 7140579 commit e796afc
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 35 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
10 changes: 10 additions & 0 deletions RELEASES.md
View file
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
76 changes: 42 additions & 34 deletions src/cast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,25 @@ pub trait ToPrimitive {
/// Converts the value of `self` to an `isize`.
#[inline]
fn to_isize(&self) -> Option<isize> {
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<i8> {
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<i16> {
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<i32> {
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`.
Expand All @@ -52,25 +52,25 @@ pub trait ToPrimitive {
/// Converts the value of `self` to a `usize`.
#[inline]
fn to_usize(&self) -> Option<usize> {
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<u8> {
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<u16> {
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<u32> {
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`.
Expand All @@ -92,13 +92,16 @@ pub trait ToPrimitive {
/// Converts the value of `self` to an `f32`.
#[inline]
fn to_f32(&self) -> Option<f32> {
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<f64> {
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),
}
}
}

Expand Down Expand Up @@ -352,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<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.
/// type cannot be represented by this value, then `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.
/// type cannot be represented by this value, then `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.
/// type cannot be represented by this value, then `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
/// 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<Self>;

/// 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.
///
Expand All @@ -397,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<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.
/// type cannot be represented by this value, then `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.
/// type cannot be represented by this value, then `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.
/// type cannot be represented by this value, then `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
/// 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<Self>;

/// 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.
///
Expand All @@ -442,31 +445,36 @@ 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<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.
/// type cannot be represented by this value, then `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),
}
}
}

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() }
#[inline] fn from_i64(n: i64) -> Option<$T> { n.$to_ty() }
#[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() }
Expand Down
77 changes: 77 additions & 0 deletions tests/cast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ 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};
use core::{f32, f64};
#[cfg(has_i128)]
use core::{i128, u128};

use core::fmt::Debug;
use core::mem;
use core::num::Wrapping;

Expand Down Expand Up @@ -317,3 +319,78 @@ 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);
}

#[test]
fn newtype_to_primitive() {
#[derive(PartialEq, Debug)]
struct New<T>(T);

// minimal impl
impl<T: ToPrimitive> ToPrimitive for New<T> {
fn to_i64(&self) -> Option<i64> {
self.0.to_i64()
}

fn to_u64(&self) -> Option<u64> {
self.0.to_u64()
}
}

macro_rules! assert_eq_to {
($( $to:ident )+) => {$(
assert_eq!(T::$to(&Bounded::min_value()),
New::<T>::$to(&New(Bounded::min_value())));
assert_eq!(T::$to(&Bounded::max_value()),
New::<T>::$to(&New(Bounded::max_value())));
)+}
}

fn check<T: PartialEq + Debug + Bounded + ToPrimitive>() {
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);
}

0 comments on commit e796afc

Please sign in to comment.