`AssertThreadSafe` (name TBD) – a more general API for lifting conservative `!Send` or `!Sync` implementations

[steffahn]

Proposal

Problem statement

This proposes a generalization of the existing unstable API SyncUnsafeCell. The SyncUnsafeCell type – as far as I understand from the PR that adds it – serves (at least) two purposes:

• allow to be a replacement for static mut
• simplify implementation of synchronization primitives, because explicit Send or Sync implementations could be skipped.

The motivation for skipping explicit Send or Sync implementations isn’t fully explained. However my take on this is that it allows to reduce verbosity, and maybe even to increase accuracy, lowering the chance to mess up manual Send or Sync implementations: For instance, if you are implementing a synchronization primitive that closely mirrors another existing one in terms of its API – say, a custom Mutex re-implementation – then you could simply use SyncUnsafeCell<T>, together with a PhantomData<Mutex<T>> and then be confident that you got the right Send and Sync implementation to inherit from std’s Mutex. (This example also doesn’t fully work with SyncUnsafeCell<T> because unlike a Mutex, SyncUnsafeCell<T>: Sync requires T: Sync, something that makes it not a full static mut replacement, anyways, but that’s a discussion separate from this proposal which can support either of keeping or dropping this T: Sync restriction.)

My observation now is: There is another type in the standard library that comes with a conservative, but non necessary for soundness, implementation of !Send and !Sync: Raw pointers. And there, too, it’s very common one has to re-write Send and Sync implementations manually, even when the type very closely resembles another existing one.

Just to give one example: The type std::slice::Iter

pub struct Iter<'a, T: 'a> {
    ptr: NonNull<T>,
    end_or_len: *const T,
    _marker: PhantomData<&'a T>,
}

which even contains a PhantomData marker already, where it could simply inherit Send and Sync implementations from &'a T, needs to verbosely (and with a chance of typo / mistake), feature some manual unsafe impl Send and unsafe impl Sync implementations mirroring the ones from &'a T.

There likely countless other use cases of pointers used in ways where the pointer is “almost like a reference” in most ways including thread safety, and coming with an accompanying PhantomData (taking care of the lifetime). For example if one uses a pointer only because the data is not aligned properly, or to opt out of strict aliasing requirements as in aliasable::boxed::AliasableBox.

Instead of offering a full copy of UnsafeCell’s API called SyncUnsafeCell without the conservative !Sync restriction, and then addressing the above analogous problems for pointers, too, by mirroring their full API in hypothetical things like SyncConstPtr<T>, SyncMutPtr<T>, SyncNonNull<T>, we can introduce a single unified way (API) for lifting instance of types with a “lint-like !Sync and !Send implementation”, quite similar to the “lint-like” nature of !UnwindSafe being lifted by AssertUnwindSafe, I’m using the name AssertThreadSafe here.

Motivating examples or use cases

To take the above example of slice::Iter, that could then look like

pub struct Iter<'a, T: 'a> {
    ptr: AssertThreadSafe<NonNull<T>>,
    end_or_len: AssertThreadSafe<*const T>,
    _marker: PhantomData<&'a T>,
}

and needs no additional Send or Sync implementation anymore.

Existing SyncUnsafeCell<T> is replaced by AssertThreadSafe<UnsafeCell<T>>.

Solution sketch

A minimal / starting point of such an API looks as follows

use core::ptr::NonNull;
use core::cell::UnsafeCell;

#[derive(Debug, Copy, Clone)]
#[repr(transparent)]
pub struct AssertThreadSafe<T>(pub T);

unsafe impl<T> Send for AssertThreadSafe<*const T> {}
unsafe impl<T> Send for AssertThreadSafe<*mut T> {}
unsafe impl<T> Send for AssertThreadSafe<NonNull<T>> {}
// UnsafeCell has safe API that makes it usable by mutable reference or owned
// access; so the `T: Send` bound is still necessary, and `AssertThreadSafe<UnsafeCell<T>>`
// only lifts the restrictions on the `Sync` implementation.
unsafe impl<T: Send> Send for AssertThreadSafe<UnsafeCell<T>> {}


