Add non-`unsafe` `get_mut` for `UnsafeCell`

impl<T : ?Sized> UnsafeCell<T> {
    pub
    fn get_mut (self: &'_ mut UnsafeCell<T>) -> &'_ mut T
    {
        unsafe {
            // Safety: outer `&mut` guarantees lack of concurrency.
            &mut *self.get()
        }
    }
}

basically there is no reason not to have it (UnsafeCell is a wrapper that only interacts with &UnsafeCell inputs: an owned or exclusive UnsafeCell-ed wrapper is a no-op, much like Pin in a Pin<impl Deref[Mut]<Target = impl Unpin>>).

On the contrary, the fact there is no such get_mut() may lead some people into wrongly thinking that UnsafeCell is special even w.r.t. to &mut access.

This is a simple, straight-forward API, that can reduce the amount of unsafe code out there: whenever one sees a .get() call on an UnsafeCell, they have to think about the soundness of such potentially concurrent or even racy access, and it currently happens even when there is an outer &mut on the UnsafeCell, when it shouldn't be the case.

8 Likes

Most of the types built on UnsafeCell have their own get_mut already, but have to use unsafe to implement it. They could all be simplified with UnsafeCell::get_mut.

3 Likes

Nitpick: I think the implementation should be &mut *self.get() (note the deref).

1 Like

My nomicon knowledge is a bit shaky. Is this allowed?

let v = UnsafeCell::new(0u32);
let y: &mut u32 = {
    let used: &UnsafeCell<_> = &v;
    unsafe { &mut *used.get() }
};
let z: &mut UnsafeCell<u32> = y;
// if this were allowed, `z.get_mut()` here would
// definitely be unsound. but is creating `z`
// allowed in the first place?

If so, it wouldn't necessarily be a problem. But it'd probably be worth adding as a note in the documentation that calling get_mut isn't always going to be sound depending on what else your code does with other unsafe accesses.

You could invent similar scenarios from any pointer, regardless of cells -- I'd say the error lies in the open-ended lifetime of that reference.

Another variant I would use a lot is this:

unsafe fn get_mut(&self) -> &mut T { &mut *self.get() }

This is particularly useful as most other methods take mutable references and the whole (*a.b.get()).do_something() dance is quite awkard.

The question is, which variant should get the name get_mut? :stuck_out_tongue:

I wondered if there was a pointer method that would make chains easier -- there's only as_mut<'a>(self) -> Option<&'a mut T>, which also says:

If you are sure the pointer can never be null and are looking for some kind of as_mut_unchecked that returns the &mut T instead of Option<&mut T> , know that you can dereference the pointer directly.

... which doesn't help you.

1 Like