Skip to content

Improve Lifetime::suggestion #139046

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Improve Lifetime::suggestion #139046

wants to merge 2 commits into from
+166 −84

Conversation

nnethercote
Copy link
Contributor

r? @ghost

@nnethercote
Augment impl-trait-missing-lifetime-gated.rs.
a719d8b 
We have coverage for `Foo` and `Foo<T>` but not for `Foo<>`. This commit
adds it. Note that the output has bogus double brackets: `Foo<<'a>>`.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 28, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nnethercote nnethercote force-pushed the hir-Lifetime-better branch from be80b19 to effd5cc Compare March 28, 2025 07:50
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nnethercote
Improve Lifetime::suggestion.
3eda3db 
It currently used `span.is_empty()` for the `is_anon_in_path` cases, and
also adjusts the spans when they are non-empty. All this is very hard to
understand.

XXX

- New `Option<AngleBracket>` type makes things much clearer.

- Simpler spans in `maybe_insert_elided_lifetimes_in_path`. They're now
  all empty, and point to the lifetime insertion point. `suggestion` no
  longer has to call `shrink_to_hi` on any spans.

- `make_suggestion` closure now just calls `Lifetime::suggestion`,
  because it's doing the same thing

Finally, some error messages are improved. First, on the `Path` case, we
have this diff:

 LL |     fn f(_: impl Foo) {}
-   |                  ^^^ expected named lifetime parameter
+   |                     ^ expected named lifetime parameter

The carets now point to where the named lifetime parameter must be
inserted, as per the subsequent suggestion:

LL |     fn f<'a>(_: impl Foo<'a>) {}
   |         ++++            ++++

Similar story on this diff on the `Path<>` case:

 LL |     fn f(_: impl Foo<>) {}
-   |                     ^ expected named lifetime parameter
+   |                      ^ expected named lifetime parameter

Plus we print the `Path<>` suggestion properly:

-LL |     fn f<'a>(_: impl Foo<<'a>>) {}
-   |         ++++             ++++
+LL |     fn f<'a>(_: impl Foo<'a>) {}
+   |         ++++             ++
@nnethercote nnethercote force-pushed the hir-Lifetime-better branch from effd5cc to 3eda3db Compare March 28, 2025 09:11
@nnethercote nnethercote mentioned this pull request Mar 28, 2025
Merged
@bors
Copy link
Collaborator

bors commented Apr 15, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nnethercote @rust-log-analyzer @bors @rustbot