unsafe impl<T> Sync for AssertThreadSafe<*const T> {}
unsafe impl<T> Sync for AssertThreadSafe<*mut T> {}
unsafe impl<T> Sync for AssertThreadSafe<NonNull<T>> {}
unsafe impl<T> Sync for AssertThreadSafe<UnsafeCell<T>> {}

This particular approach also gives some cross-crate traits with a default impl, like `Send`, should not be specialized warnings, so I’m not sure whether or not this needs some additional (!Send + !Sync) marker field to reduce the Send and Sync implementations to just the listed ones.

Alternatively, there’s the option to make this (eventually) extensible, usable even for other crates that may feature some conservatively-!Sync types, too. The trait names are placeholders, but the idea would look like this:

use core::ptr::NonNull;
use core::cell::UnsafeCell;

/// Trait for types that (generally) don't implement Send,
/// but do so only as a precaution. The standard library types `*const T`
/// and `*mut T` are typical examples.
///
/// The types `*const T` and `*mut T` can soundly implement `Send`, even though they don't enforce
/// thread safety, because all API usage that could result in producing data races when the type
/// is sent between threads is already `unsafe`, anyway.
///
/// # Safety
/// This trait may only be implemented for types that can soundly implement `Send`.
/// This is also allowed to be implemented for types that *already do* implement `Send`.
pub unsafe trait SendIsNotUnsound {}

unsafe impl<T> SendIsNotUnsound for *const T {}
unsafe impl<T> SendIsNotUnsound for *mut T {}
unsafe impl<T> SendIsNotUnsound for NonNull<T> {}
// UnsafeCell has safe API that makes it usable by mutable reference or owned
// access; so the `T: Send` bound is still necessary, and `AssertThreadSafe<UnsafeCell<T>>`
// only lifts the restrictions on the `Sync` implementation.
unsafe impl<T: Send> SendIsNotUnsound for UnsafeCell<T> {}

/// Trait for types that (generally) don't implement Sync,
/// but do so only as a precaution. The standard library types `*const T`,
/// `*mut T`, and `UnsafeCell<T>` are typical examples.
///
/// The types `*const T`, `*mut T`, and `UnsafeCell<T>` can soundly implement `Sync`, even though they don't enforce
/// thread safety, because all API usage that could result in producing data races when the type
/// is shared immutably between threads is already `unsafe`, anyway.
///
/// # Safety
/// This trait may only be implemented for types that can soundly implement `Sync`.
/// This is also allowed to be implemented for types that *already do* implement `Sync`.
pub unsafe trait SyncIsNotUnsound {}

unsafe impl<T> SyncIsNotUnsound for *const T {}
unsafe impl<T> SyncIsNotUnsound for *mut T {}
unsafe impl<T> SyncIsNotUnsound for NonNull<T> {}
unsafe impl<T> SyncIsNotUnsound for UnsafeCell<T> {}

#[derive(Debug, Copy, Clone)]
#[repr(transparent)]
pub struct AssertThreadSafe<T>(pub T);
unsafe impl<T: SendIsNotUnsound> Send for AssertThreadSafe<T> {}
unsafe impl<T: SyncIsNotUnsound> Sync for AssertThreadSafe<T> {}

One could also support additional types from std. For example Option<NonNull<T>> comes to mind, because AssertThreadSafe<Option<NonNull<T>>> would be easier to work with perhaps than Option<AssertThreadSafe<NonNull<T>>>.

unsafe impl<T> SendIsNotUnsound for Option<NonNull<T>> {}
unsafe impl<T> SyncIsNotUnsound for Option<NonNull<T>> {}

To that end, this kind if impl could even be further generalized, at least in principle.

unsafe impl<T: SendIsNotUnsound> SendIsNotUnsound for Option<T> {}
unsafe impl<T: SyncIsNotUnsound> SyncIsNotUnsound for Option<T> {}

though it’s hard to say where to stop, and also this all can be happening subsequently, anyways.

Beyond the above #[derive(Debug, Copy, Clone)], AssertThreadSafe could implement other traits, too. In particular Default. Furthermore, Deref/DerefMut (pointing to the wrapped type, similar to how ManuallyDrop does it) would also be an option. The field of AssertThreadSafe could also be private instead of public, and/or named.

Alternatives

Removing or keeping SyncUnsafeCell as-is is the alternatives regarding UnsafeCell. For pointer types: As already mentioned in the “Problem Statement” section, one could also offer non-!Sync/!Send pointer types via a dedicated API, such as individual types SyncConstPtr<T>, SyncMutPtr<T>, SyncNonNull<T>, or a wrapper only for the pointer types. But if those would be introduced in the future, we definitely need to compare the option of multiple separate types, or a smaller-scope wrapper type, to the unified API from this ACP.

Or we can decide non-!Sync/!Send pointer types will never be wanted enough, suggest relying on the crate ecosystem for anyone that may find use in it, in which case, for UnsafeCell alone, a generalized wrapper like this may be overkill.

Links and related work

Existing discussions on SyncUnsafeCell should be relevant, I don’t have a full overview of the most important ones at hand; I can update this section if anything relevant/related gets suggested.

I’ve read rust-lang/rust#53639 (comment)
and the answer rust-lang/rust#53639 (comment)
where a hypothetical wrapper type came up being called UnsafeSync<UnsafeCell<T>> , and my proposal is partially inspired by that, even though I’m not sure what API (and most likely something different from this) was envisioned for “UnsafeSync” in that comment. Also the observation in the answer that

On names, two alternative possibilities to evaluate would be something like UnsafeSync<UnsafeCell<T>> / UnsafeCell<UnsafeSync<T>> and something with a new defaulted type parameter like UnsafeCell<T, Sync>.

For UnsafeSync separated from UnsafeCell to be useful it would have to be Sync unconditionally, but with a "dual-purpose" SyncUnsafeCell type we can have impl Sync for SyncUnsafeCell<T> where T: Sync with a bound. Now that I’ve typed this out I’m not sure which is preferable.

doesn’t apply here, because AssertThreadSafe<UnsafeCell<T>> can support requiring T: Sync for Self: Sync, just as it does (and has to do to stay sound) for Send. As mentioned above, I’m questioning whether that T: Sync is even wanted, but it works either way. (Just not both ways, in case we wanted both versions for some reason.¹) (Here the idea to distinguish SyncUnsafeCell from a different AlwaysSyncCell is mentioned, for instance.)

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

Footnotes

1. Or maybe it can even work both ways, dropping the T: Sync requirement only once AssertThreadSafe<AssertThreadSafe<UnsafeCell<T>>> is used to really emphasize all thread-safety that can safely be dropped will be dropped. ↩

[jplatte]

One possible alternative / addition would be to introduce separate AssertSend and AssertSync wrappers that are always Send and Sync, with the wrapped field being private and construction either requiring those new marker traits (new), or being unsafe (new_unchecked).

[m-ou-se]

Just some thoughts on the problem you're addressing, not on the specific proposal to solve it:

---

I very much agree that the !Sync/!Send-ness of pointers and UnsafeCell should be 'weaker' than that of other types (like some MutexGuards, etc.).

It's annoying how this does not compile:

fn main() {
    let a: *const i32 = &10;
    std::thread::scope(|s| {
        s.spawn(|| {
            println!("{a:?}");
        });
    })
}

But this does:

 fn main() {
     let a: *const i32 = &10;
+    let a = a as usize;
     std::thread::scope(|s| {
         s.spawn(|| {
+            let a = a as *const i32;
             println!("{a:?}");
         });
     })
 }

One one hand it's good that this forces you to properly encapsulate the pointer (and the related unsafety) in a type, although in this trivial example (just printing an address), there is no unsafety at all to be encapsulated.

(I suppose that in any real world situation, you'd only use a pointer when doing something unsafe with it, so it's good to force encapsulation. But I've noticed that this is can cause confusion and annoyance when learning or explaining Rust with small snippets.)

This "not Send/Sync but could safely be Send/Sync" property only really applies to four types (*const T, *mut T, NonNull<T>, UnsafeCell<T>), and I wonder how things would have looked liked if we were to design Rust today. Perhaps they would all be 'thread safe' by default, with a NotThreadSafe<…> wrapper? Or would we still want their default to be the same as today? Or would we have a language syntax like *const+Sync T or something? Or would there be another way to override Send/Sync for pointers without using unsafe?

I've often thought that this problem should be solved at the language level, but this ACP shows that we can also provide a reasonable solution/workaround in the library. Whether this is the best solution we can provide is a good question.

Making pointers Send and Sync today would make existing code unsound, as many structs today rely on a pointer field making them !Send and !Sync. We could make pointers Send and Sync in a new edition, however, if we allow for another way to make your type !Send/!Sync (negative impls or std::marker::NotSync or something). But that would cause any code that is not carefully fixed/updated (including sample snippets from books etc.) to become wildly unsound when copied into a newer edition.

[m-ou-se]

The Iter example in the ACP is one of many many examples where a pointer (or UnsafeCell) forces us to write a manual (unsafe) Send and Sync implementation which is easy to get wrong, even though it would have worked perfectly fine had pointers been Send+Sync. When working on low level code, I run into these situations daily.

But I'm not sure if this solution makes it much better:

pub struct Iter<'a, T: 'a> {
    ptr: AssertThreadSafe<NonNull<T>>,
    end_or_len: AssertThreadSafe<*const T>,
    _marker: PhantomData<&'a T>,
}

Perhaps it's just a naming problem, but this reads to me as if we're a) making an unsafe assumption, b) making an assumption about the Send/Sync-ness of T itself.

I'm trying to imagine how I'd explain AssertThreadSafe<T> to someone. (I imagine I'd be using this type in my book on atomics, so I'd have to be able to explain it clearly.) But I don't think I could explain this in a paragraph or two in a way that would just "click" for most people. (It says "assert" but it's not an assert, it says "safe" but it's not removing "unsafe" from anything, but rather adding Send/Sync, etc.)

More importantly, even with this AssertThreadSafe workaround, this Iter still requires a PhantomData to make it sound. If the marker is forgotten, things will be unsound. In a way, the AssertThreadSafe<NonNull<T>> and AssertThreadSafe<*const T> types here assert a bit too much and need to be restricted by the PhantomData. :(

[m-ou-se]

That Iter struct should perhaps use something like a SyncNonNull<T> that still requires T: Sync to be Send; it doesn't need the overpowered "AssertThreadSafe" tool to completely ignore thread safety (just to add it back with the PhantomData).

Which makes me wonder how often one really wants AssertThreadSafe vs something weaker ("safer"?).

I suppose that's the exact same question as whether SyncUnsafeCell<T> should be Sync for T: !Sync:

type	Send	Sync
*const T	never	never
SyncConstPtr<T>	if T: Sync	if T: Sync
AssertThreadSafe<*const T>	always	always
UnsafeCell<T>	if T: Send	never
SyncUnsafeCell<T>	if T: Send	if T: Sync
AssertThreadSafe<UnsafeCell<T>>	if T: Send	always

You mention that SyncUnsafeCell is not a full static mut replacement, but that example is about an SyncUnsafeCell<*const ()>. I don't agree that the problem lies with SyncUnsafeCell, but rather with *const (): SyncUnsafeCell<AssertThreadSafe<*const ()>> would have worked fine.

More importantly, SyncUnsafeCell<SyncConstPtr<()>> would also have worked fine, without using the (perhaps overpowered?) AssertThreadSafe.

I can think of a few use cases where one might use an AssertThreadSafe<UnsafeCell<T>> that is still Sync when T: !Sync, such as a Mutex<T> implementation.

But.. in those cases, are we really asserting something about the UnsafeCell, or about the T? That is, should we be looking at Something<UnsafeCell<T>> or UnsafeCell<Something<T>>?

For the latter, we already have that exact type (unstably): Exclusive.

UnsafeCell<Exclusive<T>> seems like it could be the right type for the value field of a Mutex<T>.

type	Send	Sync
*const T	never	never
SyncConstPtr<T>	if T: Sync	if T: Sync
SyncConstPtr<Exclusive<T>>	always	always
UnsafeCell<T>	if T: Send	never
SyncUnsafeCell<T>	if T: Send	if T: Sync
SyncUnsafeCell<Exclusive<T>>	if T: Send	always

This idea basically splits up the functionality of AssertThreadSafe into two: 1. adding Sync to a pointer/cell, and 2. ignoring !Sync on T, with both steps having their own type, allowing you to pick one, the other, or both.

Would that be better, or just be harder to explain and get right? It feels "safer" to me.

[steffahn]

Perhaps it's just a naming problem, but this reads to me as if we're a) making an unsafe assumption, b) making an assumption about the Send/Sync-ness of T itself.

