Skip to content
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.

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

Pick one possible lifetime in case there are multiple choices #89327

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Sep 28, 2021

In case a lifetime variable is created, but doesn't have an obvious lifetime in the list of named lifetimes that it should be inferred to, just pick the first one for the diagnostic.

This happens e.g. in

fn foo<'a, 'b>(a: Struct<'a>, b: Struct<'b>) -> impl Trait<'a, 'b> {
    if bar() { a } else { b }
}

where we get a lifetime variable that combines the lifetimes of a and b creating a lifetime that is the intersection of both. Right now the type system cannot express this and thus we get an error, but that error also can't express this.

I can also create an entirely new diagnostic that mentions all involved lifetimes, so it would actually mention 'a and 'b instead of just 'b.

@rust-highfive
Copy link
Collaborator

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 28, 2021
@oli-obk oli-obk added the NLL-diagnostics Working towards the "diagnostic parity" goal label Sep 28, 2021
@jackh726
Copy link
Member

@nikomatsakis I think this was precisely the kind of example I was thinking of during the discussion of #89056

@wesleywiser
Copy link
Member

I can also create an entirely new diagnostic that mentions all involved lifetimes, so it would actually mention 'a and 'b instead of just 'b.

I think that would be ideal but I'm also happy to merge this and put an issue in to improve it later.

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 29, 2021

@bors r=wesleywiser

I'll open an issue

@bors
Copy link
Contributor

bors commented Sep 29, 2021

📌 Commit 87a4a79 has been approved by wesleywiser

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 29, 2021
@@ -4,7 +4,11 @@ error[E0700]: hidden type for `impl Trait` captures lifetime that does not appea
LL | fn upper_bounds<'a, 'b, 'c, 'd, 'e>(a: Ordinary<'a>, b: Ordinary<'b>) -> impl Trait<'d, 'e>
| ^^^^^^^^^^^^^^^^^^
|
= note: hidden type `Ordinary<'_>` captures lifetime '_#9r
note: hidden type `Ordinary<'b>` captures the lifetime `'b` as defined on the function body at 16:21
Copy link
Member

@lqd lqd Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This is pre-existing)

This message mentions Ordinary<'b> here as a "hidden type", shouldn't this be "hidden type for impl Trait" instead, or am I misunderstanding this diagnostic ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are completely right.

We should completely rewrite the diagnostics around TAIT in general... they are all not too helpful. I'll open an issue to track the various ways in which it is not great

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's maybe even impl Trait diagnostics in general, as there's no TAIT here, for example.

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 1, 2021
…arth

Rollup of 7 pull requests

Successful merges:

 - rust-lang#88838 (Do not suggest importing inaccessible items)
 - rust-lang#89251 (Detect when negative literal indices are used and suggest appropriate code)
 - rust-lang#89321 (Rebase resume argument projections during state transform)
 - rust-lang#89327 (Pick one possible lifetime in case there are multiple choices)
 - rust-lang#89344 (Cleanup lower_generics_mut and make span be the bound itself)
 - rust-lang#89397 (Update `llvm` submodule to fix function name mangling on x86 Windows)
 - rust-lang#89412 (Add regression test for issues rust-lang#88969 and rust-lang#89119 )

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8c5114b into rust-lang:master Oct 1, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 1, 2021
@oli-obk oli-obk deleted the nll_diag_infer_vars branch October 1, 2021 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NLL-diagnostics Working towards the "diagnostic parity" goal S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants