-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add min/max/clamp to Ord #12068
Comments
There are already |
The benefit being that floats could use fmin/fmax. |
...but we could probably just expose those directly |
I don't think floating point numbers should implement |
@thestinger Could you elaborate? I thought floating point comparison is quite well defined. |
The hardware comparison operations do not provide a total ordering. IEEE754 does provide a separate total ordering but hardware doesn't implement it and Rust doesn't use it for the comparison operators. If you can't say that |
I see your point now. There are obviously problems when you do floating points, though I'm not sure if it's wise to not implement the traits entirely. Users with experience with floating points ought to know of these drawbacks. My current (not too informed) opinion is that implementing |
Rust has |
Agreed. Should we have a separate fmin and fmax for floats? |
Yes, I think the floating point modules should define |
I think we have enough issues open about this already. There's a strong consensus that the current situation is bad but no consensus on how to deal with floating point. However, it's clear that it's unable to provide the requirements expected of |
Add .as_ref() to suggestion to remove .to_string() The case of `.to_owned().split(…)` is treated specially in the `unnecessary_to_owned` lint. Test cases check that it works both for slices and for strings, but they missed a corner case: `x.to_string().split(…)` when `x` implements `AsRef<str>` but not `Deref<Target = str>`. In this case, it is wrong to suggest to remove `.to_string()` without adding `.as_ref()` instead. Fix rust-lang#12068 changelog: [`unnecessary_to_owned`]: suggest replacing `.to_string()` by `.as_ref()`
#12061 removed Orderable, so now we need to add min/max/clamp methods back to
Ord
. Adding them right now creates a lot of ambiguous method error. This is probably best left until UFCS lands.The text was updated successfully, but these errors were encountered: