Skip to content

Commit b670b86

Browse files
authored
Rollup merge of #76936 - danielhenrymantilla:unsafecell_get_mut, r=RalfJung
Add non-`unsafe` `.get_mut()` for `Unsafecell` - Tracking issue: #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: 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
2 parents 02d787b + 5886c38 commit b670b86

File tree

5 files changed

+91
-25
lines changed

5 files changed

+91
-25
lines changed

library/core/src/cell.rs

+86-15
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,7 @@ impl<T: ?Sized> Cell<T> {
496496
#[inline]
497497
#[stable(feature = "cell_get_mut", since = "1.11.0")]
498498
pub fn get_mut(&mut self) -> &mut T {
499-
// SAFETY: This can cause data races if called from a separate thread,
500-
// but `Cell` is `!Sync` so this won't happen, and `&mut` guarantees
501-
// unique access.
502-
unsafe { &mut *self.value.get() }
499+
self.value.get_mut()
503500
}
504501

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

952948
/// Undo the effect of leaked guards on the borrow state of the `RefCell`.
@@ -1543,8 +1539,11 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
15431539
/// allow internal mutability, such as `Cell<T>` and `RefCell<T>`, use `UnsafeCell` to wrap their
15441540
/// internal data. There is *no* legal way to obtain aliasing `&mut`, not even with `UnsafeCell<T>`.
15451541
///
1546-
/// The `UnsafeCell` API itself is technically very simple: it gives you a raw pointer `*mut T` to
1547-
/// its contents. It is up to _you_ as the abstraction designer to use that raw pointer correctly.
1542+
/// The `UnsafeCell` API itself is technically very simple: [`.get()`] gives you a raw pointer
1543+
/// `*mut T` to its contents. It is up to _you_ as the abstraction designer to use that raw pointer
1544+
/// correctly.
1545+
///
1546+
/// [`.get()`]: `UnsafeCell::get`
15481547
///
15491548
/// The precise Rust aliasing rules are somewhat in flux, but the main points are not contentious:
15501549
///
@@ -1571,21 +1570,70 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
15711570
/// 2. A `&mut T` reference may be released to safe code provided neither other `&mut T` nor `&T`
15721571
/// co-exist with it. A `&mut T` must always be unique.
15731572
///
1574-
/// Note that while mutating or mutably aliasing the contents of an `&UnsafeCell<T>` is
1575-
/// ok (provided you enforce the invariants some other way), it is still undefined behavior
1576-
/// to have multiple `&mut UnsafeCell<T>` aliases.
1573+
/// Note that whilst mutating the contents of an `&UnsafeCell<T>` (even while other
1574+
/// `&UnsafeCell<T>` references alias the cell) is
1575+
/// ok (provided you enforce the above invariants some other way), it is still undefined behavior
1576+
/// to have multiple `&mut UnsafeCell<T>` aliases. That is, `UnsafeCell` is a wrapper
1577+
/// designed to have a special interaction with _shared_ accesses (_i.e._, through an
1578+
/// `&UnsafeCell<_>` reference); there is no magic whatsoever when dealing with _exclusive_
1579+
/// accesses (_e.g._, through an `&mut UnsafeCell<_>`): neither the cell nor the wrapped value
1580+
/// may be aliased for the duration of that `&mut` borrow.
1581+
/// This is showcased by the [`.get_mut()`] accessor, which is a non-`unsafe` getter that yields
1582+
/// a `&mut T`.
1583+
///
1584+
/// [`.get_mut()`]: `UnsafeCell::get_mut`
15771585
///
15781586
/// # Examples
15791587
///
1588+
/// Here is an example showcasing how to soundly mutate the contents of an `UnsafeCell<_>` despite
1589+
/// there being multiple references aliasing the cell:
1590+
///
15801591
/// ```
15811592
/// use std::cell::UnsafeCell;
15821593
///
1583-
/// # #[allow(dead_code)]
1584-
/// struct NotThreadSafe<T> {
1585-
/// value: UnsafeCell<T>,
1594+
/// let x: UnsafeCell<i32> = 42.into();
1595+
/// // Get multiple / concurrent / shared references to the same `x`.
1596+
/// let (p1, p2): (&UnsafeCell<i32>, &UnsafeCell<i32>) = (&x, &x);
1597+
///
1598+
/// unsafe {
1599+
/// // SAFETY: within this scope there are no other references to `x`'s contents,
1600+
/// // so ours is effectively unique.
1601+
/// let p1_exclusive: &mut i32 = &mut *p1.get(); // -- borrow --+
1602+
/// *p1_exclusive += 27; // |
1603+
/// } // <---------- cannot go beyond this point -------------------+
1604+
///
1605+
/// unsafe {
1606+
/// // SAFETY: within this scope nobody expects to have exclusive access to `x`'s contents,
1607+
/// // so we can have multiple shared accesses concurrently.
1608+
/// let p2_shared: &i32 = &*p2.get();
1609+
/// assert_eq!(*p2_shared, 42 + 27);
1610+
/// let p1_shared: &i32 = &*p1.get();
1611+
/// assert_eq!(*p1_shared, *p2_shared);
15861612
/// }
1613+
/// ```
15871614
///
1588-
/// unsafe impl<T> Sync for NotThreadSafe<T> {}
1615+
/// The following example showcases the fact that exclusive access to an `UnsafeCell<T>`
1616+
/// implies exclusive access to its `T`:
1617+
///
1618+
/// ```rust
1619+
/// #![feature(unsafe_cell_get_mut)]
1620+
/// #![forbid(unsafe_code)] // with exclusive accesses,
1621+
/// // `UnsafeCell` is a transparent no-op wrapper,
1622+
/// // so no need for `unsafe` here.
1623+
/// use std::cell::UnsafeCell;
1624+
///
1625+
/// let mut x: UnsafeCell<i32> = 42.into();
1626+
///
1627+
/// // Get a compile-time-checked unique reference to `x`.
1628+
/// let p_unique: &mut UnsafeCell<i32> = &mut x;
1629+
/// // With an exclusive reference, we can mutate the contents for free.
1630+
/// *p_unique.get_mut() = 0;
1631+
/// // Or, equivalently:
1632+
/// x = UnsafeCell::new(0);
1633+
///
1634+
/// // When we own the value, we can extract the contents for free.
1635+
/// let contents: i32 = x.into_inner();
1636+
/// assert_eq!(contents, 0);
15891637
/// ```
15901638
#[lang = "unsafe_cell"]
15911639
#[stable(feature = "rust1", since = "1.0.0")]
@@ -1663,6 +1711,29 @@ impl<T: ?Sized> UnsafeCell<T> {
16631711
self as *const UnsafeCell<T> as *const T as *mut T
16641712
}
16651713

1714+
/// Returns a mutable reference to the underlying data.
1715+
///
1716+
/// This call borrows the `UnsafeCell` mutably (at compile-time) which
1717+
/// guarantees that we possess the only reference.
1718+
///
1719+
/// # Examples
1720+
///
1721+
/// ```
1722+
/// #![feature(unsafe_cell_get_mut)]
1723+
/// use std::cell::UnsafeCell;
1724+
///
1725+
/// let mut c = UnsafeCell::new(5);
1726+
/// *c.get_mut() += 1;
1727+
///
1728+
/// assert_eq!(*c.get_mut(), 6);
1729+
/// ```
1730+
#[inline]
1731+
#[unstable(feature = "unsafe_cell_get_mut", issue = "76943")]
1732+
pub fn get_mut(&mut self) -> &mut T {
1733+
// SAFETY: (outer) `&mut` guarantees unique access.
1734+
unsafe { &mut *self.get() }
1735+
}
1736+
16661737
/// Gets a mutable pointer to the wrapped value.
16671738
/// The difference to [`get`] is that this function accepts a raw pointer,
16681739
/// which is useful to avoid the creation of temporary references.

library/core/src/sync/atomic.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -838,8 +838,7 @@ impl<T> AtomicPtr<T> {
838838
#[inline]
839839
#[stable(feature = "atomic_access", since = "1.15.0")]
840840
pub fn get_mut(&mut self) -> &mut *mut T {
841-
// SAFETY: the mutable reference guarantees unique ownership.
842-
unsafe { &mut *self.p.get() }
841+
self.p.get_mut()
843842
}
844843

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

library/std/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@
312312
#![feature(try_reserve)]
313313
#![feature(unboxed_closures)]
314314
#![feature(unsafe_block_in_unsafe_fn)]
315+
#![feature(unsafe_cell_get_mut)]
315316
#![feature(unsafe_cell_raw_get)]
316317
#![feature(untagged_unions)]
317318
#![feature(unwind_attributes)]

library/std/src/sync/mutex.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -406,9 +406,7 @@ impl<T: ?Sized> Mutex<T> {
406406
/// ```
407407
#[stable(feature = "mutex_get_mut", since = "1.6.0")]
408408
pub fn get_mut(&mut self) -> LockResult<&mut T> {
409-
// We know statically that there are no other references to `self`, so
410-
// there's no need to lock the inner mutex.
411-
let data = unsafe { &mut *self.data.get() };
409+
let data = self.data.get_mut();
412410
poison::map_result(self.poison.borrow(), |_| data)
413411
}
414412
}

library/std/src/sync/rwlock.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -404,9 +404,7 @@ impl<T: ?Sized> RwLock<T> {
404404
/// ```
405405
#[stable(feature = "rwlock_get_mut", since = "1.6.0")]
406406
pub fn get_mut(&mut self) -> LockResult<&mut T> {
407-
// We know statically that there are no other references to `self`, so
408-
// there's no need to lock the inner lock.
409-
let data = unsafe { &mut *self.data.get() };
407+
let data = self.data.get_mut();
410408
poison::map_result(self.poison.borrow(), |_| data)
411409
}
412410
}

0 commit comments

Comments
 (0)