-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tracking issue for Ord::{min, max} #25663
Comments
@aturon thoughts? Seems like adding an inherent method to integer types is the easiest way forward here. Note that there are |
+1 for an inherent method. I think this is just an oversight. |
I believe @bluss commented to this effect already, but the idea here is that there is "one true I wouldn't be totally opposed to adding an inherent method, but we'd want to clearly document that it's just an alias to |
-1 for an inherent (or any other, really) method. Float’s min and max are just bindings to libm |
Ignoring the "many-ways-to-do-the-same problem" for a second, couldn't |
For me this is a practical problem, because changing type needlessly changes syntax. let a = 1.0;
let b = 2.0;
let c = a.max(b); // c = std::cmp::max(a,b) I can't change I often write performance-sensitive graphics code, which I prototype on floats for ease of writing, and then where appropriate change to use integers for speed/memory efficiency. 90% of calculations just work, and |
@rust-lang/libs: Looks like this needs some thought; I've nominated. Possibly interesting for libz blitz discussion, too, since it relates to number types in wider library set; for example I think that the things that have been brought up are:
|
We discussed this in triage today, and the libs team is in favor of adding these methods to |
[De-nominating] |
…=BurntSushi Add max and min to Ord Pursuant to issue rust-lang#25663, this PR adds max and min methods with default implementations to std::cmp::Ord. It also modifies std::cmp::max|min to internally alias to Ord::max|min, so that any overrides of the default implementations are automatically used by std::cmp::max|min. Closes rust-lang#25663
We're pretty much committed to this. @rfcbot fcp merge |
Team member @aturon has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
…xcrichton stabilized ord_max_min (fixes rust-lang#25663)
…xcrichton stabilized ord_max_min (fixes rust-lang#25663)
…xcrichton stabilized ord_max_min (fixes rust-lang#25663)
…xcrichton stabilized ord_max_min (fixes rust-lang#25663)
It's seems inconsistent that floating-point types have a
min
method, but integer types don't.And there's a standalone
min()
function that works with integer types, but it's not defined for floating-point types.I don't have a strong preference for either style, but I'd prefer that at least one way of computing min/max was available for both floating point and integer types alike.
The text was updated successfully, but these errors were encountered: