-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Remove Drop impl of mpsc Receiver and (Sync)Sender #114965
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@bors r+ |
Rollup of 6 pull requests Successful merges: - rust-lang#114965 (Remove Drop impl of mpsc Receiver and (Sync)Sender) - rust-lang#115434 (make `Debug` impl for `ascii::Char` match that of `char`) - rust-lang#115477 (Stabilize the `Saturating` type) - rust-lang#115611 (add diagnostic for raw identifiers in format string) - rust-lang#115654 (improve PassMode docs) - rust-lang#115862 (Migrate `compiler/rustc_hir_typeck/src/callee.rs` to translatable diagnostics) r? `@ghost` `@rustbot` modify labels: rollup
Remove Drop impl of mpsc Receiver and (Sync)Sender This change removes the empty `Drop` implementations for `mpsc::Receiver`, `mpsc::Sender` and `mpsc::SyncSender`. These implementations do not specify `#[may_dangle]`, so by removing them we make `mpsc` types play nice with drop check. This was previously attempted in [rust-lang#105243](rust-lang#105243 (comment)) but then [abandoned due to a test failure](rust-lang#105243 (comment)). I've aligned the test with those for `Mutex` and `RwLock`.
Rollup merge of rust-lang#114965 - benschulz:mpsc-drop, r=dtolnay Remove Drop impl of mpsc Receiver and (Sync)Sender This change removes the empty `Drop` implementations for `mpsc::Receiver`, `mpsc::Sender` and `mpsc::SyncSender`. These implementations do not specify `#[may_dangle]`, so by removing them we make `mpsc` types play nice with drop check. This was previously attempted in [rust-lang#105243](rust-lang#105243 (comment)) but then [abandoned due to a test failure](rust-lang#105243 (comment)). I've aligned the test with those for `Mutex` and `RwLock`.
It seems like the expressed motivation mentioned here on why these |
@steffahn, that point was raised in the PR linked above and deemed a non-issue, see #105243 (comment) ff. That notwithstanding, I'm happy to file a PR that restores the |
Thanks for the link. I don't think they should be kept for to address this breakage. I believe it's very very unlikely someone would "rely" on this in the first place, and even then it'd likely not be very reasonable code to begin with. I just wanted it mentioned on this PR for context 😇. And also I was curious if perhaps removal of |
Sorry for posting so many things here - as far as I can tell the contained Even if it does make us accept code the was previously not accepted, there certainly is a lack of test for that, as this PR does not introduce any new tests. |
No worries, I want to get this right. :)
The nightly documentation shows no such implementation, but the stable documentation does. Perhaps I'm misunderstanding your point?
It's implicitly tested by the changed test, but I think you raise a good point in that there is no test whose sole purpose is to test that. I'll try to find the time to file a PR for it. |
Please do. I am confident that the standard library team would be open to this. |
This change removes the empty
Drop
implementations formpsc::Receiver
,mpsc::Sender
andmpsc::SyncSender
. These implementations do not specify#[may_dangle]
, so by removing them we makempsc
types play nice with drop check.This was previously attempted in #105243 but then abandoned due to a test failure. I've aligned the test with those for
Mutex
andRwLock
.