rustc suggests for<'a> when lifetime param already exists

[Rudxain]

Code

fn abs<'a, T>(x: &'a T) -> T
where
    &'a T: Ord + core::ops::Neg<Output = &T>,
    T: Clone,
{
    x.max(-x).clone()
}

Current output

Compiling playground v0.0.1 (/playground)
error[E0637]: `&` without an explicit lifetime name cannot be used here
 --> src/lib.rs:3:42
  |
3 |     &'a T: Ord + core::ops::Neg<Output = &T>,
  |                                          ^ explicit lifetime name needed here
  |
help: consider introducing a higher-ranked lifetime here
  |
3 |     &'a T: Ord + for<'a> core::ops::Neg<Output = &'a T>,
  |                  +++++++                          ++

For more information about this error, try `rustc --explain E0637`.
error: could not compile `playground` (lib) due to 1 previous error

Desired output

Compiling playground v0.0.1 (/playground)
error[E0637]: `&` without an explicit lifetime name cannot be used here
 --> src/lib.rs:3:42
  |
3 |     &'a T: Ord + core::ops::Neg<Output = &T>,
  |                                          ^ explicit lifetime name needed here
  |
help: consider introducing a higher-ranked lifetime here
  |
3 |     &'a T: Ord + core::ops::Neg<Output = &'a T>,
  |                                           ++

For more information about this error, try `rustc --explain E0637`.
error: could not compile `playground` (lib) due to 1 previous error

Rationale and extra context

Following the compiler's suggestion:

   Compiling playground v0.0.1 (/playground)
error[E0496]: lifetime name `'a` shadows a lifetime name that is already in scope
 --> src/lib.rs:3:22
  |
1 | fn abs<'a, T>(x: &'a T) -> T
  |        -- first declared here
2 | where
3 |     &'a T: Ord + for<'a> core::ops::Neg<Output = &'a T>,
  |                      ^^ lifetime `'a` already in scope

For more information about this error, try `rustc --explain E0496`.
error: could not compile `playground` (lib) due to 1 previous error

Other cases

No response

Rust Version

From playground:
1.79.0-nightly

(2024-04-08 ab5bda1aa70f707014e2)

Anything else?

No response

[fmease]

Slightly relates to #122714. Marking as D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. because introducing for<'a> in the presence of <'a> is not only what the user probably doesn't want but also incorrect because it would shadow the outer lifetime parameter which is forbidden.

We should walk the lifetime ribs / check the lifetime params in scope.

If there are multiple lifetime params available or if there are none, we should generally keep suggesting introducing a higher-ranked lifetime param with a fresh name that doesn't clash (*).

(*) However, in this specific case — an anonymous lifetime inside an assoc type binding / constraint — that would actually be incorrect, too, as it would always be unconstrained (E0582).

That's gonna be slightly harder to fix (but not impossible) because the check for anonymous lifetimes happens during late resolution (rustc_resolve::late) and we detect unconstrained late bound vars during HIR ty lowering in validate_late_bound_regions (because it requires some more pieces of information not available during late resolution). Either we make use of a heuristic in late (falling back to suggesting adding a lifetime param to the innermost item that can be generic or using 'static which wouldn't be that great) or if feasible we stash the diagnostic and add onto it later during HIR ty lowering.

[fmease]

If there are multiple lifetime params available or if there are none, we should generally keep suggesting introducing a higher-ranked lifetime param with a fresh name that doesn't clash (*).

Regarding the decision to keep suggesting for<'a> in certain cases: It's often more general. Note that we used to suggest 'static in these positions if I remember correctly — I might be mistaken — and we intentionally changed it to for<'a> ... 'a. Maybe it wasn't an issue but a PR 🤔

[fmease]

(*) However, in this specific case — an anonymous lifetime inside an assoc type binding / constraint — that would actually be incorrect, too, as it would always be unconstrained (E0582).

Ah, we have already have issue #122025 / PR #123245 for that, I knew I saw an issue related to that.

[estebank]

@fmease I think it makes sense to suggest both for<'x> and using the existing 'a, when in doubt. I believe that when we introduced this suggestion first we briefly tried that out. I don't recall why I didn't keep it back then. It might have been that none of the cases in the test suite benefited from not using for<'a>.