`Arc` should only require `Sync`, not `Send`

[pythonesque]

The original justification for this was from issue #2788. I am pretty confident that this reasoning no longer applies, especially now that we have Sync. The reason this is safe is that Arc<T> will not be Send unless T is, and if you try to pass Arc<T> into spawn() without Send it won't work anyway.

(Making Arc<T> only Send sometimes may not have been doable prior to unsafe traits, so this issue may have depended on #20119 being resolved. But I think the way Arc is defined this isn't true).

[pythonesque]

I guess the new way things are defined, Arc should really require Sync for Sync and Send + Sync for Send.

[alexcrichton]

Now that we have opt-in Send/Sync I think that this should be quite easy to encode (the latter T: Sync => Arc<T>: Sync and T: Send + Sync => Arc<T>: Send).

I also agree that the bounds on construction and other functions can all be removed once these are in place.

[flaper87]

#20119 not only relaxed the Send bound but also the Sync bound. 8818693

I failed at giving a better explanation in the commit message but, one of the main reasons behind this is that it is reasonable to have something ref-counted that never escapes the thread. I know @alexcrichton and @nikomatsakis have discussed this a bit more. As far as I can tell, the reasoning behind this change makes sense to me. Probably we need a new type ?

[pythonesque]

Yeah, I saw that a bit after creating this issue. However, we do still require Sync + Send for both Sync and Send, and I think Sync only really requires Sync (but I doubt this distinction will matter to anyone at all in present day Rust).

[quantheory]

I doubt this distinction will matter to anyone at all in present day Rust

I should point out that this did cause some confusion to me about a week ago (i.e. within days after this was posted). I'm perfectly willing to admit that I might be weird in this respect, but I think that for pedagogical reasons it is useful to have pretty tight bounds, even if in practice most user types that implement Sync will also implement Send.

[huonw]

I believe making this change will cause the following data structure to become invalid:

use std::marker::PhantomData;
use std::sync::Arc;
use std::sync::atomic::{AtomicPtr, Ordering};
use std::mem;

struct ArcCell<T> {
     x: AtomicPtr<T>,
     _marker: PhantomData<Arc<T>>,
}

impl<T> ArcCell<T> {
    fn new(x: Arc<T>) -> ArcCell<T> {
        ArcCell { x: AtomicPtr::new(unsafe {mem::transmute(x)}), _marker: PhantomData }
    }

    fn swap(&self, y: Arc<T>, order: Ordering) -> Arc<T> {
        unsafe {
             mem::transmute(self.x.swap(mem::transmute(y), order))
        }
    }
}

Because it allows for destroying Arcs on different threads without Sending the Cell.

let cell = ArcCell::new(Arc::new(1));

thread::scoped(|| { // (or equivalent)
     let a = cell.swap(Arc::new(2), Ordering::SeqCst);
     assert_eq!(*a, 1);
     // no other references, so `a` is destroyed here
})

This could be fixed by having ArcCell have a more restricted implementation of Sync, but it is theoretically a breaking change because the above is working and correct code now.

@rust-lang/libs (et al): are the bounds in existing trait implementations public guarantees for things like this, where they influence correctness?

[nikomatsakis]

There is of course no way to test for this automatically.

[aturon]

@huonw This is a pretty interesting form of negative reasoning. In some sense, this is what the "fundamental" attribute is meant to capture -- that it's a breaking change to implement the trait. I'm not sure yet what I think about it in this case.

[bluss]

It must be Sync for Arc<T> where T: Sync + Send because of @huonw's theorem 😄 that says that Sync + Copy => Send.

Sending a &Arc<T> (requires Arc<T>: Sync), you can .clone() it to get a new Arc<T> on that thread, and thus, the Arc<T> is effectively sent to another thread (and the T can be dropped on that thread), which must only be allowed if T: Send.

[aturon]

@bluss FWIW, this isn't what Send means in my mind. To me, it specifically requires moving ownership of the original value directly across threads.