Skip to content

Conversation

@Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Apr 30, 2025

Based on #14702

This should generally be a perf improvement. The old implementation allocated a lot of things and then did multiple passes over the data. This only does a single pass allocating significantly less. It also bails out earlier in most cases.

Some behaviour changes:

  • An ICE was fixed with debug assertions where removing multiple lifetime arguments could create overlapping spans. These spans are now merged into a single span.
  • The lint message has been changed in the first commit. Adding all the lifetime names to the message is pointless when the lint spans already highlights those.
  • The main lint span now no longer points to all uses. The suggestion already makes it clear what needs to change, no need to make the main lint span messy. It was especially bad when the signature was split over multiple lines.
  • The help message was change to be a little more direct about what gets removed, but I'm not really attached either way.
  • The lifetime in dyn Trait + 'a is now elided when possible instead of completely bailing out of the lint.
  • Lifetimes appearing in various spots no longer completely bails out. Instead this leaves just those lifetimes alone.
  • There is a regression for <'a, T: 'a> where 'a is only used for the type outlives constraint. Theses lifetimes can be removed, but I'd rather handle that at the same time as handling outlives constraints on lifetimes.

changelog: [elidable_lifetime_names]: Lint when dyn Trait + 'a can be elided.
changelog: [needless_lifetimes] [elidable_lifetime_names]: Lint more cases when only some lifetimes can be elided.

@rustbot
Copy link
Collaborator

rustbot commented Apr 30, 2025

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 1, 2025

☔ The latest upstream changes (possibly 03a5b6b) made this pull request unmergeable. Please resolve the merge conflicts.

@dswij
Copy link
Member

dswij commented Jun 18, 2025

r? clippy

@rustbot rustbot assigned llogiq and unassigned dswij Jun 18, 2025
@llogiq
Copy link
Contributor

llogiq commented Jun 18, 2025

The dogfood error hints at a missing semicolon. Should be easy to fix.

@llogiq
Copy link
Contributor

llogiq commented Jun 18, 2025

This will take more time to review than I have right now, but I have a 9 hour flight tomorrow, so I could pull that branch before departure...

@llogiq
Copy link
Contributor

llogiq commented Jun 23, 2025

In any event, add a semicolon to src/lifetimes.rs:711.

@Jarcho Jarcho force-pushed the lifetime_ice_2 branch 2 times, most recently from 556bd37 to da1ca94 Compare July 17, 2025 07:38
@github-actions
Copy link

Lintcheck changes for da1ca94

Lint Added Removed Changed
clippy::elidable_lifetime_names 30 0 1125
clippy::extra_unused_lifetimes 1 3 4
clippy::needless_lifetimes 23 0 214

This comment will be updated if you push new changes

@rustbot

This comment has been minimized.

@Jarcho Jarcho force-pushed the lifetime_ice_2 branch 2 times, most recently from 7b38cc9 to b58863d Compare September 25, 2025 13:48
Rename `get_source_text` and `check_source_text` to `get_text` and `check_text`.
* Change lint message to not mention the lifetime names * Only point to the lifetime definition in the lint span.
…done when no lint is produced, but has a few behavior changes. * Input to output inference on `dyn Trait` lifetimes is handled with a sufficient MSRV. * Elision on `dyn Trait` input lifetimes where the lifetime is used only once is suggested on all versions. * Lifetimes that are only used as a constraint on a single type are no longer detected. This is a regression. * A bunch of cases where the lints where prevented, but only a single lifetime was non-elidible now lint on remaining lifetimes.
@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

4 participants