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 non-unsafe .get_mut() for Unsafecell #76936

Merged
merged 2 commits into from
Sep 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 86 additions & 15 deletions library/core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,10 +496,7 @@ impl<T: ?Sized> Cell<T> {
#[inline]
#[stable(feature = "cell_get_mut", since = "1.11.0")]
pub fn get_mut(&mut self) -> &mut T {
// SAFETY: This can cause data races if called from a separate thread,
// but `Cell` is `!Sync` so this won't happen, and `&mut` guarantees
// unique access.
unsafe { &mut *self.value.get() }
self.value.get_mut()
}

/// Returns a `&Cell<T>` from a `&mut T`
Expand Down Expand Up @@ -945,8 +942,7 @@ impl<T: ?Sized> RefCell<T> {
#[inline]
#[stable(feature = "cell_get_mut", since = "1.11.0")]
pub fn get_mut(&mut self) -> &mut T {
// SAFETY: `&mut` guarantees unique access.
unsafe { &mut *self.value.get() }
self.value.get_mut()
}

/// Undo the effect of leaked guards on the borrow state of the `RefCell`.
Expand Down Expand Up @@ -1543,8 +1539,11 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
/// allow internal mutability, such as `Cell<T>` and `RefCell<T>`, use `UnsafeCell` to wrap their
/// internal data. There is *no* legal way to obtain aliasing `&mut`, not even with `UnsafeCell<T>`.
///
/// The `UnsafeCell` API itself is technically very simple: it gives you a raw pointer `*mut T` to
/// its contents. It is up to _you_ as the abstraction designer to use that raw pointer correctly.
/// The `UnsafeCell` API itself is technically very simple: [`.get()`] gives you a raw pointer
/// `*mut T` to its contents. It is up to _you_ as the abstraction designer to use that raw pointer
/// correctly.
///
/// [`.get()`]: `UnsafeCell::get`
///
/// The precise Rust aliasing rules are somewhat in flux, but the main points are not contentious:
///
Expand All @@ -1571,21 +1570,70 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
/// 2. A `&mut T` reference may be released to safe code provided neither other `&mut T` nor `&T`
/// co-exist with it. A `&mut T` must always be unique.
///
/// Note that while mutating or mutably aliasing the contents of an `&UnsafeCell<T>` is
/// ok (provided you enforce the invariants some other way), it is still undefined behavior
/// to have multiple `&mut UnsafeCell<T>` aliases.
/// Note that whilst mutating the contents of an `&UnsafeCell<T>` (even while other
/// `&UnsafeCell<T>` references alias the cell) is
/// ok (provided you enforce the above invariants some other way), it is still undefined behavior
/// to have multiple `&mut UnsafeCell<T>` aliases. That is, `UnsafeCell` is a wrapper
/// designed to have a special interaction with _shared_ accesses (_i.e._, through an
/// `&UnsafeCell<_>` reference); there is no magic whatsoever when dealing with _exclusive_
/// accesses (_e.g._, through an `&mut UnsafeCell<_>`): neither the cell nor the wrapped value
/// may be aliased for the duration of that `&mut` borrow.
/// This is showcased by the [`.get_mut()`] accessor, which is a non-`unsafe` getter that yields
/// a `&mut T`.
///
/// [`.get_mut()`]: `UnsafeCell::get_mut`
///
/// # Examples
///
/// Here is an example showcasing how to soundly mutate the contents of an `UnsafeCell<_>` despite
/// there being multiple references aliasing the cell:
///
/// ```
/// use std::cell::UnsafeCell;
///
/// # #[allow(dead_code)]
/// struct NotThreadSafe<T> {
/// value: UnsafeCell<T>,
/// let x: UnsafeCell<i32> = 42.into();
/// // Get multiple / concurrent / shared references to the same `x`.
/// let (p1, p2): (&UnsafeCell<i32>, &UnsafeCell<i32>) = (&x, &x);
///
/// unsafe {
/// // SAFETY: within this scope there are no other references to `x`'s contents,
/// // so ours is effectively unique.
/// let p1_exclusive: &mut i32 = &mut *p1.get(); // -- borrow --+
/// *p1_exclusive += 27; // |
/// } // <---------- cannot go beyond this point -------------------+
///
/// unsafe {
/// // SAFETY: within this scope nobody expects to have exclusive access to `x`'s contents,
/// // so we can have multiple shared accesses concurrently.
/// let p2_shared: &i32 = &*p2.get();
/// assert_eq!(*p2_shared, 42 + 27);
/// let p1_shared: &i32 = &*p1.get();
/// assert_eq!(*p1_shared, *p2_shared);
/// }
/// ```
///
/// unsafe impl<T> Sync for NotThreadSafe<T> {}
/// The following example showcases the fact that exclusive access to an `UnsafeCell<T>`
/// implies exclusive access to its `T`:
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
///
/// ```rust
/// #![feature(unsafe_cell_get_mut)]
/// #![forbid(unsafe_code)] // with exclusive accesses,
/// // `UnsafeCell` is a transparent no-op wrapper,
/// // so no need for `unsafe` here.
/// use std::cell::UnsafeCell;
///
/// let mut x: UnsafeCell<i32> = 42.into();
///
/// // Get a compile-time-checked unique reference to `x`.
/// let p_unique: &mut UnsafeCell<i32> = &mut x;
/// // With an exclusive reference, we can mutate the contents for free.
/// *p_unique.get_mut() = 0;
/// // Or, equivalently:
/// x = UnsafeCell::new(0);
///
/// // When we own the value, we can extract the contents for free.
/// let contents: i32 = x.into_inner();
/// assert_eq!(contents, 0);
/// ```
#[lang = "unsafe_cell"]
#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -1663,6 +1711,29 @@ impl<T: ?Sized> UnsafeCell<T> {
self as *const UnsafeCell<T> as *const T as *mut T
}

/// Returns a mutable reference to the underlying data.
///
/// This call borrows the `UnsafeCell` mutably (at compile-time) which
/// guarantees that we possess the only reference.
///
/// # Examples
///
/// ```
/// #![feature(unsafe_cell_get_mut)]
/// use std::cell::UnsafeCell;
///
/// let mut c = UnsafeCell::new(5);
/// *c.get_mut() += 1;
///
/// assert_eq!(*c.get_mut(), 6);
/// ```
#[inline]
#[unstable(feature = "unsafe_cell_get_mut", issue = "76943")]
pub fn get_mut(&mut self) -> &mut T {
// SAFETY: (outer) `&mut` guarantees unique access.
unsafe { &mut *self.get() }
}

/// Gets a mutable pointer to the wrapped value.
/// The difference to [`get`] is that this function accepts a raw pointer,
/// which is useful to avoid the creation of temporary references.
Expand Down
6 changes: 2 additions & 4 deletions library/core/src/sync/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,8 +838,7 @@ impl<T> AtomicPtr<T> {
#[inline]
#[stable(feature = "atomic_access", since = "1.15.0")]
pub fn get_mut(&mut self) -> &mut *mut T {
// SAFETY: the mutable reference guarantees unique ownership.
unsafe { &mut *self.p.get() }
self.p.get_mut()
}

/// Get atomic access to a pointer.
Expand Down Expand Up @@ -1275,8 +1274,7 @@ assert_eq!(some_var.load(Ordering::SeqCst), 5);
#[inline]
#[$stable_access]
pub fn get_mut(&mut self) -> &mut $int_type {
// SAFETY: the mutable reference guarantees unique ownership.
unsafe { &mut *self.v.get() }
self.v.get_mut()
}
}

Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@
#![feature(try_reserve)]
#![feature(unboxed_closures)]
#![feature(unsafe_block_in_unsafe_fn)]
#![feature(unsafe_cell_get_mut)]
#![feature(unsafe_cell_raw_get)]
#![feature(untagged_unions)]
#![feature(unwind_attributes)]
Expand Down
4 changes: 1 addition & 3 deletions library/std/src/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,7 @@ impl<T: ?Sized> Mutex<T> {
/// ```
#[stable(feature = "mutex_get_mut", since = "1.6.0")]
pub fn get_mut(&mut self) -> LockResult<&mut T> {
// We know statically that there are no other references to `self`, so
// there's no need to lock the inner mutex.
let data = unsafe { &mut *self.data.get() };
let data = self.data.get_mut();
poison::map_result(self.poison.borrow(), |_| data)
}
}
Expand Down
4 changes: 1 addition & 3 deletions library/std/src/sync/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,7 @@ impl<T: ?Sized> RwLock<T> {
/// ```
#[stable(feature = "rwlock_get_mut", since = "1.6.0")]
pub fn get_mut(&mut self) -> LockResult<&mut T> {
// We know statically that there are no other references to `self`, so
// there's no need to lock the inner lock.
let data = unsafe { &mut *self.data.get() };
let data = self.data.get_mut();
poison::map_result(self.poison.borrow(), |_| data)
}
}
Expand Down