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

Spurious warning in #[allow(clippy::non_send_fields_in_send_ty)] for complex thread-safe wrapper #8275

Open
alice-i-cecile opened this issue Jan 13, 2022 · 2 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have L-nursery Lint: Currently in the nursery group

Comments

@alice-i-cecile
Copy link

Summary

Our unsafe implementation of Send for a wrapper type that deliberately strips out all of the !Send components of our complex collection tripped the new clippy lint. This was, in fact the entire point of the type, and why we used an unsafe impl block.

Ping @DJMcNab, the author of that PR.

Lint Name

#[allow(clippy::non_send_fields_in_send_ty)]

Reproducer

See bevyengine/bevy#3519 for the case where this lint triggered spuriously.

The existing warning on World was correct: this impl was in fact unsound, and this PR was an attempt to solve that problem.

Version

rustc 1.60.0-nightly (1bd4fdc94 2022-01-12)
binary: rustc
commit-hash: 1bd4fdc943513e1004f498bbf289279c9784fc6f
commit-date: 2022-01-12
host: x86_64-pc-windows-msvc
release: 1.60.0-nightly
LLVM version: 13.0.0

Additional Labels

No response

@alice-i-cecile alice-i-cecile added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jan 13, 2022
@xFrednet
Copy link
Member

Thank you for the report. The lint sadly has several false positives.

There will be a 1.58.1 release to make the lint allow by default on next Thursday. (See #8045)

@Qwaz
Copy link
Contributor

Qwaz commented Jan 14, 2022

Thanks for sharing the spurious failures. It's still good to hear that the lint was able to prevent a soundness bug. Hope we can bring this lint back to enable-by-default in the future after solving these known false positives.

@J-ZhengLi J-ZhengLi added the L-nursery Lint: Currently in the nursery group label Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have L-nursery Lint: Currently in the nursery group
Projects
None yet
Development

No branches or pull requests

4 participants