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

Prevent clippy::needless_lifetimes false positive in async function definition #7901

Merged
merged 2 commits into from
Nov 2, 2021

Conversation

samueltardieu
Copy link
Contributor

Scan OpaqueDef bounds for lifetimes as well. Those OpaqueDef instances are generated while desugaring an async function definition.

This fixes #7893

changelog: Prevent [clippy::needless_lifetimes] false positive in async function definition

@rust-highfive
Copy link

r? @xFrednet

(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 Oct 30, 2021
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

This looks good to me 👍. I'm not quite sure how these OpaqueDef types are used inside rustc, I'll therefore ask some team members to double-check this PR 🙃

@samueltardieu
Copy link
Contributor Author

I'm not quite sure how these OpaqueDef types are used inside rustc,

According to the documentation of OpaqueDef, this is the type used to expand impl Trait (and async fn gets expanded into a function returning impl Future<_>). The documentation also indicates that the second field of OpaqueDef contains the lifetimes bound on the impl Trait, hence this patch.

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Thanks for the further explanation. This LGTM. There's a small NIT with the test.

tests/ui/needless_lifetimes.rs Outdated Show resolved Hide resolved
@flip1995
Copy link
Member

flip1995 commented Nov 2, 2021

@bors r=xFrednet,flip1995

Thanks!

@bors
Copy link
Collaborator

bors commented Nov 2, 2021

📌 Commit 9bcf966 has been approved by xFrednet,flip1995

@bors
Copy link
Collaborator

bors commented Nov 2, 2021

⌛ Testing commit 9bcf966 with merge 9720619...

@bors
Copy link
Collaborator

bors commented Nov 2, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet,flip1995
Pushing 9720619 to master...

@bors bors merged commit 9720619 into rust-lang:master Nov 2, 2021
@samueltardieu samueltardieu deleted the fix-7893 branch March 5, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lifetime cannot be elided
5 participants