Skip to content

trait Euclid takes args by reference instead of by value #349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Firestar99 opened this issue Apr 13, 2025 · 3 comments
Open

trait Euclid takes args by reference instead of by value #349

Firestar99 opened this issue Apr 13, 2025 · 3 comments

Comments

@Firestar99
Copy link

Firestar99 commented Apr 13, 2025

fn div_euclid(&self, v: &Self) -> Self;

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?

@cuviper
Copy link
Member

cuviper commented Apr 15, 2025

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.

@Firestar99
Copy link
Author

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

@cuviper
Copy link
Member

cuviper commented Apr 15, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants