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

Creating PhantomUnSend and PhantomUnSync #175

Closed
Aandreba opened this issue Jan 31, 2023 · 13 comments
Closed

Creating PhantomUnSend and PhantomUnSync #175

Aandreba opened this issue Jan 31, 2023 · 13 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. T-libs-api

Comments

@Aandreba
Copy link

Proposal

This would add the types PhantomUnSend and PhantomUnSync, in the same style as the PhantomPinned type

Problem statement

To mark a type as !Send or !Sync, there is currently only two ways:

  • Use the nightly negative_impls feature
  • Use PhantomData of types that are !Send ans/or !Sync

The first solution requires nightly, and the second one is unrellyable, since the implementation of the phantom type may change and make that type Send or Sync

Motivation, use-cases

Current solution using negative_impls

#![feature(negative_impls)]

#[derive(Clone)]
#[repr(transparent)]
struct ThreadLock (std::thread::Thread);

impl ThreadLock {
    #[inline]
    pub fn wait (self) {
        std::thread::park()
    }

    #[inline]
    pub fn wake (self) {
        self.0.unpark()
    }
}

impl !Send for ThreadLock {}

Current solution using PhantomData

use core::mem::PhantomData;

#[derive(Clone)]
#[repr(transparent)]
struct ThreadLock (std::thread::Thread, PhantomData<std::sync::MutexGuard<'static, ()>>);

impl ThreadLock {
    #[inline]
    pub fn wait (self) {
        std::thread::park()
    }

    #[inline]
    pub fn wake (self) {
        self.0.unpark()
    }
}

Solution sketches

#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub struct PhantomUnSend;
#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub struct PhantomUnSync;

impl !Send for PhantomUnSend {}
impl !Sync for PhantomUnSync {}

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@Aandreba Aandreba added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Jan 31, 2023
@pitaj
Copy link

pitaj commented Jan 31, 2023

I like the symmetry with PhantomPinned. I do wonder though: How far from stabilization is negative_impls?

@Aandreba
Copy link
Author

Aandreba commented Feb 1, 2023

How far from stabilization is negative_impls?

It seems like it's still far from stabilization

@pitaj
Copy link

pitaj commented Feb 1, 2023

I assume these would live in std::marker?

@scottmcm
Copy link
Member

scottmcm commented Feb 1, 2023

Unimportant bikeshedding: I'm not sure these are exactly "phantom"?

Might be able to just call them NotSend and NotSync, or similar.

@pitaj
Copy link

pitaj commented Feb 1, 2023

What don't you think they're phantom? Phantom just means "zero sized", doesn't it? Phantom data, phantom pinned, etc

@scottmcm
Copy link
Member

scottmcm commented Feb 2, 2023

My interpretation of "phantom data" was that it has "a ghost of a T".

() is a ZST, but isn't "Phantom Unit".

But I guess PhantomPinned already broke the "ghost of a _____", so I guess Phantom here is fine too.

@traviscross
Copy link

Agreed NotSend and NotSync would be better names. We could always fix PhantomPinned by adding Pinned and deprecating PhantomPinned. If we erred by calling it PhantomPinned, we shouldn't continue down that path.

@traviscross
Copy link

This issue is receiving some new attention due to recent discussion. The reason is as follows.

Right now people sometimes write PhantomData<Cell<()>> to express core::marker::NotSync, and they sometimes write PhantomData<Cell<&'a ()>> to express invariance.

We've never told people not to do this because all of the ways of expressing these things rely on using the implicit properties of core types.

The problem is that any new property we might want to have pass through PhantomData from Cell (or any new property of Cell that would pass through PhantomData) now would infect these user-defined types that are using Cell simply to express NotSync or invariance. These new properties may make sense for Cell, but they don't necessarily make sense for user-defined types that have simply used PhantomData<Cell<..>> in this way.

I.e., that fact that users are relying on Cell rather than on more expressive marker types is inhibiting the evolution of the language.

@pitaj
Copy link

pitaj commented Oct 20, 2023

I think PhantomNotSend and PhantomNotSync are the best option.

I don't think we should use Un in the name because Unpin does not really mean "not pin". But rather "can be moved from a pin".

I think Phantom matches exactly what we want:

  • Phantom - Data<T> means "ghost of a T"
  • Phantom - Pinned means "ghost of something !Unpin
  • Phantom - NotSync means "ghost of something !Sync"
  • Phantom - NotSend means "ghost of something !Send"

@traviscross
Copy link

Fair enough. That's a reasonable interpretation.

Agreed definitely it should be NotX rather than UnX here.

@traviscross
Copy link

@rustbot labels +I-nominated

Nominating for T-libs-api discussion as this issue has been receiving renewed interest as described here. As we've been finding, not stabilizing these marker types is the opposite of conservative because it causes people to rely instead on the properties (and absence of properties) of existing types that aren't meant explicitly for this purpose, and that reliance inhibits language evolution.

The proposal on the table is to add and then stabilize:

  • core::marker::PhantomNotSync
  • core::marker::PhantomNotSend

As part of this, and for the same reasons, T-libs-api might also consider adding and stabilizing:

  • core::marker::PhantomInvariant<T: ?Sized> - So that people stop using Cell for this.
  • core::marker::PhantomContravariant<T: ?Sized> - For parity.

@rustbot rustbot added the I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. label Oct 21, 2023
@programmerjake
Copy link
Member

it might be nice to also have PhantomCovariant<T: ?Sized> and Invariant/Covariant/ContravariantLifetime<'a>

@Amanieu
Copy link
Member

Amanieu commented Dec 5, 2023

We discussed this in the libs-api meeting last week. We would prefer a language-level solution based on negative impls which would allow you to explicitly implement !Send or !Sync for a type to disable the implicit trait implementation. We are pushing the lang team to partially stabilize this feature to enable this use case.

@Amanieu Amanieu closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. T-libs-api
Projects
None yet
Development

No branches or pull requests

7 participants