Skip to content

Commit

Permalink
Rollup merge of rust-lang#76936 - danielhenrymantilla:unsafecell_get_…
Browse files Browse the repository at this point in the history
…mut, r=RalfJung

Add non-`unsafe` `.get_mut()` for `Unsafecell`

  - Tracking issue: rust-lang#76943

As discussed in: https://internals.rust-lang.org/t/add-non-unsafe-get-mut-for-unsafecell/12407

  - ### [Rendered documentation](https://modest-dubinsky-1f9f47.netlify.app/core/cell/struct.unsafecell)

This PR tries to move the sound `&mut UnsafeCell<T> -> &mut T` projection that all the "downstream" constructions were already relying on, up to the root abstraction, where it rightfully belongs, and officially blessing it.

  - this **helps reduce the amount of `unsafe` snippets out there** (_c.f._, the second commit of this PR: rust-lang@09503fd)

The fact that this getter is now expose for `UnsafeCell<T>` itself, will also help convey the idea that **`UnsafeCell` is not magical _w.r.t._ `&mut` accesses**, contrary to what some people incorrectly think.

  - Even the standard library itself at some point had such a confusion, _c.f._ this comment where there is a mention of multi-threaded (and thus _shared_) access despite dealing with exclusive references over unique ownership: https://github.com/rust-lang/rust/blob/59fb88d061544a035f3043b47594b34789204cee/library/core/src/cell.rs#L498-L499

r? @RalfJung
  • Loading branch information
Dylan-DPC authored Sep 21, 2020
2 parents 72a86bf + 5886c38 commit 4289998
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 25 deletions.
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`:
///
/// ```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 @@ -312,6 +312,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

0 comments on commit 4289998

Please sign in to comment.