`checked_sub` and `saturating_sub` for NonZero unsigned integers

libs
1

I just noticed that NonZeroU8 and friends don't have a subtraction method. I felt like they ought to have checked_sub as they have checked_add, checked_mul, and checked_pow.

Looking into it, it seems like the reason checked_sub wasn't added was because it doesn't distinguish between a None that is returned because of overflow and a None that is returned because 0 is not a valid NonZeroU8. This seems like a perfectly reasonable way to constrain the effect of that PR but it is annoying to have to write something like my_u8.get().checked_sub(42).and_then(|x| NonZeroUsize::new(x)) every time.

saturating_sub seems to have been left off for the same reason.

I'm thinking of making a RFC for this. Does anyone have any thoughts?

I think the method signatures for NonZeroU8 should be


pub const fn checked_sub(self, other: u8) -> Option<NonZeroU8>

pub const fn saturating_sub(self, other: u8) -> NonZeroU8

Obviously I would also add the equivalents for all NonZeroU* types.

I don't think the Sub trait should be implemented as the Add, Mul, and Pow traits aren't.

I could add the equivalent Div and Rem methods at the same time but I think they are very odd. Returning an Option<NonZeroU*> from one of these seems weird. My instinct is that if they existed, they would more frequently be used accidentally than legitimately. Happy to be corrected about this.

Likewise, I'm not super interested in adding these for the signed nonzero types as those don't even have addition or multiplication yet.

2 Likes
2

Shouldn't saturating_sub always succeed, but return 1 in the event of underflow?

3

Yes. Brain error on my part. Corrected.

4

This was recently discussed on URLO:

I liked @CAD97 's summary:

I personally agree that {saturating|checked}_sub, if provided, should have the definition in integer ring 1..2N. But I fail to see a meaningful benefit in breaking the current dual applicability of NonZeroUnn that allows it to be a direct substitution for uNN in a fearless compiler error driven refactor.

3 Likes
5

Aha. I hadn't seen that post. That makes a lot of sense. If I have code that uses U8 and change the type to NonZeroU8 (or vice versa) it's a very nice property that any code that still compiles will continue doing the same thing.

6

What about adding the functionality under different names?

7

This is already violated if you use MIN: NonZeroU8::MIN is 1, but u8::MIN is 0.

Nitpick: that is not a ring.

I think checked_sub should be rather uncontroversial. saturating_sub returning 1 might be more surprising.

3 Likes
8

A solution might be not naming them saturating_sub then when you refactor to NonZeroXXX while having saturating_sub calls you would get compiler errors. Those could be customized to hint towards the slightly differently named none zero equivalents.

You could call it saturating_nonzero_sub.

Another note, Saturating in std::num - Rust also misses NonZeroXXX types. Might be best to keep them out from there, that seems better to me then introducing a SaturatingNonZero struct.

1 Like
9

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.