OnceCell mut API extension

[danielSanchezQ]

Motivation:

I've experience that sometimes you want to init a type and get a mutable reference to it. Right now the way of doing that is to use use any of the available API calls that can initialise the value, depending on the needs:

• get_or_init
• get_or_try_init
• try_insert

Then, discard the &T returned and use get_mut to get a &mut T. For example:

let mut cell: OnceCell<u32> = OnceCell::new();
_ = cell.get_or_init(|| 10);
let v = cell.get_mut().unwrap_or_else(|| unreachable!());
*v += 1;

Playground

let v = cell.get_mut().unwrap_or_else(|| unreachable!()); looks like a code smell.

This, even if working, breaks a couple of things:

• It is not intuitive, almost every API in the std counts with its mut counterpart (see slices or vec)
• forces to write extra calls where we could just return a &mut T at the time we need

Proposition:

Implement mut versions of the API calls already implemented. To get something like:

let mut cell: OnceCell<u32> = OnceCell::new();
let v = cell.get_mut_or_init(|| 10);
*v += 1;

[danielSanchezQ]

This was once implemented as part of #193 , but was removed. I keep a branch with it implemented which we could bring back and work with again.

[matklad]

My gut feeling is that this is a rather narrow use-case, and that it isn't worth to duplicate many API functions. The .set(...); .get_mut().unwrap() workaround seems OK enough.

If we do want to add anything here, it would be, perhaps,

try_insert_mut(&mut self, value: T) -> Result<&mut T, (&mut T, T)>

as try_insert is intended as the most general API to use when one needs control. For _or_inig variants specifically, I don't think they are worth is especially because with &mut the caller can write the if cell.get().is_none() themselves.

[danielSanchezQ]

My gut feeling is that this is a rather narrow use-case, and that it isn't worth to duplicate many API functions. The .set(...); .get_mut().unwrap() workaround seems OK enough.

Using set force you to build the value each time instead of being something lazy.

If we do want to add anything here, it would be, perhaps,

try_insert_mut(&mut self, value: T) -> Result<&mut T, (&mut T, T)>

as try_insert is intended as the most general API to use when one needs control.

Well, better this than nothing yes.

For _or_inig variants specifically, I don't think they are worth is especially because with &mut the caller can write the if cell.get().is_none() themselves.

I don't really understand this 😅

[matklad]

I don't really understand this sweat_smile

So, the reason why we need .get_or_init() is because the following code:

let cell: &OnceCell<T> = ...;
let r: &T = match cell.get() {
    Some(it) => it,
    None => {
        cell.set(f());
        cell.get().unwrap()
    }
};

would be racy -- the .set() might not actually set the value, because it might be filled by some different thread. The API with closure is needed to ensure atomicity, that the closure is only called once.

However, if we have &mut OnceCell, than the above code will be correct -- exclusive reference guarantees that there are no other threads that can sneakily set the value.

[danielSanchezQ]

Oh, ok, I get it now. Imo it doesn't do harm to have them. But I understand that keeping simple is a good choice too.
Maybe we could go with try_insert_mut(&mut self, value: T) -> Result<&mut T, (&mut T, T)> as you commented, which would be enough for the other cases without adding the other extra methods. Your call @matklad !
And thanks for taking the time to explain everything!!

[matklad]

I think I am inclined to not add these for now!