num-traits icon indicating copy to clipboard operation
num-traits copied to clipboard

`trait Euclid` takes args by reference instead of by value

Open Firestar99 opened this issue 7 months ago • 3 comments

https://github.com/rust-num/num-traits/blob/022f250f7c12955166dd2ebe776f74e1707312a2/src/ops/euclid.rs#L23

Is there any particular reason the functions of trait Euclid takes their args by reference, instead of by value? In std f32::rem_euclid also takes them by value, not reference, making them incompatible.

Also, should trait Float depend on this trait to better match std?

Firestar99 avatar Apr 13 '25 09:04 Firestar99

Is there any particular reason the functions of trait Euclid takes their args by reference, instead of by value? In std f32::rem_euclid also takes them by value, not reference, making them incompatible.

This difference occurs in a lot of our traits, and the main reason is so BigInt and BigUint (and types like them) can implement the operation without unnecessary clones, whereas primitive types can just Copy.

Also, should trait Float depend on this trait to better match std?

That would be a breaking change. If you want the Float + Euclid combination though, it's not hard to create your own new sub trait with a blanket implementation.

cuviper avatar Apr 15 '25 00:04 cuviper

I'm using it in the context of rust-gpu where we conditionally use your Float trait to implement libm functionality missing in core but present in std: (src)

#[cfg(target_arch = "spirv")]
use spirv_std::num_traits::Float;

So for us, it is really important that these match with std. Also this just feels a bit silly to write (src):

(a + d * 15.0).rem_euclid(&(d * 30.))

Could there be an option to implement rem_euclid (and maybe other std functions) in Float taking args by value, without that trait interface? Otherwise, we could keep this interface around for now

Firestar99 avatar Apr 15 '25 08:04 Firestar99

Could there be an option to implement rem_euclid (and maybe other std functions) in Float taking args by value, without that trait interface?

I think that would introduce method ambiguity if anyone is already combining Float + Euclid.

Otherwise, we could keep this interface around for now

The num-traits implementation is no better than that anyway.

cuviper avatar Apr 15 '25 16:04 cuviper