Implemented Zero, One and Num for std::num::Wrapping<T> #279
Conversation
| @@ -243,9 +256,9 @@ float_trait_impl!(Num for f32 f64); | |||
|
|
|||
| /// A value bounded by a minimum and a maximum | |||
| /// | |||
| /// If input is less than min then this returns min. | |||
There was a problem hiding this comment.
I have no idea what caused these changes - didn't do these on purpose anyway.
There was a problem hiding this comment.
I guess your editor is set to strip trailing whitespace. That's OK.
traits/src/identities.rs
Outdated
| @@ -50,6 +51,16 @@ zero_impl!(i64, 0i64); | |||
| zero_impl!(f32, 0.0f32); | |||
| zero_impl!(f64, 0.0f64); | |||
|
|
|||
| impl<T: Zero + PartialEq> Zero for Wrapping<T> where Wrapping<T>: Add<Output=Wrapping<T>> { | |||
There was a problem hiding this comment.
Hmm. I was worried that a generic impl would be a breaking change, in case someone had already implemented it for their own Foo and Wrapping<Foo>, but it looks like they couldn't do that:
|
115 | impl Zero for Wrapping<Foo> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate
|
= note: the impl does not reference any types defined in this crate
= note: define and implement a trait or new type instead
... even though Foo is defined on the line right before. I always forget exactly how coherence works. I think you can have this overlap with users if the custom type were a trait parameter instead.
Anyway, looks like this is OK.
traits/src/identities.rs
Outdated
| @@ -50,6 +51,16 @@ zero_impl!(i64, 0i64); | |||
| zero_impl!(f32, 0.0f32); | |||
| zero_impl!(f64, 0.0f64); | |||
|
|
|||
| impl<T: Zero + PartialEq> Zero for Wrapping<T> where Wrapping<T>: Add<Output=Wrapping<T>> { | |||
| fn is_zero(&self) -> bool { | |||
| self.0 == T::zero() | |||
There was a problem hiding this comment.
Should use self.0.is_zero() here.
traits/src/lib.rs
Outdated
| { | ||
| type FromStrRadixErr = T::FromStrRadixErr; | ||
| fn from_str_radix(str: &str, radix: u32) -> Result<Self, Self::FromStrRadixErr> { | ||
| T::from_str_radix(str, radix).map(|x| Wrapping(x)) |
There was a problem hiding this comment.
I think this can just be map(Wrapping), but it doesn't make much difference.
There was a problem hiding this comment.
Wow, that's a pretty nice trick!
| @@ -243,9 +256,9 @@ float_trait_impl!(Num for f32 f64); | |||
|
|
|||
| /// A value bounded by a minimum and a maximum | |||
| /// | |||
| /// If input is less than min then this returns min. | |||
There was a problem hiding this comment.
I guess your editor is set to strip trailing whitespace. That's OK.
traits/src/identities.rs
Outdated
| @@ -94,6 +105,11 @@ one_impl!(i64, 1i64); | |||
| one_impl!(f32, 1.0f32); | |||
| one_impl!(f64, 1.0f64); | |||
|
|
|||
| impl<T: One> One for Wrapping<T> where Wrapping<T>: Add<Output=Wrapping<T>> + Mul<Output=Wrapping<T>> { | |||
There was a problem hiding this comment.
I think this only needs Mul, not Add.
There was a problem hiding this comment.
I vaguely remember that I had to put it for some reason, but now it should indeed be removed.
|
⚡ Test exempted - status |
Implemented Zero, One and Num for std::num::Wrapping<T> Attempts to fix issue #278
impl remaining num-traits for std::num::Wrapping<T> This is a (late) follow-up for [https://github.com/rust-num/num/pull/279](https://github.com/rust-num/num/pull/279) since I realized that implementing `Num` for `Wrapping<T>` was merely half of the work. This PR makes `Wrapping<T>` implement the remaining appropriate traits, granting it the ability to really be used a complete substitute for its primitive integer counterparts. Some benefits are : - Less refactoring for users using `num`'s traits replacing some primitives by their `Wrapping` counterpart (same for the opposite); - Since `Wrapping<T>` is from `std`, nobody except us can `impl` our traits for it, so people don't have to create their own.
Attempts to fix issue #278