Rollup merge of rust-lang#111087 - ibraheemdev:patch-15, r=dtolnay

Implement `Sync` for `mpsc::Sender` `mpsc::Sender` is currently `!Sync` because the previous implementation contained an optimization where the channel started out as single-producer and was dynamically upgraded on the first clone, which relied on a unique reference to the sender. This optimization is one of the main reasons the old implementation was so complex and was removed in rust-lang#93563. `mpsc::Sender` can now soundly implement `Sync`. Note for any potential confusion, this chance does *not* add MPMC behavior. This only affects the already `Send + Clone` *sender*, not *receiver*. It's technically possible to rely on the `!Sync` behavior in the same way as a `PhantomData<*mut T>`, but that seems very unlikely in practice. Either way, this change is insta-stable and needs an FCP. `@rustbot` label +T-libs-api -T-libs
compiler-errors · Jun 24, 2023 · 4a01a38 · 4a01a38
2 parents 1d67eba + 4ceca09
commit 4a01a38
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 81 deletions.
diff --git a/library/std/src/sync/mpsc/mod.rs b/library/std/src/sync/mpsc/mod.rs
@@ -347,8 +347,8 @@ pub struct Sender<T> {
 #[stable(feature = "rust1", since = "1.0.0")]
 unsafe impl<T: Send> Send for Sender<T> {}
 
-#[stable(feature = "rust1", since = "1.0.0")]
-impl<T> !Sync for Sender<T> {}
+#[stable(feature = "mpsc_sender_sync", since = "CURRENT_RUSTC_VERSION")]
+unsafe impl<T: Send> Sync for Sender<T> {}
 
 /// The sending-half of Rust's synchronous [`sync_channel`] type.
 ///

diff --git a/tests/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr b/tests/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr
@@ -1,30 +1,37 @@
-error[E0277]: `Sender<i32>` cannot be shared between threads safely
-  --> $DIR/issue-70935-complex-spans.rs:13:45
+error[E0277]: `*mut ()` cannot be shared between threads safely
+  --> $DIR/issue-70935-complex-spans.rs:18:23
    |
-LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
-   |                                             ^^^^^^^^^^^^^^^^^^ `Sender<i32>` cannot be shared between threads safely
+LL | fn foo(x: NotSync) -> impl Future + Send {
+   |                       ^^^^^^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
    |
-   = help: the trait `Sync` is not implemented for `Sender<i32>`
-   = note: required for `&Sender<i32>` to implement `Send`
+   = help: within `NotSync`, the trait `Sync` is not implemented for `*mut ()`
+note: required because it appears within the type `PhantomData<*mut ()>`
+  --> $SRC_DIR/core/src/marker.rs:LL:COL
+note: required because it appears within the type `NotSync`
+  --> $DIR/issue-70935-complex-spans.rs:12:8
+   |
+LL | struct NotSync(PhantomData<*mut ()>);
+   |        ^^^^^^^
+   = note: required for `&NotSync` to implement `Send`
 note: required because it's used within this closure
-  --> $DIR/issue-70935-complex-spans.rs:17:13
+  --> $DIR/issue-70935-complex-spans.rs:22:13
    |
-LL |         baz(|| async{
+LL |         baz(|| async {
    |             ^^
 note: required because it's used within this `async fn` body
-  --> $DIR/issue-70935-complex-spans.rs:10:67
+  --> $DIR/issue-70935-complex-spans.rs:15:67
    |
 LL |   async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
    |  ___________________________________________________________________^
 LL | | }
    | |_^
    = note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()`
 note: required because it's used within this `async` block
-  --> $DIR/issue-70935-complex-spans.rs:16:5
+  --> $DIR/issue-70935-complex-spans.rs:21:5
    |
 LL | /     async move {
-LL | |         baz(|| async{
-LL | |             foo(tx.clone());
+LL | |         baz(|| async {
+LL | |             foo(x.clone());
 LL | |         }).await;
 LL | |     }
    | |_____^

diff --git a/tests/ui/async-await/issue-70935-complex-spans.drop_tracking_mir.stderr b/tests/ui/async-await/issue-70935-complex-spans.drop_tracking_mir.stderr
@@ -1,30 +1,37 @@
-error[E0277]: `Sender<i32>` cannot be shared between threads safely
-  --> $DIR/issue-70935-complex-spans.rs:13:45
+error[E0277]: `*mut ()` cannot be shared between threads safely
+  --> $DIR/issue-70935-complex-spans.rs:18:23
    |
-LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
-   |                                             ^^^^^^^^^^^^^^^^^^ `Sender<i32>` cannot be shared between threads safely
+LL | fn foo(x: NotSync) -> impl Future + Send {
+   |                       ^^^^^^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
    |
-   = help: the trait `Sync` is not implemented for `Sender<i32>`
-   = note: required for `&Sender<i32>` to implement `Send`
+   = help: within `NotSync`, the trait `Sync` is not implemented for `*mut ()`
+note: required because it appears within the type `PhantomData<*mut ()>`
+  --> $SRC_DIR/core/src/marker.rs:LL:COL
+note: required because it appears within the type `NotSync`
+  --> $DIR/issue-70935-complex-spans.rs:12:8
+   |
+LL | struct NotSync(PhantomData<*mut ()>);
+   |        ^^^^^^^
+   = note: required for `&NotSync` to implement `Send`
 note: required because it's used within this closure
-  --> $DIR/issue-70935-complex-spans.rs:17:13
+  --> $DIR/issue-70935-complex-spans.rs:22:13
    |
-LL |         baz(|| async{
+LL |         baz(|| async {
    |             ^^
 note: required because it's used within this `async fn` body
-  --> $DIR/issue-70935-complex-spans.rs:10:67
+  --> $DIR/issue-70935-complex-spans.rs:15:67
    |
 LL |   async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
    |  ___________________________________________________________________^
 LL | | }
    | |_^
    = note: required because it captures the following types: `impl Future<Output = ()>`
 note: required because it's used within this `async` block
-  --> $DIR/issue-70935-complex-spans.rs:16:5
+  --> $DIR/issue-70935-complex-spans.rs:21:5
    |
 LL | /     async move {
-LL | |         baz(|| async{
-LL | |             foo(tx.clone());
+LL | |         baz(|| async {
+LL | |             foo(x.clone());
 LL | |         }).await;
 LL | |     }
    | |_____^

diff --git a/tests/ui/async-await/issue-70935-complex-spans.no_drop_tracking.stderr b/tests/ui/async-await/issue-70935-complex-spans.no_drop_tracking.stderr
@@ -1,21 +1,21 @@
 error: future cannot be sent between threads safely
-  --> $DIR/issue-70935-complex-spans.rs:13:45
+  --> $DIR/issue-70935-complex-spans.rs:18:23
    |
-LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
-   |                                             ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
+LL | fn foo(x: NotSync) -> impl Future + Send {
+   |                       ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
    |
-   = help: the trait `Sync` is not implemented for `Sender<i32>`
+   = help: within `NotSync`, the trait `Sync` is not implemented for `*mut ()`
 note: future is not `Send` as this value is used across an await
-  --> $DIR/issue-70935-complex-spans.rs:19:12
+  --> $DIR/issue-70935-complex-spans.rs:24:12
    |
-LL |           baz(|| async{
+LL |           baz(|| async {
    |  _____________-
-LL | |             foo(tx.clone());
+LL | |             foo(x.clone());
 LL | |         }).await;
    | |         -  ^^^^^- the value is later dropped here
    | |         |  |
    | |_________|  await occurs here, with the value maybe used later
-   |           has type `[closure@$DIR/issue-70935-complex-spans.rs:17:13: 17:15]` which is not `Send`
+   |           has type `[closure@$DIR/issue-70935-complex-spans.rs:22:13: 22:15]` which is not `Send`
 
 error: aborting due to previous error
 
diff --git a/tests/ui/async-await/issue-70935-complex-spans.rs b/tests/ui/async-await/issue-70935-complex-spans.rs
@@ -6,16 +6,21 @@
 // with newlines which lead complex diagnostics.
 
 use std::future::Future;
+use std::marker::PhantomData;
+
+#[derive(Clone)]
+struct NotSync(PhantomData<*mut ()>);
+unsafe impl Send for NotSync {}
 
 async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
 }
 
-fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
+fn foo(x: NotSync) -> impl Future + Send {
     //[no_drop_tracking]~^ ERROR future cannot be sent between threads safely
-    //[drop_tracking,drop_tracking_mir]~^^ ERROR `Sender<i32>` cannot be shared between threads
+    //[drop_tracking,drop_tracking_mir]~^^ ERROR `*mut ()` cannot be shared between threads
     async move {
-        baz(|| async{
-            foo(tx.clone());
+        baz(|| async {
+            foo(x.clone());
         }).await;
     }
 }
@@ -24,6 +29,6 @@ fn bar(_s: impl Future + Send) {
 }
 
 fn main() {
-    let (tx, _rx) = std::sync::mpsc::channel();
-    bar(foo(tx));
+    let x = NotSync(PhantomData);
+    bar(foo(x));
 }
diff --git a/tests/ui/closures/closure-move-sync.rs b/tests/ui/closures/closure-move-sync.rs
@@ -13,10 +13,4 @@ fn bar() {
     t.join().unwrap();
 }
 
-fn foo() {
-    let (tx, _rx) = channel();
-    thread::spawn(|| tx.send(()).unwrap());
-    //~^ ERROR `Sender<()>` cannot be shared between threads safely
-}
-
 fn main() {}
diff --git a/tests/ui/closures/closure-move-sync.stderr b/tests/ui/closures/closure-move-sync.stderr
@@ -20,24 +20,6 @@ LL |     let t = thread::spawn(|| {
 note: required by a bound in `spawn`
   --> $SRC_DIR/std/src/thread/mod.rs:LL:COL
 
-error[E0277]: `Sender<()>` cannot be shared between threads safely
-  --> $DIR/closure-move-sync.rs:18:19
-   |
-LL |     thread::spawn(|| tx.send(()).unwrap());
-   |     ------------- ^^^^^^^^^^^^^^^^^^^^^^^ `Sender<()>` cannot be shared between threads safely
-   |     |
-   |     required by a bound introduced by this call
-   |
-   = help: the trait `Sync` is not implemented for `Sender<()>`
-   = note: required for `&Sender<()>` to implement `Send`
-note: required because it's used within this closure
-  --> $DIR/closure-move-sync.rs:18:19
-   |
-LL |     thread::spawn(|| tx.send(()).unwrap());
-   |                   ^^
-note: required by a bound in `spawn`
-  --> $SRC_DIR/std/src/thread/mod.rs:LL:COL
-
-error: aborting due to 2 previous errors
+error: aborting due to previous error
 
 For more information about this error, try `rustc --explain E0277`.
diff --git a/tests/ui/stdlib-unit-tests/not-sync.rs b/tests/ui/stdlib-unit-tests/not-sync.rs
@@ -17,6 +17,4 @@ fn main() {
 
     test::<Receiver<i32>>();
     //~^ ERROR `std::sync::mpsc::Receiver<i32>` cannot be shared between threads safely [E0277]
-    test::<Sender<i32>>();
-    //~^ ERROR `Sender<i32>` cannot be shared between threads safely [E0277]
 }
diff --git a/tests/ui/stdlib-unit-tests/not-sync.stderr b/tests/ui/stdlib-unit-tests/not-sync.stderr
@@ -65,19 +65,6 @@ note: required by a bound in `test`
 LL | fn test<T: Sync>() {}
    |            ^^^^ required by this bound in `test`
 
-error[E0277]: `Sender<i32>` cannot be shared between threads safely
-  --> $DIR/not-sync.rs:20:12
-   |
-LL |     test::<Sender<i32>>();
-   |            ^^^^^^^^^^^ `Sender<i32>` cannot be shared between threads safely
-   |
-   = help: the trait `Sync` is not implemented for `Sender<i32>`
-note: required by a bound in `test`
-  --> $DIR/not-sync.rs:5:12
-   |
-LL | fn test<T: Sync>() {}
-   |            ^^^^ required by this bound in `test`
-
-error: aborting due to 6 previous errors
+error: aborting due to 5 previous errors
 
 For more information about this error, try `rustc --explain E0277`.