From 741e1f4e0935b787c3f9ec9b7c86e5474461bad9 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Tue, 19 Dec 2017 23:49:24 +0000 Subject: [PATCH 1/6] Add AsPrimitive trait, impls and tests --- src/cast.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/cast.rs b/src/cast.rs index 62e6bf6..881e735 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -452,6 +452,48 @@ impl NumCast for Wrapping { } } +/// A generic interface for casting between machine scalars with the +/// `as` operator, which admits narrowing and precision loss. +/// +/// # Examples +/// +/// ``` +/// # use num_traits::AsPrimitive; +/// let three: i32 = (3.14159265f32).as_(); +/// assert_eq!(three, 3); +/// ``` +pub trait AsPrimitive: 'static + Copy +where + T: 'static + Copy +{ + /// Convert a value to another, using the `as` operator. + fn as_(self) -> T; +} + +macro_rules! impl_as_primitive { + ($T: ty => $( $U: ty ),* ) => { + $( + impl AsPrimitive<$U> for $T { + #[inline] fn as_(self) -> $U { self as $U } + } + )* + }; +} + +impl_as_primitive!(u8 => char, u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(i8 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(u16 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(i16 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(u32 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(i32 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(u64 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(i64 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(usize => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(isize => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(f32 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(f64 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); +impl_as_primitive!(char => char, u8, i8, u16, i16, u32, i32, u64, isize, usize, i64); + #[test] fn to_primitive_float() { use std::f32; @@ -509,3 +551,15 @@ fn wrapping_is_numcast() { fn require_numcast(_: &T) {} require_numcast(&Wrapping(42)); } + +#[test] +fn as_primitive() { + let x: f32 = (1.625f64).as_(); + assert_eq!(x, 1.625f32); + + let x: f32 = (3.14159265358979323846f64).as_(); + assert_eq!(x, 3.1415927f32); + + let x: u8 = (768i16).as_(); + assert_eq!(x, 0); +} From 773a222237eff246f8169ccec14ba3ca6a711ceb Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Wed, 20 Dec 2017 21:41:45 +0000 Subject: [PATCH 2/6] Add conversions from `bool` in `AsPrimitive` --- src/cast.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cast.rs b/src/cast.rs index 881e735..12ad6c2 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -493,6 +493,7 @@ impl_as_primitive!(isize => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, impl_as_primitive!(f32 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); impl_as_primitive!(f64 => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64, f32, f64); impl_as_primitive!(char => char, u8, i8, u16, i16, u32, i32, u64, isize, usize, i64); +impl_as_primitive!(bool => u8, i8, u16, i16, u32, i32, u64, isize, usize, i64); #[test] fn to_primitive_float() { From af693fef480a0618d67abba7c454253122e95b17 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Thu, 21 Dec 2017 21:03:25 +0000 Subject: [PATCH 3/6] Add safety notice in AsPrimitive docs --- src/cast.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/cast.rs b/src/cast.rs index 12ad6c2..9e6974f 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -462,6 +462,28 @@ impl NumCast for Wrapping { /// let three: i32 = (3.14159265f32).as_(); /// assert_eq!(three, 3); /// ``` +/// +/// # Safety +/// +/// Currently, some uses of the `as` operator are not entirely safe. +/// In particular, it is undefined behavior if: +/// +/// - A truncated floating point value cannot fit in the target integer +/// type ([#10184](https://github.com/rust-lang/rust/issues/10184)); +/// +/// ```ignore +/// # use num_traits::AsPrimitive; +/// let x: u8 = (1.04E+17).as_(); // UB +/// ``` +/// +/// - Or a floating point value does not fit in another floating +/// point type ([#15536](https://github.com/rust-lang/rust/issues/15536)). +/// +/// ```ignore +/// # use num_traits::AsPrimitive; +/// let x: f32 = (1e300f64).as_(); // UB +/// ``` +/// pub trait AsPrimitive: 'static + Copy where T: 'static + Copy From 21dfae004cf778b0a45e9f38fb12c797f5e3d977 Mon Sep 17 00:00:00 2001 From: Fabian Schuiki Date: Wed, 3 Jan 2018 16:25:13 +0100 Subject: [PATCH 4/6] Add checked shifts Add traits `CheckedShl` and `CheckedShr` that correspond to the standard library's `checked_shl` and `checked_shr` functions. Implement the trait on all primitive integer types by default, akin to what the standard library does. The stdlib is somewhat inconsistent when it comes to the type of the shift amount. The `checked_*` functions have a `u32` shift amount, but the `std::ops::{Shl,Shr}` traits are generic over the shift amount. Also the stdlib implements these traits for all primitive integer types as right-hand sides. Our implementation mimics this behaviour. --- src/int.rs | 2 ++ src/ops/checked.rs | 53 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/int.rs b/src/int.rs index 4f9221f..18ce873 100644 --- a/src/int.rs +++ b/src/int.rs @@ -21,6 +21,8 @@ pub trait PrimInt + CheckedSub + CheckedMul + CheckedDiv + + CheckedShl + + CheckedShr + Saturating { /// Returns the number of ones in the binary representation of `self`. diff --git a/src/ops/checked.rs b/src/ops/checked.rs index 45e6716..6527b5b 100644 --- a/src/ops/checked.rs +++ b/src/ops/checked.rs @@ -1,4 +1,4 @@ -use std::ops::{Add, Sub, Mul, Div}; +use std::ops::{Add, Sub, Mul, Div, Shl, Shr}; /// Performs addition that returns `None` instead of wrapping around on /// overflow. @@ -90,3 +90,54 @@ checked_impl!(CheckedDiv, checked_div, i32); checked_impl!(CheckedDiv, checked_div, i64); checked_impl!(CheckedDiv, checked_div, isize); +/// Performs a left shift that returns `None` on overflow. +pub trait CheckedShl: Sized + Shl { + /// Shifts a number to the left, checking for overflow. If overflow happens, `None` is + /// returned. + fn checked_shl(&self, rhs: &RHS) -> Option; +} + +macro_rules! checked_shift_impl { + ($trait_name:ident, $method:ident, $rhs:ty, $t:ty) => { + impl $trait_name<$rhs> for $t { + #[inline] + fn $method(&self, rhs: &$rhs) -> Option<$t> { + // Note the cast to `u32` here: The standard library is somewhat inconsistent here. + // The `Shl` and `Shr` trait are generic over the right-hand side `T`, but + // the checked versions of the shifts all operate on `u32`. + + // TODO: Maybe we should use a conversion that can fail here. This would allow us + // to catch the case where `rhs` exceeds the `u32` accepted by the stdlib, and + // return a `None` instead. + <$t>::$method(*self, *rhs as u32) + } + } + } +} + +macro_rules! checked_shift_impl_all { + ($trait_name:ident, $method:ident, $($t:ty)*) => ($( + checked_shift_impl! { $trait_name, $method, u8 , $t } + checked_shift_impl! { $trait_name, $method, u16 , $t } + checked_shift_impl! { $trait_name, $method, u32 , $t } + checked_shift_impl! { $trait_name, $method, u64 , $t } + checked_shift_impl! { $trait_name, $method, usize, $t } + + checked_shift_impl! { $trait_name, $method, i8 , $t } + checked_shift_impl! { $trait_name, $method, i16 , $t } + checked_shift_impl! { $trait_name, $method, i32 , $t } + checked_shift_impl! { $trait_name, $method, i64 , $t } + checked_shift_impl! { $trait_name, $method, isize, $t } + )*) +} + +checked_shift_impl_all!(CheckedShl, checked_shl, u8 u16 u32 u64 usize i8 i16 i32 i64 isize); + +/// Performs a right shift that returns `None` on overflow. +pub trait CheckedShr: Sized + Shr { + /// Shifts a number to the left, checking for overflow. If overflow happens, `None` is + /// returned. + fn checked_shr(&self, rhs: &RHS) -> Option; +} + +checked_shift_impl_all!(CheckedShr, checked_shr, u8 u16 u32 u64 usize i8 i16 i32 i64 isize); From 809ccff63fab3abfa08e9c02221f06588a8f0556 Mon Sep 17 00:00:00 2001 From: Fabian Schuiki Date: Sat, 13 Jan 2018 15:02:38 +0100 Subject: [PATCH 5/6] Fix checked shift RHS to u32, drop from PrimInt Make the checked left and right shifts take a `u32` as right-hand side, which is more consistent with the other checked operations. Also drop `CheckedShl` and `CheckedShr` from the `PrimInt` trait, to not break existing code. Add doctests for the two traits. --- src/int.rs | 2 - src/ops/checked.rs | 91 ++++++++++++++++++++++++++++------------------ 2 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/int.rs b/src/int.rs index 18ce873..4f9221f 100644 --- a/src/int.rs +++ b/src/int.rs @@ -21,8 +21,6 @@ pub trait PrimInt + CheckedSub + CheckedMul + CheckedDiv - + CheckedShl - + CheckedShr + Saturating { /// Returns the number of ones in the binary representation of `self`. diff --git a/src/ops/checked.rs b/src/ops/checked.rs index 6527b5b..95214a2 100644 --- a/src/ops/checked.rs +++ b/src/ops/checked.rs @@ -91,53 +91,72 @@ checked_impl!(CheckedDiv, checked_div, i64); checked_impl!(CheckedDiv, checked_div, isize); /// Performs a left shift that returns `None` on overflow. -pub trait CheckedShl: Sized + Shl { - /// Shifts a number to the left, checking for overflow. If overflow happens, `None` is - /// returned. - fn checked_shl(&self, rhs: &RHS) -> Option; +pub trait CheckedShl: Sized + Shl { + /// Shifts a number to the left, checking for overflow. If overflow happens, + /// `None` is returned. + /// + /// ``` + /// use num_traits::CheckedShl; + /// + /// let x: u16 = 0x0001; + /// + /// assert_eq!(CheckedShl::checked_shl(&x, 0), Some(0x0001)); + /// assert_eq!(CheckedShl::checked_shl(&x, 1), Some(0x0002)); + /// assert_eq!(CheckedShl::checked_shl(&x, 15), Some(0x8000)); + /// assert_eq!(CheckedShl::checked_shl(&x, 16), None); + /// ``` + fn checked_shl(&self, rhs: u32) -> Option; } macro_rules! checked_shift_impl { - ($trait_name:ident, $method:ident, $rhs:ty, $t:ty) => { - impl $trait_name<$rhs> for $t { + ($trait_name:ident, $method:ident, $t:ty) => { + impl $trait_name for $t { #[inline] - fn $method(&self, rhs: &$rhs) -> Option<$t> { - // Note the cast to `u32` here: The standard library is somewhat inconsistent here. - // The `Shl` and `Shr` trait are generic over the right-hand side `T`, but - // the checked versions of the shifts all operate on `u32`. - - // TODO: Maybe we should use a conversion that can fail here. This would allow us - // to catch the case where `rhs` exceeds the `u32` accepted by the stdlib, and - // return a `None` instead. - <$t>::$method(*self, *rhs as u32) + fn $method(&self, rhs: u32) -> Option<$t> { + <$t>::$method(*self, rhs) } } } } -macro_rules! checked_shift_impl_all { - ($trait_name:ident, $method:ident, $($t:ty)*) => ($( - checked_shift_impl! { $trait_name, $method, u8 , $t } - checked_shift_impl! { $trait_name, $method, u16 , $t } - checked_shift_impl! { $trait_name, $method, u32 , $t } - checked_shift_impl! { $trait_name, $method, u64 , $t } - checked_shift_impl! { $trait_name, $method, usize, $t } +checked_shift_impl!(CheckedShl, checked_shl, u8); +checked_shift_impl!(CheckedShl, checked_shl, u16); +checked_shift_impl!(CheckedShl, checked_shl, u32); +checked_shift_impl!(CheckedShl, checked_shl, u64); +checked_shift_impl!(CheckedShl, checked_shl, usize); - checked_shift_impl! { $trait_name, $method, i8 , $t } - checked_shift_impl! { $trait_name, $method, i16 , $t } - checked_shift_impl! { $trait_name, $method, i32 , $t } - checked_shift_impl! { $trait_name, $method, i64 , $t } - checked_shift_impl! { $trait_name, $method, isize, $t } - )*) -} - -checked_shift_impl_all!(CheckedShl, checked_shl, u8 u16 u32 u64 usize i8 i16 i32 i64 isize); +checked_shift_impl!(CheckedShl, checked_shl, i8); +checked_shift_impl!(CheckedShl, checked_shl, i16); +checked_shift_impl!(CheckedShl, checked_shl, i32); +checked_shift_impl!(CheckedShl, checked_shl, i64); +checked_shift_impl!(CheckedShl, checked_shl, isize); /// Performs a right shift that returns `None` on overflow. -pub trait CheckedShr: Sized + Shr { - /// Shifts a number to the left, checking for overflow. If overflow happens, `None` is - /// returned. - fn checked_shr(&self, rhs: &RHS) -> Option; +pub trait CheckedShr: Sized + Shr { + /// Shifts a number to the left, checking for overflow. If overflow happens, + /// `None` is returned. + /// + /// ``` + /// use num_traits::CheckedShr; + /// + /// let x: u16 = 0x8000; + /// + /// assert_eq!(CheckedShr::checked_shr(&x, 0), Some(0x8000)); + /// assert_eq!(CheckedShr::checked_shr(&x, 1), Some(0x4000)); + /// assert_eq!(CheckedShr::checked_shr(&x, 15), Some(0x0001)); + /// assert_eq!(CheckedShr::checked_shr(&x, 16), None); + /// ``` + fn checked_shr(&self, rhs: u32) -> Option; } -checked_shift_impl_all!(CheckedShr, checked_shr, u8 u16 u32 u64 usize i8 i16 i32 i64 isize); +checked_shift_impl!(CheckedShr, checked_shr, u8); +checked_shift_impl!(CheckedShr, checked_shr, u16); +checked_shift_impl!(CheckedShr, checked_shr, u32); +checked_shift_impl!(CheckedShr, checked_shr, u64); +checked_shift_impl!(CheckedShr, checked_shr, usize); + +checked_shift_impl!(CheckedShr, checked_shr, i8); +checked_shift_impl!(CheckedShr, checked_shr, i16); +checked_shift_impl!(CheckedShr, checked_shr, i32); +checked_shift_impl!(CheckedShr, checked_shr, i64); +checked_shift_impl!(CheckedShr, checked_shr, isize); From 31218add95ce6c94234be2364e7ebbd276101ab9 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sun, 14 Jan 2018 21:23:19 +0000 Subject: [PATCH 6/6] Include note for implementers of AsPrimitive --- src/cast.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cast.rs b/src/cast.rs index 9e6974f..2d7fe19 100644 --- a/src/cast.rs +++ b/src/cast.rs @@ -454,6 +454,9 @@ impl NumCast for Wrapping { /// A generic interface for casting between machine scalars with the /// `as` operator, which admits narrowing and precision loss. +/// Implementers of this trait AsPrimitive should behave like a primitive +/// numeric type (e.g. a newtype around another primitive), and the +/// intended conversion must never fail. /// /// # Examples ///