Skip to content

Fix early lints inside an async desugaring #81541

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

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

Aaron1011
Copy link
Member

Fixes #81531

When we buffer an early lint for a macro invocation,
we need to determine which NodeId to take the lint level from.
Currently, we use the NodeId of the closest def parent. However, if
the macro invocation is inside the desugared closure from an async fn
or async closure, that NodeId does not actually exist in the AST.

This commit uses the parent of a desugared closure when computing
lint_node_id, which is something that actually exists in the AST (an
async fn or async closure).

@rust-highfive
Copy link
Contributor

r? @varkor

(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 Jan 30, 2021
@varkor
Copy link
Member

varkor commented Jan 31, 2021

I see @petrochenkov assigned themselves, so I'll defer the review to them. (@petrochenkov: feel free to reassign me if you don't want to take it.)

@varkor varkor removed their assignment Jan 31, 2021
@petrochenkov
Copy link
Contributor

@varkor I mostly wanted to be aware of the changes in this area and assigned myself to not forget.

@Aaron1011 Aaron1011 force-pushed the early-lint-async-fn branch from f112bc6 to ab1d400 Compare February 1, 2021 18:24
@petrochenkov petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 2, 2021
Fixes rust-lang#81531

When we buffer an early lint for a macro invocation,
we need to determine which NodeId to take the lint level from.
Currently, we use the `NodeId` of the closest def parent. However, if
the macro invocation is inside the desugared closure from an `async fn`
or async closure, that `NodeId` does not actually exist in the AST.

This commit explicitly calls `check_lint` for the `NodeId`s of closures
desugared from async expressions, ensuring that we do not miss any
buffered lints.
@Aaron1011 Aaron1011 force-pushed the early-lint-async-fn branch from ab1d400 to a74b2fb Compare February 2, 2021 18:58
@Aaron1011
Copy link
Member Author

@petrochenkov: I've updated the PR explicitly handle Async::Yes during linting.

@Aaron1011 Aaron1011 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 2, 2021
@petrochenkov
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Feb 2, 2021

📌 Commit a74b2fb 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. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 2, 2021
@Aaron1011
Copy link
Member Author

This bug has been affecting a lot of users (see #81531)

@bors p=1

@bors
Copy link
Collaborator

bors commented Feb 2, 2021

⌛ Testing commit a74b2fb with merge 3682750...

@bors
Copy link
Collaborator

bors commented Feb 2, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 2, 2021
@bors bors merged commit 3682750 into rust-lang:master Feb 2, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 2, 2021
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

internal compiler error: failed to process buffered lint here
7 participants