Add `[f32]::sort_floats` and `[f64]::sort_floats` by joshtriplett · Pull Request #93397 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation36 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

It's inconvenient to sort a slice or Vec of floats, compared to sorting integers. To simplify numeric code, add a convenience method to [f32] and [f64] to sort them using sort_unstable_by with total_cmp.

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

This comment has been minimized.

They'll need distinct new lang items, just like slice_u8 for its ascii methods.

rustbot added T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

T-rustdoc

Relevant to the rustdoc team, which will review and decide on the PR/issue.

labels

Jan 28, 2022

joshtriplett added the T-libs-api

Relevant to the library API team, which will review and decide on the PR/issue.

label

Jan 28, 2022

This comment has been minimized.

Marked as cfg(not(bootstrap)) since the bootstrap compiler doesn't have these lang items.

This comment has been minimized.

...works better if I actually sort the sorted array in the right order...

This comment has been minimized.

This looks like the mir-opt segfault hitting CI for various PRs right now (#93447); waiting for that to get sorted out before retrying.

joshtriplett changed the title~~Add [f32]::sort_floats and [f64]::sort_floats~~ Add [f32]::sort and [f64]::sort

Jan 30, 2022

This comment has been minimized.

I've rebased this to eliminate the merge conflicts. Thanks to #94963 this got a lot simpler.

I continue to believe that this doesn't hide NaN behavior any more than total_cmp itself does.

However, if people feel that a name like sort_floats_total would address this concern more explicitly, I can rename the method. Would that address the objections, or are there other concerns beyond that?

Dylan-DPC added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Jul 16, 2022

I find these questionably useful, but I guess we can defer this discussion until stabilization.

@bors r+

📌 Commit bded8fc has been approved by Amanieu

It is now in the queue for this repository.

bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jul 22, 2022

Finished benchmarking commit (ed793d8): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: mixed results

mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.3%	2.3%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-3.0%	-3.0%	1
All 😿🎉 (primary)	N/A	N/A	0

Cycles

Results

Primary benchmarks: 😿 relevant regression found
Secondary benchmarks: no relevant changes found

mean1	max	count2
Regressions 😿 (primary)	3.4%	3.4%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	3.4%	3.4%	1