Generalizing GhostCell for more flexibility in the token type

[jasoncarr0]

Hello all,

I've made two small crates which are intended to expand the scope of GhostCell to allow for more generic solutions for singletons (e.g. a single global instance, one instance created in main). This was to fix an issue I had where I couldn't keep GhostCells in a data structure and still have it be 'static for use cases that didn't need to ever access the cell.
https://crates.io/crates/singleton-cell
https://crates.io/crates/singleton-trait

These are in the "somewhat messy but working/documented" stage. I'm hoping for some critiques of the idea, any safety problems, or how I can be more clear/rigorous on the invariants for the Singleton trait (I'd be unsurprised if this is underspecified on happens-before relationships), or the Erased struct which is inherently a tiny bit more complicated than one would hope?

Is it clear how it could be used with more general token types, besides the GhostCell clone provided by the with_token function?

[Yandros]

Very nice documentation, it's well appreciated

At first glance it feels fine, although I'll have to think a bit more about it (and actually play with the code ).

I do think you might want to add some examples about the "get your hands a bit dirty unsafe" approach of using a 'static singleton such as one created in main:

fn main ()
{
    let token = {
        struct MySingleton();
        unsafe impl Singleton for MySingleton {}
        Erased::new(MySingleton())
    };
}

More generally, I wonder if some extra macros could be added to help users not write unsafe

For instance, what about:

macro_rules! new_singleton {( $pub:vis $Name:ident ) => (
    ::paste::paste! {
        $pub use [< __ $Name _helpers__ >]::$Name;
        #[allow(nonstandard_style)]
        mod [< __ $Name _helpers__ >] {
            pub
            struct $Name(());

            impl $Name {
                pub
                fn new ()
                  -> Option<Self>
                {
                    let mut ret = ::core::option::Option::None;
                    {
                        use ::std::sync::Once;
                        static ONCE: Once = Once::new();
                        &ONCE
                    }.call_once(|| {
                        ret.replace( $Name(()) );
                    });
                    ret
                }
            }

            /// Safety: construction is `Once`-guarded.
            unsafe
            impl $crate::singleton_trait::Singleton for $Name {}
        }
    }
)}

this would allow doing:

def_singleton!(MySingleton);

fn main ()
{
    // `impl Exists<impl Singleton>`
    let token = Erased::new(MySingleton::new().expect("in main"));
    …
}

[jasoncarr0]

Thanks for taking a look!

If you are playing with it, there's definitely still some rough patches I'm running into that I'd like to iron out, particularly around Exists/Erased not coercing as cleanly as I would hope. Do let me know if you run into anything here as I'd like to avoid any necessity for unsafe, although it's not a critical win to use Exists I'd like to be able to have zero-sized references be usable.

Fair point about the macros, it looks like your new_singleton macro is spot on with what I had expected / had been using. Macros have been my weak point but I'm very much open to adding that or similar to the base crate if it is possible to do so (the trait/cell crates are no-std, so it may need to be behind a feature or in a separate crate but I guess for macros it might be possible?).

[Yandros]

Actually, there is another approach which involves a tiny bit of extra runtime (that required to implement "generic static"s), but which would have the advantage of being macro-less / not requiring macros.

Imagined API:

enum MyBrand {}

fn main ()
{
    let token = Erased::new(
        SingletonFactory::new::<MyBrand>()
            .expect("`Singleton<MyBrand>` was already instanced")
    );
}

[jasoncarr0]

I think the macro has better safety properties. In this example the problem is that either:

• As in your example, MyBrand is uninhabited so that it's UB to have an instance (and technically the contract of Erased says it would also be allowed to be UB to have an erased copy; this could be avoided but it makes the safety contract less clean).
• Or the user can create an instance pretty easily by accident and has to manually abide by the safety requirements.

[jasoncarr0]

Any thoughts on this simpler version? Maybe could get a concurrency expert in here but it seems to be fine and avoids depending on std and removes bells&whistles for multi-threaded use (which frankly seems like an edge case, I'm not clear on a need for anything but safely supporting uniqueness).

macro_rules! simple_singleton {( $pub:vis $Name:ident ) => (
    ::paste::paste! {
        $pub use [< __ $Name _helpers__ >]::$Name;
        #[allow(nonstandard_style)]
        mod [< __ $Name _helpers__ >] {
            use core::sync::atomic::AtomicBool;

            fn take_lock(b: &AtomicBool) -> bool {
                use core::sync::atomic::Ordering;
                b.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed).is_ok()
            }

            pub
            struct $Name(());

            impl $Name {
                pub
                fn new ()
                  -> Option<Self>
                {
                    let mut ret = ::core::option::Option::None;
                    let first_run = {
                        use core::sync::atomic::AtomicBool;
                        static ONCE: AtomicBool = AtomicBool::new(false);
                        &ONCE
                    };
                    if take_lock(first_run) {
                         // SAFETY
                        // We never store to the lock: it begins the program as false
                        // And CAS guarantees that only one thread will succeed before it is seen as true by all other threads
                        ret.replace( $Name(()) );
                    }
                    ret
                }
            }

            /// Safety: construction is guarded by an atomic lock.
            unsafe
            impl ::singleton_trait::Singleton for $Name {}
        }
    }
)}