Add display of lifetime of Ref in fn fmt() for TypeError

[yanchen4791]

Fix for #106517.

Problem:
When explaining the source of a type error, the lifetime portion of references is sometimes omitted in order to shorten the message. This can lead to confusion as the message can disagree with other spans which use the long form with the lifetime portion. For example, for the same type error, one span may print expected '&str', found enum 'option' while another span may print expected '&'static str' because of return type and note: expected reference '&'static str' found enum 'option<&str>'.

Solution:
To add the lifetime portion of the references when generating the message. This will ensure consistency between the messages and avoid confusion, especially when the message is already short and the omitted information does not have to be removed.

[rustbot]

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

[yanchen4791]

r? nagisa

[nagisa]

Sorry for the lack of earlier feedback on the issue. The bug that I was reporting here was specifically the mismatch between:

help: consider introducing a named lifetime parameter
1 | fn parse_type<'a>(iter: Box<dyn Iterator<Item=&'a str>+'static>) -> &'a str { iter.next() }

and the

expected `&'static str` because of return type

Rather than between these:

  |                                                              ----   ^^^^^^^^^^^ expected `&str`, found enum `Option`
  |                                                              |
  |                                                              expected `&'static str` because of return type

As thus, I don’t believe this PR to be a fix for the referenced issue. I’m not particularly familiar with the code, since I haven’t had an opportunity to look at this, but I’m surprised that ReStatic is the lifetime the compiler comes up with to pass onto later passes. It might make sense to introduce (if there isn’t already) a ReError or something to indicate that a region-related error has already been output, and to signal to downstream diagnostics to not print any lifetime for that type, or to potentially not attempt any further diagnostics at all, regardless of the context. Alternatively, I think having these be TyError might also make sense

I also don’t think this PR is the right thing to do in general. Adding lifetimes to all notes that are likely pointing out larger structural differences (like reference vs enum) won’t make the diagnostic much clearer – in fact it will likely contribute to clutter. Adding lifetimes probably only makes sense if the lifetime mismatch is between two reference types of otherwise equivalent structure.

cc @estebank

[estebank]

I’m surprised that ReStatic is the lifetime the compiler comes up with to pass onto later passes. It might make sense to introduce (if there isn’t already) a ReError or something to indicate that a region-related error has already been output

There isn't, and ReStatic is chosen because it is the closest "safe" lifetime to use, but I'm in favor of introducing ReError.

Edit: another case where ReError would have helped #69314

Edit 2: #107652

[bors]

☔ The latest upstream changes (presumably #106399) made this pull request unmergeable. Please resolve the merge conflicts.

[nagisa]

Given that there's a broader agreement that this is not an approach that will make the situation better, and given the introduction of ReError, I’ll close this.

Thanks for the contribution @yanchen4791, and sorry for not making myself clearer in the issue description!