matklad / once_cell

Rust library for single assignment cells and lazy statics without macros
Apache License 2.0
1.87k stars 109 forks source link

Proof of Concept: making `Lazy<T, F>` covariant in `F` #230

Closed danielhenrymantilla closed 1 year ago

danielhenrymantilla commented 1 year ago

Proof of Concept of my idea from https://github.com/matklad/once_cell/issues/167#issuecomment-1564727077

If this PoC were to be merged, it would fix https://github.com/matklad/once_cell/issues/167.

Although it does so at the cost of loss of discriminant elision of Option<F>, which means it makes Lazys bigger. This could be deemed too high a cost for a somewhat niche feature.

matklad commented 1 year ago

TBH, I am off two minds here, as the following concerns conflict:

I guess, on balance, yeah, we should do that.

But before, I am curious, can we optimize this a bit? In particular, the current impl,

    pub struct Lazy<T, F = fn() -> T> {
        cell: OnceCell<T>,
        init: Cell<Option<F>>,
    }

isn't really what it should be, as we have two flags here one in OnceCell, and one in Option. And we don't share the space between T and F.

So I think ideally we want something like

struct Lazy<T, F> {
    state: enum { Init, Uninit, Poison },
    data: union {
        f: F,
        t: T,
        poison: (),
    }
}

can we make that work pedantically correct wrt variance?

matklad commented 1 year ago

I guess, as another consideration here, give that std::sync::LazyLock is almost stable, I'll probably change what this crate is about, and move it from "stable foundation" to "fancy extras", as the foundational role would be subsumed by std. From this perspective, doing something more creative here also makes sense.

danielhenrymantilla commented 1 year ago

can we make that work pedantically correct wrt variance?

Yes, by slightly amending it 🙂:

use ManuallyDrop as MD; // <- necessary red tape

struct Lazy<T, F> {
    state: SomeMutWrapper<enum { Init, Uninit, Poison }>, // Uninit=can read `f`, Init=can read 't', Poison=cannot read either
    data: union {
        f: MD<F>,
        t: MD<UnsafeCell<T>>, // 👈 `UnsafeCell` only here
    }
}

This has the right variance w.r.t. both F and T and would indeed allow squashing the is_some flag of .f in my PR within state. The only drawback is that it requires reïmplementing the flag-handling for Lazy, rather than being able to rely on the OnceCell<T> already existing abstraction.

danielhenrymantilla commented 1 year ago

I'm thus closing this draft PR, but may be submitting a PR implementing this for unsync::Lazy (the "easy" to write implementation of OnceCell). From there, I'll let sync-savy people follow-up on this and handle the sync flavor thereof