rust-num / num-traits

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

Generic exp for pow and checked_pow #300

Open mtshr opened 9 months ago

mtshr commented 9 months ago

Due to the exp being usize, the current implementation has been inconsistent with core implementation of pow and checked_pow taking u32 for exp. This has also inhibited implementation of some Pow<u*> for primitive integers.

This pull request generalizes exp to use unsigned primitive integer for the sake of the implementation of lacking Pow<u*> and more importantly, I guess, the consistency with core pow and checked_pow utilizing u32, for the use of architecture dependently sized type, namely usize here, for arithmetic function does not sound reasonable.

Although this change should be a minor change as usize does implements PrimInt + Unsigned, please note that the existing crate depending on the functions in question may come to be in need of adding some type notation, as {integer} is assumed to be i32.

num_traits::pow(7, 7);         // ng
num_traits::pow(7, 7usize);    // ok
cuviper commented 9 months ago

It is an unfortunate historical difference, but I think the {integer} inference loss is probably not worth it when the more flexible Pow<RHS> already exists. Do you need an equivalent CheckedPow?

mtshr commented 9 months ago

Thank you for taking a look.

I think the {integer} inference loss is probably not worth it when the more flexible Pow<RHS> already exists.

I am sorry that my English ability is not sufficient, but am I right in interpreting you think it is too much to change exp: usize to general one and therefore are not in favor of the change of the pull request? What I give more importance here is actually not the fact that the user can implement their own pow with Pow<RHS>, but the reusability of the library's existing code and mitigating the concern about type size. After implementing Clone (and preferably Copy for the case I am describing at the moment considering the cost) + One for their own struct, the user will be able to just call num_traits::pow without writing their own pow. Also for example, for Wrapping<T> or something similar, the user can reasonably take exp >= 2^32, but always having to take usize's actual size into account leads to implementing their own pow and in the end the user cannot benefit much from pub pow.

Do you need an equivalent CheckedPow?

I know others may have a interest in one (ref #254) and I am not against the idea to add one. I consider making a pull request once the idea of generalizing exp is accepted, for I do not want to yield another FIXME.

https://github.com/rust-num/num-traits/blob/61d9a1ba1a52b3ebc7229c9a9dd5c99abc1c1e1a/src/pow.rs#L26-L29

Maybe I should have created an issue first. I apologize if discussing the idea here is quite inappropriate.