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

Remove modes from the compiler #6078

Closed
wants to merge 4 commits into from
Closed

Conversation

pcwalton
Copy link
Contributor

r? @brson

@pcwalton pcwalton closed this Apr 27, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 9, 2020
…hansch

unnecessary sort by: avoid dereferencing the suggested closure parameter

This change tries to simplify the solution for problematic cases but is less restrictive than  rust-lang#6006.

* We can't dereference shared references to non-Copy types, so the new suggestion does not do that. Note that this implies that the suggested closure parameter will be a reference.
* We can't take a reference to the closure parameter in the returned key, so we don't lint in those cases. This can happen either because the key borrows from the parameter (e.g. `|a| a.borrows()`), or because we suggest `|a| Reverse(a)`. If we did we would hit this error:
```
error: lifetime may not live long enough
  --> /home/ebroto/src/ebroto-clippy/tests/ui/unnecessary_sort_by.fixed:19:25
   |
19 |     vec.sort_by_key(|b| Reverse(b));
   |                      -- ^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
   |                      ||
   |                      |return type of closure is Reverse<&'2 isize>
   |                      has type `&'1 isize`

error: aborting due to previous error
```

Note that Clippy does not currently have the (MIR-based) machinery necessary to check that what is borrowed is actually the closure parameter.

changelog: [`unnecessary_sort_by`]: avoid dereferencing the suggested closure parameter

Fixes rust-lang#6001
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants