Skip to content

Perform lifetime resolution on the AST for lowering #91557

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
Merged
merged 8 commits into from
Apr 28, 2022
Merged

Perform lifetime resolution on the AST for lowering #91557

merged 8 commits into from
Apr 28, 2022

Conversation

@cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Dec 5, 2021 •
edited
Loading

Lifetime resolution is currently implemented several times. Once during lowering in order to introduce in-band lifetimes, and once in the resolve_lifetimes query. However, due to the global nature of lifetime resolution and how it interferes with hygiene, it is better suited on the AST.

This PR implements a first draft of lifetime resolution on the AST. For now, we specifically target named lifetimes and everything we need to remove lifetime resolution from lowering. Some diagnostics have already been ported, and sometimes made more precise using available hygiene information. Follow-up PRs will address in particular the resolution of anonymous lifetimes on the AST.

We reuse the rib design of the current resolution framework. Specific LifetimeRib and LifetimeRibKind types are introduced. The most important variant is LifetimeRibKind::Generics, which happens each time we encounter something which may introduce generic lifetime parameters. It can be an item or a for<...> binder. The LifetimeBinderKind specifies how this rib behaves with respect to in-band lifetimes.

r? @petrochenkov

@rust-highfive
Copy link
Contributor

Some changes occurred in src/tools/rustfmt.

cc @calebcartwright

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2021
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from cf0a7cf to 7a82c55 Compare December 5, 2021 22:14
@petrochenkov
Copy link
Contributor

Blocked on #91403.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 6, 2021
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 9, 2021
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from 7a82c55 to fbaff1a Compare December 9, 2021 17:13
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from 312028b to 1aed8ce Compare December 9, 2021 17:37
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from 1aed8ce to 815a52e Compare December 9, 2021 18:58
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

This comment was marked as resolved.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from 815a52e to 89e9723 Compare December 18, 2021 18:14
@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 18, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from 89e9723 to 80435dc Compare December 19, 2021 18:54
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from 80435dc to a9e0018 Compare December 26, 2021 13:49
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from a9e0018 to ab6b005 Compare December 28, 2021 18:22
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

This comment was marked as resolved.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from ab6b005 to 5f1723b Compare January 26, 2022 16:00
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

This comment was marked as resolved.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from 5f1723b to ac45362 Compare January 29, 2022 15:44
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the ast-lifetimes-named branch from cdcc78b to 21b6d23 Compare April 27, 2022 20:04
@cjgillot
Copy link
Contributor Author

@bors r=petrochenkov

@bors
Copy link
Collaborator

bors commented Apr 27, 2022

📌 Commit 21b6d23 has been approved by petrochenkov

@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 27, 2022
@bors
Copy link
Collaborator

bors commented Apr 27, 2022

⌛ Testing commit 21b6d23 with merge c95346b...

@bors
Copy link
Collaborator

bors commented Apr 28, 2022

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing c95346b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 28, 2022
@bors bors merged commit c95346b into rust-lang:master Apr 28, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 28, 2022
This was referenced Apr 28, 2022
Closed
Closed
Merged
Merged
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c95346b): comparison url.

Summary:

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvements found
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 0 0 3 0
mean2 N/A N/A N/A -0.2% N/A
max N/A N/A N/A -0.3% N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes ↩

  2. the arithmetic mean of the percent change ↩

@cjgillot cjgillot deleted the ast-lifetimes-named branch April 28, 2022 16:10
@sciguy16 sciguy16 mentioned this pull request Apr 28, 2022
Closed
@oli-obk
Copy link
Contributor

oli-obk commented Apr 29, 2022

This caused a regression on nightly. The beta cutoff is somewhat soonish, so if we can't land a fix by next week we should revert and try again in a new pr

@oli-obk
Copy link
Contributor

oli-obk commented Apr 29, 2022

Regression issue: #96540

This was referenced May 19, 2022
Closed
Closed
@lqd lqd mentioned this pull request Jul 5, 2022
Closed
@lqd lqd mentioned this pull request Sep 16, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels

merged-by-bors This PR was explicitly merged by bors. 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.62.0

Development

Successfully merging this pull request may close these issues.

9 participants

@cjgillot @rust-highfive @petrochenkov @rust-log-analyzer @bors @rust-timer @oli-obk @apiraino @rustbot