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

Improve span for consider adding an explicit lifetime bound suggestions under NLL #96352

Merged
merged 3 commits into from
Apr 24, 2022

Conversation

marmeladema
Copy link
Contributor

Because NLL borrowck is run after typeck, in_progress_typeck_results was always None which was preventing the retrieval of the span to which the suggestion is suppose to add the lifetime bound.
We now manually pass the LocalDefId owner to construct_generic_bound_failure so that under NLL, we give the owner id of the current body.

This helps with #96332

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 23, 2022
@rust-highfive
Copy link
Collaborator

r? @oli-obk

(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 Apr 23, 2022
@marmeladema
Copy link
Contributor Author

r? @jackh726

@rust-highfive rust-highfive assigned jackh726 and unassigned oli-obk Apr 23, 2022
@jackh726
Copy link
Member

Looks good :) need to compare nll vs base before I r+ though

Why wouldn't this close #96332

@marmeladema
Copy link
Contributor Author

Why wouldn't this close #96332

Because while I was investigating this, I realized that under NLL those suggestions are missing a part:

...so that the type `T` will meet its required lifetime bounds

but also a note:

note: ...that is required by this bound
  --> $DIR/regions-close-object-into-object-5.rs:13:17
   |
LL | struct B<'a, T: 'a>(&'a (A<T> + 'a));

I do believe this PR is a step in the right direction, but imo is not enough to recover everything lost under NLL.

…ions under NLL

Because NLL borrowck is run after typeck, `in_progress_typeck_results`
was always `None` which was preventing the retrieval of the span to which
the suggestion is suppose to add the lifetime bound.

We now manually pass the `LocalDefId` owner to `construct_generic_bound_failure`
so that under NLL, we give the owner id of the current body.
@marmeladema marmeladema force-pushed the fix-nll-lifetime-bound-suggestions branch from 22f53b6 to 53120b5 Compare April 24, 2022 07:36
@marmeladema
Copy link
Contributor Author

Why wouldn't this close #96332

Because while I was investigating this, I realized that under NLL those suggestions are missing a part:

...so that the type `T` will meet its required lifetime bounds

Actually, I think I've just solved that first problem with the second commit.

but also a note:

note: ...that is required by this bound
  --> $DIR/regions-close-object-into-object-5.rs:13:17
   |
LL | struct B<'a, T: 'a>(&'a (A<T> + 'a));

I do believe this PR is a step in the right direction, but imo is not enough to recover everything lost under NLL.

@jackh726
Copy link
Member

I think this is a step in the right direction, but it doesn't touch any of the tests from #96212. It's an improvement though, so we can land.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 24, 2022

📌 Commit 53120b5 has been approved by jackh726

@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 Apr 24, 2022
@jackh726
Copy link
Member

@bors r- needs rebase

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 24, 2022
@jackh726
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 24, 2022

📌 Commit 8d561d2 has been approved by jackh726

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 24, 2022
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#94893 (diagnostics: regression test for `<usize as Iterator>::rev`)
 - rust-lang#95504 (Add `x {check,build,doc} {compiler,library}` aliases.)
 - rust-lang#96237 (compiletest: combine `--*-python` args)
 - rust-lang#96303 (Improve bootstrap tests)
 - rust-lang#96352 (Improve span for `consider adding an explicit lifetime bound` suggestions under NLL)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit aef9eb5 into rust-lang:master Apr 24, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 24, 2022
@marmeladema marmeladema deleted the fix-nll-lifetime-bound-suggestions branch April 24, 2022 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants