matklad / once_cell

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

Feature request: Implement Clone for sync::OnceCell #31

Closed manuthambi closed 5 years ago

manuthambi commented 5 years ago

Is there a reason Clone is not implemented for sync::OnceCell? It is implemented for unsync::OnceCell. The logic can be identical to what is done for unsync.

Without Clone, I am forced to manually implement Clone for structs with a OnceCell member.

matklad commented 5 years ago

Because it contains a Mutex inside, a call to .clone could block the thread, and that is highly surprising behavior.

manuthambi commented 5 years ago

Thanks for pointing it out. A blocking clone would indeed be surprising.

But I am suggesting an implementation like this:

fn clone(&self) -> OnceCell<T> {
    let res = OnceCell::new();
    if let Some(value) = self.get() {
        match res.set(value.clone()) {
             Ok(()) => (),
             Err(_) => unreachable!(),
        }
    }
    res
}

The above only requires get(), and get() as it is currently implemented does not block (for both imp_pl and imp_std). It just uses is_initialized atomic boolean. It returns None if the OnceCell is in the process of being initialized (and the lock held).

I think we should specify that get() never blocks, because doing otherwise would be a very surprising implementation.

matklad commented 5 years ago

Hm, that makes sense actually, let's do this! Could you send a PR?