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

Skip single use lifetime lint for generated opaque types #88650

Merged
merged 2 commits into from
Sep 18, 2021

Conversation

sapessi
Copy link
Contributor

@sapessi sapessi commented Sep 4, 2021

Fix: #77175

The opaque type generated by the desugaring process of an async function uses the lifetimes defined by the originating function. The DefId for the lifetimes in the opaque type are different from the ones in the originating async function - as they should be, as far as I understand, and could therefore be considered a single use lifetimes, this causes the single_use_lifetimes lint to fail compilation if explicitly denied. This fix skips the lint for lifetimes used only once in generated opaque types for an async function that are declared in the parent async function definition.

More info in the comments on the original issue: 1 and 2

As reported in issue rust-lang#77175, the opaque type generated by the desugaring process of an async function uses the lifetimes defined by the originating function. The definition ID for the lifetimes in the opaque method is different from the one in the originating async function and it could therefore be considered a single use of the lifetimne, this causes the single_use_lifetimes lint to fail compilation if explicitly denied. This fix skips the lint for lifetimes used only once in generated opaque types for an async function that are declared in the parent async function definition.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @wesleywiser (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 4, 2021
@sapessi
Copy link
Contributor Author

sapessi commented Sep 4, 2021

This is a copy of the original #78004 PR. The CR comments on using an FxHashSet for the generic properties is not yet addressed

@estebank
Copy link
Contributor

estebank commented Sep 5, 2021

@bors try @rust-timer queue

Noticed that we're fetching the item unconditionally on every DefId to see if it might be an async fn. Running a perf check to see if it has performance implications.

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 5, 2021
@bors
Copy link
Contributor

bors commented Sep 5, 2021

⌛ Trying commit 0696c28 with merge 5d2576145eba4a6649c0c2b27d4a774b8138797b...

@bors
Copy link
Contributor

bors commented Sep 5, 2021

☀️ Try build successful - checks-actions
Build commit: 5d2576145eba4a6649c0c2b27d4a774b8138797b (5d2576145eba4a6649c0c2b27d4a774b8138797b)

@rust-timer
Copy link
Collaborator

Queued 5d2576145eba4a6649c0c2b27d4a774b8138797b with parent f7c00dc, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (5d2576145eba4a6649c0c2b27d4a774b8138797b): comparison url.

Summary: This benchmark run did not return any relevant changes.

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

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 5, 2021
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 13, 2021
@sapessi
Copy link
Contributor Author

sapessi commented Sep 17, 2021

Anything else we need to do here @estebank?

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 18, 2021

📌 Commit 0696c28 has been approved by estebank

@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 18, 2021
@bors
Copy link
Contributor

bors commented Sep 18, 2021

⌛ Testing commit 0696c28 with merge 23afad6...

@bors
Copy link
Contributor

bors commented Sep 18, 2021

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 23afad6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 18, 2021
@bors bors merged commit 23afad6 into rust-lang:master Sep 18, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 18, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (23afad6): comparison url.

Summary: This benchmark run did not return any relevant changes.

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

@rustbot label: -perf-regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive: single_use_lifetimes
8 participants