rust-num / num-traits

Numeric traits for generic mathematics in Rust
Apache License 2.0
732 stars 135 forks source link

Question on checked traits #179

Closed tspiteri closed 4 years ago

tspiteri commented 4 years ago

Is there a reason why checked traits are of a different form from std's unchecked traits? I mean CheckedAdd is

pub trait CheckedAdd: Sized + Add<Self, Output = Self> {
    fn checked_add(&self, v: &Self) -> Option<Self>;
}

Why isn't it something like this?

pub trait CheckedAdd<Rhs = Self>: Add<Rhs> {
    fn checked_add(self, rhs: Rhs) -> Option<Self::Output>;
}

An example use case: I was implementing some checked traits for fixed-point numbers in the fixed crate. Now for example I32F32 is a fixed-point number with 32 integer bits and 32 fractional bits, and its internal representation is an i64. It implements both Mul<I32F32> and Mul<i64>, and has respective checked methods checked_mul and checked_mul_int. It would have made sense to implement both CheckedMul and CheckedMul<i64> for that type. Note that I'm merely showing this example to show the better flexibility of the more std-like implementation.

cuviper commented 4 years ago

That's how it was in pre-1.0 std, and we never took the chance to make breaking changes and redesign things. https://doc.rust-lang.org/0.12.0/std/num/trait.CheckedAdd.html

tspiteri commented 4 years ago

Interesting. And in 0.12.0, even Add was different, with (a) the add method taking references rather than values and (b) a generic output type rather than an associated type.

In any case, probably the time to update CheckedAdd would have been around 1.0 (edit: I mean rustc 1.0), not now.

bragov4ik commented 1 year ago

Probably I miss something; why is it a breaking change? Shouldn't providing default type to Rhs cover all previous cases?

cuviper commented 1 year ago

Rhs might be ok, but dropping Sized and the Output constraint would be more problematic, because someone using where T: CheckedAdd can also rely on those properties.