Add comments regarding superfluous !Sync impls

[bstrie]

No description provided.

[rust-highfive]

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

[yaahc]

I know you mentioned this in Discord but just to reiterate here, we should check what error messages we produce with and without the explicit !Send/!Sync impls and either remove the maybe or remove the entire parenthesized section at the end of the new comments before merging.

Once you've done the clarification please feel free to approve on my behalf.

[yaahc]

@bors delegate+

[bors]

✌️ @bstrie can now approve this pull request

[JohnCSimon]

Ping from triage:
@bstrie - fyi this was changed to S-waiting-on-author.

[JohnCSimon]

Ping from triage:
@bstrie - can you please post your status on this PR? Thanks

[bstrie]

we should check what error messages we produce with and without the explicit !Send/!Sync impls and either remove the maybe or remove the entire parenthesized section at the end of the new comments before merging.

Rather than rebuilding all of std just to test the error messages, I found it faster to copy the definition of Rc into its own file, since presumably the error messages will be the same. Here's the error with the explicit !Sync:

error[E0277]: `Rc<()>` cannot be shared between threads safely
--> src/main.rs:24:19
|
24 | requires_sync(rc);
| ------------- ^^ `Rc<()>` cannot be shared between threads safely
| |
| required by a bound introduced by this call
|
= help: the trait `Sync` is not implemented for `Rc<()>`
note: required by a bound in `requires_sync`
--> src/main.rs:27:21
|
27 | fn requires_sync<T: Sync>(t: T) {
| ^^^^ required by this bound in `requires_sync`

and the error without it:

error[E0277]: `NonNull<RcBox<()>>` cannot be shared between threads safely
--> src/main.rs:22:19
|
22 | requires_sync(rc);
| ------------- ^^ `NonNull<RcBox<()>>` cannot be shared between threads safely
| |
| required by a bound introduced by this call
|
= help: within `Rc<()>`, the trait `Sync` is not implemented for `NonNull<RcBox<()>>`
note: required because it appears within the type `Rc<()>`
--> src/main.rs:15:12
|
15 | pub struct Rc<T: ?Sized> {
| ^^
note: required by a bound in `requires_sync`
--> src/main.rs:25:21
|
25 | fn requires_sync<T: Sync>(t: T) {
| ^^^^ required by this bound in `requires_sync`
error[E0277]: `Cell<usize>` cannot be shared between threads safely
--> src/main.rs:22:19
|
22 | requires_sync(rc);
| ------------- ^^ `Cell<usize>` cannot be shared between threads safely
| |
| required by a bound introduced by this call
|
= help: within `Rc<()>`, the trait `Sync` is not implemented for `Cell<usize>`
note: required because it appears within the type `RcBox<()>`
--> src/main.rs:9:8
|
9 | struct RcBox<T: ?Sized> {
| ^^^^^
= note: required because it appears within the type `PhantomData<RcBox<()>>`
note: required because it appears within the type `Rc<()>`
--> src/main.rs:15:12
|
15 | pub struct Rc<T: ?Sized> {
| ^^
note: required by a bound in `requires_sync`
--> src/main.rs:25:21
|
25 | fn requires_sync<T: Sync>(t: T) {
| ^^^^ required by this bound in `requires_sync`

So yes, this indicates that removing this impl would be a diagnostic regression.

[bstrie]

@bors r+

[bors]

📌 Commit 86c0ef8 has been approved by bstrie

[bstrie]

@bors r+ rollup=always

[bors]

💡 This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: Rollup of 4 pull requests #90695

[bors]

📌 Commit 86c0ef8 has been approved by bstrie