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

Suggest using a lock for *Cell: Sync bounds #106944

Merged
merged 2 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions library/core/src/cell/once.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,3 +298,7 @@ impl<T> const From<T> for OnceCell<T> {
OnceCell { inner: UnsafeCell::new(Some(value)) }
}
}

// Just like for `Cell<T>` this isn't needed, but results in nicer error messages.
#[unstable(feature = "once_cell", issue = "74465")]
impl<T> !Sync for OnceCell<T> {}
Comment on lines +302 to +304
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually... Removing this would be a breaking change: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=1f4ad2c6d839ce8a21237c4faa7b2e9e (tl;dr: you'll be able to impl trait for <T: Sync> and OnceCell).

Should we consult t-libs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not believe so (I'm not totally sure) - coherence only lets you do this in the crate where S is defined, for now there is no (stable) way to rely on a negative impl.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it is said to be a breaking change by the docs: https://doc.rust-lang.org/beta/unstable-book/language-features/negative-impls.html, so I'm again not sure about this.

56 changes: 56 additions & 0 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,62 @@ pub macro Copy($item:item) {
#[cfg_attr(not(test), rustc_diagnostic_item = "Sync")]
#[lang = "sync"]
#[rustc_on_unimplemented(
on(
_Self = "std::cell::OnceCell<T>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::OnceLock` instead"
),
on(
_Self = "std::cell::Cell<u8>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU8` instead",
),
on(
_Self = "std::cell::Cell<u16>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU16` instead",
),
on(
_Self = "std::cell::Cell<u32>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU32` instead",
),
on(
_Self = "std::cell::Cell<u64>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU64` instead",
),
on(
_Self = "std::cell::Cell<usize>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicUsize` instead",
),
on(
_Self = "std::cell::Cell<i8>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI8` instead",
),
on(
_Self = "std::cell::Cell<i16>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI16` instead",
),
on(
_Self = "std::cell::Cell<i32>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead",
),
on(
_Self = "std::cell::Cell<i64>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI64` instead",
),
on(
_Self = "std::cell::Cell<isize>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicIsize` instead",
),
on(
_Self = "std::cell::Cell<bool>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicBool` instead",
),
on(
_Self = "std::cell::Cell<T>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`",
),
on(
_Self = "std::cell::RefCell<T>",
note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead",
),
message = "`{Self}` cannot be shared between threads safely",
label = "`{Self}` cannot be shared between threads safely"
)]
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/async-await/issue-68112.drop_tracking.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | require_send(send_fut);
| ^^^^^^^^ future created by async block is not `Send`
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
note: future is not `Send` as it awaits another future which is not `Send`
--> $DIR/issue-68112.rs:34:17
|
Expand All @@ -23,6 +24,7 @@ LL | require_send(send_fut);
| ^^^^^^^^ future created by async block is not `Send`
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
note: future is not `Send` as it awaits another future which is not `Send`
--> $DIR/issue-68112.rs:43:17
|
Expand All @@ -43,6 +45,7 @@ LL | require_send(send_fut);
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
= note: required for `Arc<RefCell<i32>>` to implement `Send`
note: required because it's used within this `async fn` body
--> $DIR/issue-68112.rs:50:31
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/async-await/issue-68112.no_drop_tracking.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | require_send(send_fut);
| ^^^^^^^^ future created by async block is not `Send`
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
note: future is not `Send` as it awaits another future which is not `Send`
--> $DIR/issue-68112.rs:34:17
|
Expand All @@ -23,6 +24,7 @@ LL | require_send(send_fut);
| ^^^^^^^^ future created by async block is not `Send`
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
note: future is not `Send` as it awaits another future which is not `Send`
--> $DIR/issue-68112.rs:43:17
|
Expand All @@ -43,6 +45,7 @@ LL | require_send(send_fut);
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
= note: required for `Arc<RefCell<i32>>` to implement `Send`
note: required because it's used within this `async fn` body
--> $DIR/issue-68112.rs:50:31
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/generator/issue-68112.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ fn test1() {
require_send(send_gen);
//~^ ERROR generator cannot be sent between threads
//~| NOTE not `Send`
//~| NOTE use `std::sync::RwLock` instead
}

pub fn make_gen2<T>(t: T) -> impl Generator<Return = T> {
Expand All @@ -66,6 +67,7 @@ fn test2() {
//~| NOTE required for
//~| NOTE required by a bound introduced by this call
//~| NOTE captures the following types
//~| NOTE use `std::sync::RwLock` instead
}

fn main() {}
12 changes: 7 additions & 5 deletions tests/ui/generator/issue-68112.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | require_send(send_gen);
| ^^^^^^^^ generator is not `Send`
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
note: generator is not `Send` as this value is used across a yield
--> $DIR/issue-68112.rs:36:9
|
Expand All @@ -23,33 +24,34 @@ LL | fn require_send(_: impl Send) {}
| ^^^^ required by this bound in `require_send`

error[E0277]: `RefCell<i32>` cannot be shared between threads safely
--> $DIR/issue-68112.rs:63:18
--> $DIR/issue-68112.rs:64:18
|
LL | require_send(send_gen);
| ------------ ^^^^^^^^ `RefCell<i32>` cannot be shared between threads safely
| |
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
= note: required for `Arc<RefCell<i32>>` to implement `Send`
note: required because it's used within this generator
--> $DIR/issue-68112.rs:48:5
--> $DIR/issue-68112.rs:49:5
|
LL | || {
| ^^
note: required because it appears within the type `impl Generator<Return = Arc<RefCell<i32>>>`
--> $DIR/issue-68112.rs:45:30
--> $DIR/issue-68112.rs:46:30
|
LL | pub fn make_gen2<T>(t: T) -> impl Generator<Return = T> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
note: required because it appears within the type `impl Generator<Return = Arc<RefCell<i32>>>`
--> $DIR/issue-68112.rs:53:34
--> $DIR/issue-68112.rs:54:34
|
LL | fn make_non_send_generator2() -> impl Generator<Return = Arc<RefCell<i32>>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: required because it captures the following types: `impl Generator<Return = Arc<RefCell<i32>>>`, `()`
note: required because it's used within this generator
--> $DIR/issue-68112.rs:59:20
--> $DIR/issue-68112.rs:60:20
|
LL | let send_gen = || {
| ^^
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/generator/not-send-sync.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ LL | | });
| |_____^ `Cell<i32>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
= note: required for `&Cell<i32>` to implement `Send`
note: required because it's used within this generator
--> $DIR/not-send-sync.rs:16:17
Expand All @@ -36,6 +37,7 @@ LL | | });
| |_____^ generator is not `Sync`
|
= help: within `[generator@$DIR/not-send-sync.rs:9:17: 9:19]`, the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
note: generator is not `Sync` as this value is used across a yield
--> $DIR/not-send-sync.rs:12:9
|
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/generator/print/generator-print-verbose-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | require_send(send_gen);
| ^^^^^^^^ generator is not `Send`
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
note: generator is not `Send` as this value is used across a yield
--> $DIR/generator-print-verbose-1.rs:35:9
|
Expand All @@ -29,6 +30,7 @@ LL | require_send(send_gen);
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
= note: required for `Arc<RefCell<i32>>` to implement `Send`
note: required because it's used within this generator
--> $DIR/generator-print-verbose-1.rs:42:5
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/generator/print/generator-print-verbose-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ LL | | });
| |_____^ `Cell<i32>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
= note: required for `&'_#4r Cell<i32>` to implement `Send`
note: required because it's used within this generator
--> $DIR/generator-print-verbose-2.rs:19:17
Expand All @@ -36,6 +37,7 @@ LL | | });
| |_____^ generator is not `Sync`
|
= help: within `[main::{closure#0} upvar_tys=() {Cell<i32>, ()}]`, the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
note: generator is not `Sync` as this value is used across a yield
--> $DIR/generator-print-verbose-2.rs:15:9
|
Expand Down
1 change: 1 addition & 0 deletions tests/ui/issues/issue-7364.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | static boxed: Box<RefCell<isize>> = Box::new(RefCell::new(0));
| ^^^^^^^^^^^^^^^^^^^ `RefCell<isize>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `RefCell<isize>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
= note: required for `Unique<RefCell<isize>>` to implement `Sync`
= note: required because it appears within the type `Box<RefCell<isize>>`
= note: shared static variables must have a type that implements `Sync`
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/stdlib-unit-tests/not-sync.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | test::<Cell<i32>>();
| ^^^^^^^^^ `Cell<i32>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
note: required by a bound in `test`
--> $DIR/not-sync.rs:5:12
|
Expand All @@ -18,6 +19,7 @@ LL | test::<RefCell<i32>>();
| ^^^^^^^^^^^^ `RefCell<i32>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `RefCell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
note: required by a bound in `test`
--> $DIR/not-sync.rs:5:12
|
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ LL | test_sync(guard);
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
= note: required for `MutexGuard<'_, Cell<i32>>` to implement `Sync`
note: required by a bound in `test_sync`
--> $DIR/mutexguard-sync.rs:5:17
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/sync/suggest-cell.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
fn require_sync<T: Sync>() {}
//~^ NOTE required by this bound in `require_sync`
//~| NOTE required by this bound in `require_sync`
//~| NOTE required by this bound in `require_sync`
//~| NOTE required by this bound in `require_sync`
//~| NOTE required by a bound in `require_sync`
//~| NOTE required by a bound in `require_sync`
//~| NOTE required by a bound in `require_sync`
//~| NOTE required by a bound in `require_sync`

fn main() {
require_sync::<std::cell::Cell<()>>();
//~^ ERROR `Cell<()>` cannot be shared between threads safely
//~| NOTE `Cell<()>` cannot be shared between threads safely
//~| NOTE if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`

require_sync::<std::cell::Cell<u8>>();
//~^ ERROR `Cell<u8>` cannot be shared between threads safely
//~| NOTE `Cell<u8>` cannot be shared between threads safely
//~| NOTE if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU8` instead

require_sync::<std::cell::Cell<i32>>();
//~^ ERROR `Cell<i32>` cannot be shared between threads safely
//~| NOTE `Cell<i32>` cannot be shared between threads safely
//~| NOTE if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead

require_sync::<std::cell::Cell<bool>>();
//~^ ERROR `Cell<bool>` cannot be shared between threads safely
//~| NOTE `Cell<bool>` cannot be shared between threads safely
//~| NOTE if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicBool` instead
}
59 changes: 59 additions & 0 deletions tests/ui/sync/suggest-cell.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
error[E0277]: `Cell<()>` cannot be shared between threads safely
--> $DIR/suggest-cell.rs:12:20
|
LL | require_sync::<std::cell::Cell<()>>();
| ^^^^^^^^^^^^^^^^^^^ `Cell<()>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<()>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`
note: required by a bound in `require_sync`
--> $DIR/suggest-cell.rs:1:20
|
LL | fn require_sync<T: Sync>() {}
| ^^^^ required by this bound in `require_sync`