More importantly, even with this AssertThreadSafe workaround, this Iter still requires a PhantomData to make it sound. If the marker is forgotten, things will be unsound. In a way, the AssertThreadSafe<NonNull<T>> and AssertThreadSafe<*const T> types here assert a bit too much and need to be restricted by the PhantomData. :(

Thanks for the feedback on the naming. I think it’s safe to say that including the word “assert” has its problems, and it now seems like using it for thread-safety traits (Send/Sync) can have even worse interpretations than its usage with unwind safety (AssertUnwindSafe). On the other hand, AssertUnwindSafe is probably the worse offender in incorrectly conveying it had something to do with “unsafe assumptions”.

More than asserting anything about the contained type, instead “AssertThreadSafe” asserts that the context the value is used in uses it in a thread-safe manner. It means “in this case of using this type that has to do with raw pointers, I will¹ not only make sure that all pointer accesses are valid, but also that they are safe (or safely encapsulated) for multi-threaded contexts”. Not that that is something particularly new, because coercions to and from usize could already me used to send pointers to other threads without typing out the words “unsafe” at the very place where that conversion or sending happens.

---

That Iter struct should perhaps use something like a SyncNonNull<T> that still requires T: Sync to be Send

Let’s not forget about IterMut, which wouldn’t be able to profit from a SyncNonNull that requires T: Sync for SyncNonNull<T>: Send.

pub struct IterMut<'a, T: 'a> {
    ptr: NonNull<T>,
    end_or_len: *mut T,
    _marker: PhantomData<&'a mut T>,
}

---

You mention that SyncUnsafeCell is not a full static mut replacement, but that example is about an SyncUnsafeCell<*const ()>. I don't agree that the problem lies with SyncUnsafeCell, but rather with *const (): SyncUnsafeCell<AssertThreadSafe<*const ()>> would have worked fine.

Fair point. My example was using *const () just as an example for “any non-Sync type” which is not the best example if followed up by an argument about making types like *const () less strictly non-Sync. The relevant question here was just the question whether or not all instances of static mut could – for newer editions – be eliminated, e.g. with a cargo fix kind of approach or by turning static mut into a syntactic sugar for something using an UnsafeCell-like type.

I figured it was important to point out (and demonstrate with a code example) that static mut has no Sync requirements on the contained value at all.

I don't even know what the exact reasoning was behind the design decision that static mut X: T; not require T: Sync, but it might be one of “you need to include manual synchronization anyways”, and depending on the nature of that synchronization, the “correct” bound may be any of T: Sync, T: Send, T: Send + Sync, or no bound at all. Obviously, this design decision should/would/could have been a fairly similar discussion as this one on how to handle SyncUnsafeCell and “sync” versions of raw pointers.

---

I can think of a few use cases where one might use an AssertThreadSafe<UnsafeCell<T>> that is still Sync when T: !Sync, such as a Mutex<T> implementation.

But.. in those cases, are we really asserting something about the UnsafeCell, or about the T? That is, should we be looking at Something<UnsafeCell<T>> or UnsafeCell<Something<T>>?

I think the opposite case is just as important. Many use cases will require more than T: Sync. The whole point of UnsafeCell is to enable mutations, so T: Send + Sync is a very common requirement, too. In fact, maybe the discussion of whether or not AssertThreadSafe<UnsafeCell<T>> ≅ SyncUnsafeCell<T> should have a bound on T for Self: Sync should shift entirely to discussing the candidate T: Send + Sync for SyncUnsafeCell<T>: Sync, as in this moment, I can hardly think of a use-case where T: Sync is (necessary and) sufficient.

---

Would that be better, or just be harder to explain and get right? It feels "safer" to me.

How useful Exclusive could be here is an interesting question. I think I’ll look at that a few more times before concluding how I’d feel about that. However, I think you’re also right in questioning whether that’s not just making it harder to get right. I think there’s some value in the consistency that you’ll always need an additional PhantomData marker to properly convey the access pattern to the T. And unlike this new SyncConstPtr/Exclusive/… calculus being introduced, a PhantomData at least features common familiar types, so there’s no need to think too hardly (or ask Rustdoc) to be sure you have the right bounds after all.

One would teach that raw pointers (and UnsafeCell) are !Sync + !Send for a good reason, and while one can lift this bound without requiring unsafe directly, that typically weakens the restrictions beyond the soundness requirements for use-cases, and it should always be accompanied with alternative means of ensuring thread safety: In structs that’s easy, just add PhantomData. I’m not sure I have a good enough idea of what proper use-cases usually look like that want to use an AssertThreadSafe<unsafe cell / pointer / …> type directly, like the thread::spawn one you showed, instead of in a struct field.

Footnotes

1. or maybe “I assert I will …” ↩

[steffahn]

I think the requirements between “thread-safe versions of pointers”¹ and “thread-safe versions of UnsafeCell”¹ might be sufficiently different that the two things could be discussed separately, and perhaps only unified if (or as far as) the same (or a compatible) design is reached. I will consider perhaps opening an IRLO thread about “thread-safe versions of pointers” alone.

It could even be the case that SyncUnsafeCell<T> (that requires T: Send + Sync for Self: Sync) is deemed useful in addition to a version that doesn’t place such a restriction, and then the version that more closely mirrors the approach taken for pointers could possibly get a unified wrapper API type.

For example… if in the end of such a discussion, “thread-safe versions of pointers” doesn’t get any T: Sync or T: Send requirements, then AssertThreadSafe<*const T> and AssertThreadSafe<UnsafeCell<T>> both do so consistently, and SyncUnsafeCell<T> could coexist as the alternative that does put bounds on T.

Footnotes

1. This sounds not quite right, and also relates to the general difficulty of naming these types. Anything that just puts “sync” in its name has the same kind of issue, including SyncUnsafeCell: A “thread-safe version of pointers” or say a “SyncConstPtr<T>” or in fact SyncUnsafeCell<T> might be misunderstood as being something to ordinary raw pointers as Arc<T> is to Rc<T>, i.e. the implementation somehow checks thread-safety and makes this pointer safer, even though the opposite is the case, and the “Sync” version of the pointer is less safe to use, and does nothing in terms of additional checks. ↩ ↩²

[the8472]

It's annoying how this does not compile:

fn main() {
    let a: *const i32 = &10;
    std::thread::scope(|s| {
        s.spawn(|| {
            println!("{a:?}");
        });
    })
}

But this does:

 fn main() {
     let a: *const i32 = &10;
+    let a = a as usize;
     std::thread::scope(|s| {
         s.spawn(|| {
+            let a = a as *const i32;
             println!("{a:?}");
         });
     })
 }

One one hand it's good that this forces you to properly encapsulate the pointer (and the related unsafety) in a type, although in this trivial example (just printing an address), there is no unsafety at all to be encapsulated.

Under provenance rules (which ralf is going to RFC) this might become a dubious justification. Do we have any other way to smuggle pointers between scopes that would remain valid?

[steffahn]

Do we have any other way to smuggle pointers between scopes that would remain valid?

Probably conversion to and from AtomicPtr does the same.

[steffahn]

AtomicPtr<T> is an interesting thing to compare against, anyways, as it does not place any constraint on T either. Thus any SyncConstPtr<T>-like type would also be more consistent with the precedent of AtomicPtr if it didn’t restrict T: Sync for Self: Sync.

---

Here’s the relevant code example

 fn main() {
+    use core::sync::atomic::AtomicPtr;
     let a: *const i32 = &10;
+    let a = AtomicPtr::new(a.cast_mut());
     std::thread::scope(|s| {
         s.spawn(|| {
+            let a = a.into_inner() as *const i32;
             println!("{a:?}");
         });
     })
 }

[joshtriplett]

A thought to consider before stabilization of an API like this: Given some better trait coherence handling, we could instead write this such that it can't be instantiated at all on certain types (e.g. MutexGuard), rather than such that it can be instantiated but isn't actually Send:

This is not a blocker for the ACP. It's something I'd like added to unresolved questions in the corresponding tracking issue for the unstable feature if this is added. That way, if coherence has improved by the time we go to stabilize this, we can use it, and if it hasn't, we'll probably go ahead with this as-is.

[steffahn]

@joshtriplett Can you give some more context to this? This sounds a bit like a thought I’d had, too, that supporting types T inside of the AssertThreadSafe<T> that don’t implement the [Send|Sync]IsNotUnsound traits at all can be undesired. But I’m curious what your thinking is here, too. Also I’m not sure what “coherence handling” you have in mind that helps here. For comparison, these are my thoughts on this so far.

---

Here’s my reasoning why that could be undesired:

There’s the point that “there’s little use in supporting it”, because the indended use-case of AssertThreadSafe is to use it always with concrete types like AssertThreadSafe<*const T> or AssertThreadSafe<UnsafeCell<T>>, not a generically as AssertThreadSafe<T> with a generic parameter T or T: SomeTrait from somewhere. With concrete types, the AssertThreadSafe is either useless and does nothing, it you put a type into it that is supported in which case that’s exactly the usage that would still be allowed.

Besides the point that there’s no reason against such a restriction, I could see the downside that something like AssertThreadSafe<OtherType<T>>, where the “AssertThreadSafe” does essentially nothing, can be harmful:

User’s structs can include a !Send or !Sync type as field and rely on it to mark the whole type non threadsafe, in turn relying on that automatic !Send and/or !Sync for soundness. Even though that’s only actually proper when using a type that somehow promises never to implement Send/Sync in the future, still there’s no need to create more opportunity for issues.¹

---

And now how it could be achieved: I don’t seem to be able to spot any blockers or language features that could help make this any more easily achieved, but I’m curious to learn more. I though, it could be a simple trait bound on the struct.

• Make one trait trait CanAssertThreadSafe for all types that can be put into AssertThreadSafe. (So it’s struct AssertThreadSafe<T: CanAssertThreadSafe>(pub T); then.)
• Additionally have the two traits SendIsNotUnsound & SyncIsNotUnsound like before. Maybe these could be renamed to something relating to AssertThreadSafe, too. Yeah, I still have no good naming ideas here.
• Maybe establish a supertrait relation [Send|Sync]IsNotUnsound: CanAssertThreadSafe to make sure a sensible CanAssertThreadSafe implementation isn’t forgotten.

The third trait is sometimes more generally implemented and thus necessary. E.g. UnsafeCell<T>: Send absolutely needs T: Send for soundness. However we don’t want to limit a struct using AssertUnwindSafe<UnsafeCell<T>> to only T: Send types. So one would have

impl<T> CanAssertThreadSafe for UnsafeCell<T> {}

fully general, but

impl<T: Send> SendIsNotUnsound for UnsafeCell<T> {}

with the T: Send restriction.

---

Also I’m right now realizing that I’m missing a lot of ?Sized bounds everywhere in my code examples. Shouldn’t forget that in any actual implementation either :-)

