Skip to content
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

Make stable_sort_primitive message less authoritative #8241

Closed
jongiddy opened this issue Jan 8, 2022 · 2 comments · Fixed by #8716
Closed

Make stable_sort_primitive message less authoritative #8241

jongiddy opened this issue Jan 8, 2022 · 2 comments · Fixed by #8716
Assignees
Labels
A-category Area: Categorization of lints A-documentation Area: Adding or improving documentation C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy

Comments

@jongiddy
Copy link

jongiddy commented Jan 8, 2022

Description

The current message for stable_sort_primitive says:

an unstable sort would perform faster without any observable difference for this data type
for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive

Although it is a reasonable guideline and typically true for random data, it is not always true. For example, the append benchmark in https://gist.github.com/jongiddy/b29ce7d2251775dcf24c595bc245891b demonstrates that stable sort can be significantly faster for a scenario where a sorted vector is extended with new data and resorted.

running 6 tests
test almost_sorted_stable   ... bench:      12,290 ns/iter (+/- 97)
test almost_sorted_unstable ... bench:       3,392 ns/iter (+/- 74)
test append_stable          ... bench:       2,545 ns/iter (+/- 19)
test append_unstable        ... bench:       8,450 ns/iter (+/- 236)
test random_stable          ... bench:      16,803 ns/iter (+/- 396)
test random_unstable        ... bench:       6,149 ns/iter (+/- 156)

Because of this, the initial message should be less definite:

an unstable sort typically performs faster without any observable difference for this data type
for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive

and the linked message should also be less definite, and suggest benchmarking specific situations.

Version

No response

Additional Labels

No response

@xFrednet xFrednet added C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy labels Jan 8, 2022
@camsteffen
Copy link
Contributor

Bummer, that also makes it a false positive and may warrant moving the lint to pedantic.

@camsteffen camsteffen added A-category Area: Categorization of lints A-documentation Area: Adding or improving documentation labels Jan 11, 2022
@spiral-ladder
Copy link
Contributor

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-category Area: Categorization of lints A-documentation Area: Adding or improving documentation C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants