Lifetime code style consistency in impls and other easy grabs

[BenjaminBrienen]

Objective

A certain level of consistency in how lifetimes are (or aren't) written.

Solution

Fix 'a -> 'w where appropriate.
Fix "'w should always be written" where appropriate.
Fix "'s should always be written" where appropriate.
Fix "named lifetimes should always be written" where appropriate.

Testing

Changes should be effectively "code style", not semantic.

[BenjaminBrienen]

Hold on, I made a mistake

[BenjaminBrienen]

I just want you to know that this is all coming from clippy

[BenjaminBrienen]

@alice-i-cecile No longer 2 lines :(

[BenjaminBrienen]

Ok, clippy isn't requiring anything more.

[Victoronz]

All the 'a lifetimes that are hidden away are a nice cleanup, but this change also hides away a lot of named lifetimes, mostly 'w and 's.
There are cases when only one of the two is present, and now when hidden, is no longer discernible from just any elided 'a lifetime. In other places now only one of these two is hidden, which is no longer be discernible from <'w, 'a>, which also exists.

[alice-i-cecile]

Yeah, I don't like these changes as a whole, and would prefer to allow this lint.

[alice-i-cecile]

Rescinding my approval: we shouldn't do this systematically. The 'a lifetimes are fine, but as @Victoronz points out, the loss of named lifetimes is a net downgrade for readability.

[Victoronz]

Yeah, I don't like these changes as a whole, and would prefer to allow this lint.

I agree. My opinion (for impls) generally is:
Unnamed lifetime -> elide when possible
Named lifetime -> never elide

[alice-i-cecile]

Only a couple more fixes left! I'm in favor of these changes now, and I'm glad to see the lint allowed.

[Victoronz]

I would be careful about keeping the scope of this PR down. I see it now also specifies some named lifetimes, which is fine for impls, but there are also some function (and trait function?) definitions in there now.

Caution should be taken with other locations, because specifying lifetimes in function definitions (i.e. adding generic arguments) can be a breaking change. It also means no longer systematically following/allowing a lint, so that should be a separate decision/PR imo.

[BenjaminBrienen]

I totally understand if we aren't interested in tackling "lifetime style consistency" right now. The problems I noticed are:

• Many 'a should be 'w, so it's not obvious which changes should be reverted.
• Existing inconsistencies in the way lifetimes are written that go against feedback I've received in the PR, which makes me want to fix those, too.
• Other inconsistencies that are easy fixes, like Formatter<'_>.

[chescock]

For what it's worth, I like the elisions described as "regressions" above, so I'd like to argue in favor of them briefly.

The advantage of impl Foo<'_> over impl<'w> Foo<'w> is that you know the lifetime can never be used elsewhere! (This is different from eliding lifetimes in a function signature, where the return type can still refer to one of them as '_.) Knowing that the impl can never depend on that lifetime can be a powerful tool for understanding it.

This is especially useful for SystemParam impls, since the 'w and 's on the type itself are never used, and would be shadowed by the 'w and 's lifetimes in the Item associated type and get_param method.

The other advantage is that the lint can enforce consistency. If the preferred style is to include 'w and 's, then PRs adding code that elides them will still slip in, and we'll wind up with a mix of styles. But if we can turn on a lint, then the CI will ensure that everything is written the same way! (An automatic check can also reduce the time spent bikeshedding on future PRs, although I've probably cost us more than that just by writing this. :) )

[BenjaminBrienen]

I also prefer to just follow the lints, for similar reasons. I'm just doing my best given the feedback since I think pretty much every member has been against that strategy. I'm trying to find a balance between "We don't want to have to write all the lifetimes" in #15916 (for example, writing Mut<'w, T> everywhere and "We don't want to ellide 'w" from here.

[chescock]

I'm just doing my best given the feedback since I think pretty much every member has been against that strategy.

Just in case it wasn't clear: I think you're doing a great job with this PR! My comment was for the folks who were against that strategy. There are definitely tradeoffs to be made here, but I wanted to highlight some of the benefits of your original proposal in case they hadn't been considered.