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

Make multiple_unsafe_ops_per_block ignore await desugaring #11646

Merged

Conversation

Nilstrieb
Copy link
Member

The await desugaring contains two calls (Poll::new_unchecked and get_context) inside a single unsafe block. That violates the lint.

fixes #11312

changelog: [multiple_unsafe_ops_per_block]: fix false positives in .await

@rustbot
Copy link
Collaborator

rustbot commented Oct 8, 2023

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 8, 2023
@Centri3
Copy link
Member

Centri3 commented Oct 9, 2023

We should really ignore any expansion, right? It's not like multiple operations always causes soundness problems or anything, and the author can't change it. Macros in the current crate I'm not sure, but I'd err on the safe side.

@bend-n
Copy link
Contributor

bend-n commented Oct 10, 2023

ignore any expansion

It would be interesting if that was done for all warnings, but I don't see why it should be special cased for this one?
You get meaningless warnings from macros fairly often, and while it would be nice to not have them, it doesnt really make sense for it to only apply to multiple_unsafe_ops_per_block.

@Centri3
Copy link
Member

Centri3 commented Oct 10, 2023

This is already done for most lints, so no, it shouldn't be special cased for this one :)

There are reasons to lint macro-generated code though, for example when it's UB or almost certainly incorrect

The await desugaring contains two calls (`Poll::new_unchecked` and
`get_context`) inside a single unsafe block. That violates the lint.
@Nilstrieb Nilstrieb force-pushed the compiler-does-not-comply-with-the-lints!! branch from 3dff1c3 to 6ed04af Compare October 14, 2023 21:15
@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Oct 17, 2023

📌 Commit 6ed04af has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 17, 2023

⌛ Testing commit 6ed04af with merge 2cf708d...

@bors
Copy link
Collaborator

bors commented Oct 17, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 2cf708d to master...

@bors bors merged commit 2cf708d into rust-lang:master Oct 17, 2023
5 checks passed
@Nilstrieb Nilstrieb deleted the compiler-does-not-comply-with-the-lints!! branch October 17, 2023 06:59
@ferologics
Copy link

thanks for fixing this here @Nilstrieb 🙇

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.

lots of multiple_unsafe_ops_per_block after toolchain update
7 participants