error[E0277]: `Cell<u8>` cannot be shared between threads safely
--> $DIR/suggest-cell.rs:17:20
|
LL | require_sync::<std::cell::Cell<u8>>();
| ^^^^^^^^^^^^^^^^^^^ `Cell<u8>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<u8>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU8` instead
note: required by a bound in `require_sync`
--> $DIR/suggest-cell.rs:1:20
|
LL | fn require_sync<T: Sync>() {}
| ^^^^ required by this bound in `require_sync`

error[E0277]: `Cell<i32>` cannot be shared between threads safely
--> $DIR/suggest-cell.rs:22:20
|
LL | require_sync::<std::cell::Cell<i32>>();
| ^^^^^^^^^^^^^^^^^^^^ `Cell<i32>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<i32>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
note: required by a bound in `require_sync`
--> $DIR/suggest-cell.rs:1:20
|
LL | fn require_sync<T: Sync>() {}
| ^^^^ required by this bound in `require_sync`

error[E0277]: `Cell<bool>` cannot be shared between threads safely
--> $DIR/suggest-cell.rs:27:20
|
LL | require_sync::<std::cell::Cell<bool>>();
| ^^^^^^^^^^^^^^^^^^^^^ `Cell<bool>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `Cell<bool>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicBool` instead
note: required by a bound in `require_sync`
--> $DIR/suggest-cell.rs:1:20
|
LL | fn require_sync<T: Sync>() {}
| ^^^^ required by this bound in `require_sync`

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.
12 changes: 12 additions & 0 deletions tests/ui/sync/suggest-once-cell.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![feature(once_cell)]

fn require_sync<T: Sync>() {}
//~^ NOTE required by this bound in `require_sync`
//~| NOTE required by a bound in `require_sync`

fn main() {
require_sync::<std::cell::OnceCell<()>>();
//~^ ERROR `OnceCell<()>` cannot be shared between threads safely
//~| NOTE `OnceCell<()>` cannot be shared between threads safely
//~| NOTE use `std::sync::OnceLock` instead
}
17 changes: 17 additions & 0 deletions tests/ui/sync/suggest-once-cell.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0277]: `OnceCell<()>` cannot be shared between threads safely
--> $DIR/suggest-once-cell.rs:8:20
|
LL | require_sync::<std::cell::OnceCell<()>>();
| ^^^^^^^^^^^^^^^^^^^^^^^ `OnceCell<()>` cannot be shared between threads safely
|
= help: the trait `Sync` is not implemented for `OnceCell<()>`
= note: if you want to do aliasing and mutation between multiple threads, use `std::sync::OnceLock` instead
note: required by a bound in `require_sync`
--> $DIR/suggest-once-cell.rs:3:20
|
LL | fn require_sync<T: Sync>() {}
| ^^^^ required by this bound in `require_sync`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
12 changes: 12 additions & 0 deletions tests/ui/sync/suggest-ref-cell.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![feature(once_cell)]

fn require_sync<T: Sync>() {}
//~^ NOTE required by this bound in `require_sync`
//~| NOTE required by a bound in `require_sync`

fn main() {
require_sync::<std::cell::RefCell<()>>();
//~^ ERROR `RefCell<()>` cannot be shared between threads safely
//~| NOTE `RefCell<()>` cannot be shared between threads safely
//~| NOTE use `std::sync::RwLock` instead
}
Loading