Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a conversion from &mut T to &mut UnsafeCell<T> #198

Closed
JoJoJet opened this issue Mar 29, 2023 · 4 comments
Closed

Add a conversion from &mut T to &mut UnsafeCell<T> #198

JoJoJet opened this issue Mar 29, 2023 · 4 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@JoJoJet
Copy link

JoJoJet commented Mar 29, 2023

Proposal

Problem statement

There should be a safe API that converts an &mut T reference into an &mut UnsafeCell<T> reference within a certain scope.

Motivation, use-cases

When dealing with exclusive references, sometimes you may wish to opt in to temporary shared mutable access. If your type implements Copy, you can use Cell::from_mut to convert a mutable reference to a Cell, which allows interior mutability. For non-Copy types, it is often required to use UnsafeCell to allow interior mutability. Converting from &mut T to &mut UnsafeCell<T> is always sound, however there is currently no safe way to do this so the user is required to use std::mem::transmute. Use of this function is potentially error-prone, as a simple mistake might result in the returned reference having an incorrect lifetime.

For an example of a situation where this would be useful, consider this code I wrote for the bevy game engine:

// SAFETY: Converting `&mut T` -> `&UnsafeCell<T>`
// is explicitly allowed in the docs for `UnsafeCell`.
let world: &'w UnsafeCell<World> = unsafe { std::mem::transmute(world) };
Func::combine(
    input,
    // SAFETY: Since these closures are `!Send + !Sync + !'static`, they can never
    // be called in parallel. Since mutable access to `world` only exists within
    // the scope of either closure, we can be sure they will never alias one another.
    |input| self.a.run(input, unsafe { world.deref_mut() }),
    |input| self.b.run(input, unsafe { world.deref_mut() }),
)

Solution sketches

We should add an associated fn to the UnsafeCell type:

pub fn from_mut(value: &mut T) -> &mut UnsafeCell<T> {
    // SAFETY: UnsafeCell is repr(transparent).
    unsafe { &mut *(value as *mut T as *mut Self) }
}

This returns &mut UnsafeCell<T>, unlike Cell::from_mut which returns &Cell. It was a mistake for Cell::from_mut to work this way, since exclusive references are more useful due to the {Unsafe}Cell::get_mut function.

Links and related work

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@JoJoJet JoJoJet added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Mar 29, 2023
@JoJoJet
Copy link
Author

JoJoJet commented Mar 29, 2023

I have an initial implementation at rust-lang/rust#107917.

@programmerjake
Copy link
Member

do note that Cell::<T>::from_mut doesn't actually require T: Copy or even T: Sized

@JoJoJet
Copy link
Author

JoJoJet commented Mar 30, 2023

Of course. Cell is not as useful for non-Copy types though (or any types that you can't easily move around), so sometimes you need UnsafeCell.

james7132 pushed a commit to bevyengine/bevy that referenced this issue Mar 31, 2023
# Objective

The function `SyncUnsafeCell::from_mut` returns `&SyncUnsafeCell<T>`,
even though it could return `&mut SyncUnsafeCell<T>`. This means it is
not possible to call `get_mut` on the returned value, so you need to use
unsafe code to get exclusive access back.

## Solution

Return `&mut Self` instead of `&Self` in `SyncUnsafeCell::from_mut`.
This is consistent with my proposal for `UnsafeCell::from_mut`:
rust-lang/libs-team#198.

Replace an unsafe pointer dereference with a safe call to `get_mut`.

---

## Changelog

+ The function `bevy_utils::SyncUnsafeCell::get_mut` now returns a value
of type `&mut SyncUnsafeCell<T>`. Previously, this returned an immutable
reference.

## Migration Guide

The function `bevy_utils::SyncUnsafeCell::get_mut` now returns a value
of type `&mut SyncUnsafeCell<T>`. Previously, this returned an immutable
reference.
@joshtriplett joshtriplett added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label May 16, 2023
@joshtriplett
Copy link
Member

The library team discussed this, and we think this API makes sense. Approved!

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 17, 2023
…r=joshtriplett

Add a conversion from `&mut T` to `&mut UnsafeCell<T>`

Provides a safe way of downgrading an exclusive reference into an alias-able `&UnsafeCell<T>` reference.

ACP: rust-lang/libs-team#198.
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jul 18, 2023
…plett

Add a conversion from `&mut T` to `&mut UnsafeCell<T>`

Provides a safe way of downgrading an exclusive reference into an alias-able `&UnsafeCell<T>` reference.

ACP: rust-lang/libs-team#198.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants