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

False positive for unnecessary_sort_by lint: Suggestion does not compile #5754

Closed
psychon opened this issue Jun 27, 2020 · 1 comment · Fixed by #5756
Closed

False positive for unnecessary_sort_by lint: Suggestion does not compile #5754

psychon opened this issue Jun 27, 2020 · 1 comment · Fixed by #5756
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@psychon
Copy link

psychon commented Jun 27, 2020

I tried this code:

#[derive(Debug)]
pub struct Test {
    name: String,
}

impl Test {
    fn name(&self) -> &str {
        &self.name
    }
}

fn main() {
    let mut args: Vec<_> = std::env::args().map(|name| Test { name }).collect();
    args.sort_by(|a, b| a.name().cmp(b.name()));
    // Suggested by clippy, but causes a lifetime error:
    // args.sort_by_key(|a| a.name());
    println!("{:?}", args);
}

Clippy says:

    Checking playground v0.0.1 (/playground)
warning: use Vec::sort_by_key here instead
  --> src/main.rs:14:5
   |
14 |     args.sort_by(|a, b| a.name().cmp(b.name()));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|&a| a.name())`
   |
   = note: `#[warn(clippy::unnecessary_sort_by)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.33s

When applying the suggestion (= uncommenting the marked line in the code), the following happens:

    Checking playground v0.0.1 (/playground)
error: lifetime may not live long enough
  --> src/main.rs:16:26
   |
16 |     args.sort_by_key(|a| a.name());
   |                       -- ^^^^^^^^ returning this value requires that `'1` must outlive `'2`
   |                       ||
   |                       |return type of closure is &'2 str
   |                       has type `&'1 Test`

error: aborting due to previous error

error: could not compile `playground`.

To learn more, run the command again with --verbose.

Meta

No idea. This happens on the Rust playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=732732b92adbc28314716081acc4e733

The playground says this is clippy 0.0.212 (2020-06-26 7750c3d) and I have no idea were to find the rustc version. Sorry.

I cannot reproduce locally with clippy 0.0.212 (d4092ac 2020-05-11).

@psychon psychon added the C-bug Category: Clippy is not doing the correct thing label Jun 27, 2020
@phansch phansch added L-suggestion Lint: Improving, adding or fixing lint suggestions I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied labels Jun 28, 2020
@phansch
Copy link
Member

phansch commented Jun 28, 2020

Thanks for the report, this is certainly a bug in Clippy.

There's an old open rust issue about fixing the differences between sort_by and sort_by_key: rust-lang/rust#34162 about exactly your case. However, that requires more fundamental work, so we should fix this bug in Clippy first.

bors added a commit that referenced this issue Jul 3, 2020
unnecessary_sort_by: avoid linting if key borrows

changelog: Avoid linting if key borrows in [`unnecessary_sort_by`]

Fixes #5754
Closes #2313
@bors bors closed this as completed in 32ef448 Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants