impl remaining num-traits for std::num::Wrapping<T>

[yoanlcq]

This is a (late) follow-up for #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.

[cuviper]

I'm with you on Signed, Unsigned, Bounded, ToPrimitive, FromPrimitive, and NumCast. WrappingAdd et al. seem silly, but I can buy it. But I'm less convinced about the rest, because you're treating Wrapped<T> in a way that's not supported by the standard library.

The #[inline] hints shouldn't be necessary in generic code like this, as codegen waits until it's monomorphized anyway.

We need tests for the stuff you're adding too. (I should have asked for that on the Num addition.)

[yoanlcq]

Got it - I'll rollback PrimInt, Checked and Saturating, remove inline hints and add tests.

In the meantime, I'd like to understand what you meant by "treating Wrapped in a way that's not supported by the standard library" - AFAIK the doc only says that "all standard arithmetic operations on the underlying value are intended to have wrapping semantics" and I don't see how that would be incompatible with PrimInt, Checked* and Saturating*. Would you mind elaborating a bit ? I don't intend to argue.

About the tests, I'll give them a honest try, looking at what's already there, but if you have specific expectations in the meantime I'd be happy to hear them.

[cuviper]

In the meantime, I'd like to understand what you meant by "treating Wrapped in a way that's not supported by the standard library" - AFAIK the doc only says that "all standard arithmetic operations on the underlying value are intended to have wrapping semantics" and I don't see how that would be incompatible with PrimInt, Checked* and Saturating*. Would you mind elaborating a bit ? I don't intend to argue.

It's OK to argue a little -- if I'm wrong, someone should convince me of the right way. :)

To me, the problem is that the standard Wrapping<T> doesn't implement checked or saturating operations itself, and it's not clear at all to me that passing those operations down to the underlying type is the correct thing to do. For instance, one might expect that CheckedAdd should always return Some, because there's no failing semantics for wrapped types. Similarly, I would expect a Wrapping<T>::pow() to just wrap around freely. But one can always use wrapped.0 to get that inner type if they really want the non-wrapping semantics.

Yes, this means you can't pass a Wrapping<T> to generic code that takes Checked ops et al. This is a more conservative position, but I think that's OK. We can remain open to reconsider that in the future.

About the tests, I'll give them a honest try, looking at what's already there, but if you have specific expectations in the meantime I'd be happy to hear them.

I had no particular expectations beyond trying to exercise to code in some way at all. If we don't even try it, we could find that we were missing some conditional trait bounds that made them ineffective, for example.

[yoanlcq]

I see your point, and I'm fine with this since the point of this PR is merely to make Wrapping<T>s a bit more convenient for use in generic code - as you said, one can always fall back to wrapped.0.checked_add(...) which makes the intent clear at the (negligible) cost of potentially being a bit annoying (perhaps by requiring some refactoring, etc), and it's okay for my use cases which are pretty niche to start with.

If you are fine with the tests (which I can't tell are overdone or not enough), I think this is ready to be merged.

And just for future discussion on the matter, here's my opinion the unimplemented traits :

• I think it's fine for Wrapping to implement Checked* by passing operations down to the underlying type, to support users who would use Wrapping to have wrapping semantics by default and explicitly perform "checked" ops at times. In other words, I think we shouldn't assume that people would want to purposefully inhibit checked ops when using Wrapping.
• Same for Saturating*, for the same reasons as above;
• Same for PrimInt, because Wrappings are release-mode primitive integers (after all, they made it to libcore). However I agree that <Wrapping as PrimInt>::pow() should too have wrapping semantics instead of being just self.0.pow(). For this, looks like a good start would be to grab i32::pow()'s source and then test.

[cuviper]

This looks very nice, thanks!

@homu r+

[homu]

📌 Commit 9115df6 has been approved by cuviper

[homu]

⌛ Testing commit 9115df6 with merge ef08fe2...

[homu]

☀️ Test successful - status