Footnotes

1. E.g. assume for now Option<NonNull<T>> doesn’t implement [Send|Sync]IsNotUnsound. Then someone for some reason uses AssertThreadSafe<Option<NonNull<T>> nonetheless, is perhaps confused that it doesn’t work as expected in lifting any bounds, but ends up not removing the useless AssertThreadSafe wrapper anyways, and then modifies their type further that it turns out it shouldn’t ever be considered thread-safe to begin with, but the type is only !Sync because of the AssertThreadSafe<Option<NonNull<T>>.

   Finally, at some point in the future Option<NonNull<T>> gets the SyncIsNotUnsound implementation after all, and that breaks the user’s type’s soundness. Not the most realistic scenario… sure… but maybe it’s even enough of an argument to say we don’t want the confusing “try AssertThreadSafe<Option<NonNull<T>> and be confused why it didn’t change anything” experience to be able to happen in the first place. ↩

[theemathas]

How about naming the types UncheckedSend and UncheckedSync? This would, in my opinion, very clearly communicate the intent of what the types should do.

As for the API:

• They would unconditionally implement Send and Sync, respectively. This makes the common case of statics convenient. As for the case of struct fields, users can use PhantomData to make the Send and Sync conditional on a generic type, since a PhantomData is probably needed anyway.
• For construction, the new() function would be unsafe. All other operations would be safe. This avoids littering the codebase with unsafe everywhere, making usage more convenient.
• The two types would implement Deref and DerefMut. This minimizes the burden of having to navigate yet another wrapper type. This is sound because new() is unsafe.