rust-num / num-traits

Numeric traits for generic mathematics in Rust
Apache License 2.0
694 stars 131 forks source link

Add CheckedNumOps #314

Open cod10129 opened 5 months ago

cod10129 commented 5 months ago

Working with numerics in num-traits is biased towards using the standard traits, which is generally good. However, checked arithmetic is lacking a unifying trait (like NumOps). This PR adds that. This trait should theoretically be a supertrait of PrimInt. However, this trait (like NumOps: Rem), requires CheckedRem. PrimInt does not currently require that, meaning that this would be a breaking change. (PrimInt: CheckedNumOps for a future release?)

cuviper commented 4 months ago

This trait should theoretically be a supertrait of PrimInt. However, this trait (like NumOps: Rem), requires CheckedRem. PrimInt does not currently require that, meaning that this would be a breaking change. (PrimInt: CheckedNumOps for a future release?)

I think it's just that CheckedRem didn't exist at the time. Yes, it would make sense, but also yes, it would be a breaking change.

You did not actually include CheckedNumOps: CheckedRem though -- did you mean to?

cod10129 commented 3 months ago

I wrote all that out about it and forgot to actually require CheckedRem. I think my idea was that I'm not sure what should happen with it. CheckedNumOps could not require CheckedRem and be a supertrait of PrimInt, or it could require CheckedRem and be disconnected from PrimInt. I wasn't sure on the best course of action, so I think that's why I left CheckedRem out. However, now I think that using CheckedRem is the right way to go. If you don't think so you can roll back the most recent commit to the fork, which actually adds CheckedRem.