Tracking Issue for sort_floats

[joshtriplett]

Feature gate: #![feature(sort_floats)]

This is a tracking issue for the sort_floats method on [f32] and [f64], a convenience method to sort a slice of floats by calling sort_unstable_by using total_cmp.

Public API

impl [f32] {
    pub fn sort_floats(&mut self);
}

impl [f64] {
    pub fn sort_floats(&mut self);
}

Steps / History

• Implementation: Add [f32]::sort_floats and [f64]::sort_floats #93397
• Final comment period (FCP)
• Stabilization PR

Open questions

• Should we rename this to sort, which would require some additional steps (possibly negative impls for Ord on f32 and f64) to avoid a name conflict?

[clarfonthey]

I feel like total_sort would be a better name, since it matches the total_cmp naming and makes it clear what it does.

[Riolku]

I don't think these are useful. Surely it's not very hard to use sort_unstable_by(f64::total_cmp).

I think, although longer, forcing the long usage makes users aware of the possible issues, instead of sweeping them under the rug.

As for name, I think total_float_sort would be more sensible maybe?

[thomcc]

As mentioned in rust-lang/unsafe-code-guidelines#237 (comment), we probably shouldn't stabilize the current implementation of this at the moment -- it looks increasingly likely that we're going to have to say that the sign bit of NaN is fundamentally non-deterministic, at least in the short term.

A sorting function that puts NaNs non-deterministically at the start or end of the slice is probably far too error-prone for us to want to stabilize. (I probably should have pushed back harder than I did on total_cmp too, but I expected the non-determinism around NaN sign bit to be a bug we eventually fix, rather than something we have to live with indefinitely).

[shssoichiro]

I just learned the existence of this... and while I like the idea of having this capability, just tacking on a separate method called "sort_floats" feels very unwieldy. It feels like it would be much more ergonomic to do something like adding an implementation of sort for PartialOrd. As a developer, the idea that I have to be aware of a separate method specifically for sorting floats is quite unintuitive.

[kornelski]

I don't like that it uses total_cmp. This is already easy to do with the existing sort_by.

If there's going to be a dedicated method, I'd like one that is optimized for finite floats which can use the cheapest comparison available, ignoring what that means for NaN and other float edge cases. As long as it beats hacks like .sort_by(|a, b| a.partial_cmp(b).unwrap_or(Equal)) or .sort_by(|a, b| if a < b { Less } else { Greater }) on speed, correctness or stability.

Also it should be .sort_floats_by_key() to be able to handle arbitrary structs. I rarely have [f32], but very often [(f32, str)] or .sort_by_key(|item| item.score()).

[joshtriplett]

We discussed this in today's @rust-lang/libs-api meeting.

We agreed that we don't want to ship these as-is. Instead, we'd love to add OrderedFloat to the standard library, pretty much as implemented in crates in the community.

One of the main arguments for doing so was the ability to handle tuples and similar.