Skip to content

make RefCell unstably const #137843

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+154 −42
Open

make RefCell unstably const #137843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
120 changes: 78 additions & 42 deletions library/core/src/cell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ use crate::fmt::{self, Debug, Display};
use crate::marker::{PhantomData, PointerLike, Unsize};
use crate::mem;
use crate::ops::{CoerceUnsized, Deref, DerefMut, DerefPure, DispatchFromDyn};
use crate::panic::const_panic;
use crate::pin::PinCoerceUnsized;
use crate::ptr::{self, NonNull};

Expand Down Expand Up @@ -787,16 +788,24 @@ impl Display for BorrowMutError {
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[track_caller]
#[cold]
fn panic_already_borrowed(err: BorrowMutError) -> ! {
panic!("already borrowed: {:?}", err)
const fn panic_already_borrowed(err: BorrowMutError) -> ! {
const_panic!(
"already borrowed",
"already borrowed: {err:?}",
err: BorrowMutError = err,
)
}

// This ensures the panicking code is outlined from `borrow` for `RefCell`.
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[track_caller]
#[cold]
fn panic_already_mutably_borrowed(err: BorrowError) -> ! {
panic!("already mutably borrowed: {:?}", err)
const fn panic_already_mutably_borrowed(err: BorrowError) -> ! {
const_panic!(
"already mutably borrowed",
"already mutably borrowed: {err:?}",
err: BorrowError = err,
)
}

// Positive values represent the number of `Ref` active. Negative values
Expand All @@ -816,12 +825,12 @@ type BorrowFlag = isize;
const UNUSED: BorrowFlag = 0;

#[inline(always)]
fn is_writing(x: BorrowFlag) -> bool {
const fn is_writing(x: BorrowFlag) -> bool {
x < UNUSED
}

#[inline(always)]
fn is_reading(x: BorrowFlag) -> bool {
const fn is_reading(x: BorrowFlag) -> bool {
x > UNUSED
}

Expand Down Expand Up @@ -890,8 +899,9 @@ impl<T> RefCell<T> {
#[stable(feature = "refcell_replace", since = "1.24.0")]
#[track_caller]
#[rustc_confusables("swap")]
pub fn replace(&self, t: T) -> T {
mem::replace(&mut *self.borrow_mut(), t)
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn replace(&self, t: T) -> T {
mem::replace(&mut self.borrow_mut(), t)
}

/// Replaces the wrapped value with a new one computed from `f`, returning
Expand Down Expand Up @@ -941,7 +951,8 @@ impl<T> RefCell<T> {
/// ```
#[inline]
#[stable(feature = "refcell_swap", since = "1.24.0")]
pub fn swap(&self, other: &Self) {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn swap(&self, other: &Self) {
mem::swap(&mut *self.borrow_mut(), &mut *other.borrow_mut())
}
}
Expand Down Expand Up @@ -981,7 +992,8 @@ impl<T: ?Sized> RefCell<T> {
#[stable(feature = "rust1", since = "1.0.0")]
#[inline]
#[track_caller]
pub fn borrow(&self) -> Ref<'_, T> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn borrow(&self) -> Ref<'_, T> {
match self.try_borrow() {
Ok(b) => b,
Err(err) => panic_already_mutably_borrowed(err),
Expand Down Expand Up @@ -1016,14 +1028,15 @@ impl<T: ?Sized> RefCell<T> {
#[stable(feature = "try_borrow", since = "1.13.0")]
#[inline]
#[cfg_attr(feature = "debug_refcell", track_caller)]
pub fn try_borrow(&self) -> Result<Ref<'_, T>, BorrowError> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn try_borrow(&self) -> Result<Ref<'_, T>, BorrowError> {
match BorrowRef::new(&self.borrow) {
Some(b) => {
#[cfg(feature = "debug_refcell")]
{
// `borrowed_at` is always the *first* active borrow
if b.borrow.get() == 1 {
self.borrowed_at.set(Some(crate::panic::Location::caller()));
self.borrowed_at.replace(Some(crate::panic::Location::caller()));
}
}

Expand Down Expand Up @@ -1077,7 +1090,8 @@ impl<T: ?Sized> RefCell<T> {
#[stable(feature = "rust1", since = "1.0.0")]
#[inline]
#[track_caller]
pub fn borrow_mut(&self) -> RefMut<'_, T> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn borrow_mut(&self) -> RefMut<'_, T> {
match self.try_borrow_mut() {
Ok(b) => b,
Err(err) => panic_already_borrowed(err),
Expand Down Expand Up @@ -1109,12 +1123,13 @@ impl<T: ?Sized> RefCell<T> {
#[stable(feature = "try_borrow", since = "1.13.0")]
#[inline]
#[cfg_attr(feature = "debug_refcell", track_caller)]
pub fn try_borrow_mut(&self) -> Result<RefMut<'_, T>, BorrowMutError> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn try_borrow_mut(&self) -> Result<RefMut<'_, T>, BorrowMutError> {
match BorrowRefMut::new(&self.borrow) {
Some(b) => {
#[cfg(feature = "debug_refcell")]
{
self.borrowed_at.set(Some(crate::panic::Location::caller()));
self.borrowed_at.replace(Some(crate::panic::Location::caller()));
}

// SAFETY: `BorrowRefMut` guarantees unique access.
Expand Down Expand Up @@ -1145,7 +1160,8 @@ impl<T: ?Sized> RefCell<T> {
#[stable(feature = "cell_as_ptr", since = "1.12.0")]
#[rustc_as_ptr]
#[rustc_never_returns_null_ptr]
pub fn as_ptr(&self) -> *mut T {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn as_ptr(&self) -> *mut T {
self.value.get()
}

Expand Down Expand Up @@ -1182,7 +1198,8 @@ impl<T: ?Sized> RefCell<T> {
/// ```
#[inline]
#[stable(feature = "cell_get_mut", since = "1.11.0")]
pub fn get_mut(&mut self) -> &mut T {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn get_mut(&mut self) -> &mut T {
self.value.get_mut()
}

Expand All @@ -1208,7 +1225,8 @@ impl<T: ?Sized> RefCell<T> {
/// assert!(c.try_borrow().is_ok());
/// ```
#[unstable(feature = "cell_leak", issue = "69099")]
pub fn undo_leak(&mut self) -> &mut T {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn undo_leak(&mut self) -> &mut T {
*self.borrow.get_mut() = UNUSED;
self.get_mut()
}
Expand Down Expand Up @@ -1242,7 +1260,8 @@ impl<T: ?Sized> RefCell<T> {
/// ```
#[stable(feature = "borrow_state", since = "1.37.0")]
#[inline]
pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, BorrowError> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const unsafe fn try_borrow_unguarded(&self) -> Result<&T, BorrowError> {
if !is_writing(self.borrow.get()) {
// SAFETY: We check that nobody is actively writing now, but it is
// the caller's responsibility to ensure that nobody writes until
Expand Down Expand Up @@ -1406,7 +1425,8 @@ struct BorrowRef<'b> {

impl<'b> BorrowRef<'b> {
#[inline]
fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRef<'b>> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
const fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRef<'b>> {
let b = borrow.get().wrapping_add(1);
if !is_reading(b) {
// Incrementing borrow can result in a non-reading value (<= 0) in these cases:
Expand All @@ -1423,33 +1443,41 @@ impl<'b> BorrowRef<'b> {
// 1. It was = 0, i.e. it wasn't borrowed, and we are taking the first read borrow
// 2. It was > 0 and < isize::MAX, i.e. there were read borrows, and isize
// is large enough to represent having one more read borrow
borrow.set(b);
borrow.replace(b);
Some(BorrowRef { borrow })
}
}

/// FIXME(const-hack): `Clone` is not a `const_trait`, so work around that by making our own method
#[inline]
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
const fn clone(&self) -> Self {
// Since this Ref exists, we know the borrow flag
// is a reading borrow.
let borrow = self.borrow.get();
debug_assert!(is_reading(borrow));
// Prevent the borrow counter from overflowing into
// a writing borrow.
assert!(borrow != BorrowFlag::MAX);
self.borrow.replace(borrow + 1);
BorrowRef { borrow: self.borrow }
}
}

impl Drop for BorrowRef<'_> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
impl const Drop for BorrowRef<'_> {
#[inline]
fn drop(&mut self) {
let borrow = self.borrow.get();
debug_assert!(is_reading(borrow));
self.borrow.set(borrow - 1);
self.borrow.replace(borrow - 1);
}
}

impl Clone for BorrowRef<'_> {
#[inline]
fn clone(&self) -> Self {
// Since this Ref exists, we know the borrow flag
// is a reading borrow.
let borrow = self.borrow.get();
debug_assert!(is_reading(borrow));
// Prevent the borrow counter from overflowing into
// a writing borrow.
assert!(borrow != BorrowFlag::MAX);
self.borrow.set(borrow + 1);
BorrowRef { borrow: self.borrow }
self.clone()
}
}

Expand All @@ -1469,7 +1497,8 @@ pub struct Ref<'b, T: ?Sized + 'b> {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> Deref for Ref<'_, T> {
#[rustc_const_unstable(feature = "const_deref", issue = "88955")]
impl<T: ?Sized> const Deref for Ref<'_, T> {
type Target = T;

#[inline]
Expand All @@ -1494,7 +1523,8 @@ impl<'b, T: ?Sized> Ref<'b, T> {
#[stable(feature = "cell_extras", since = "1.15.0")]
#[must_use]
#[inline]
pub fn clone(orig: &Ref<'b, T>) -> Ref<'b, T> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn clone(orig: &Ref<'b, T>) -> Ref<'b, T> {
Ref { value: orig.value, borrow: orig.borrow.clone() }
}

Expand Down Expand Up @@ -1616,7 +1646,8 @@ impl<'b, T: ?Sized> Ref<'b, T> {
/// assert!(cell.try_borrow_mut().is_err());
/// ```
#[unstable(feature = "cell_leak", issue = "69099")]
pub fn leak(orig: Ref<'b, T>) -> &'b T {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn leak(orig: Ref<'b, T>) -> &'b T {
// By forgetting this Ref we ensure that the borrow counter in the RefCell can't go back to
// UNUSED within the lifetime `'b`. Resetting the reference tracking state would require a
// unique reference to the borrowed RefCell. No further mutable references can be created
Expand Down Expand Up @@ -1782,7 +1813,8 @@ impl<'b, T: ?Sized> RefMut<'b, T> {
/// assert!(cell.try_borrow_mut().is_err());
/// ```
#[unstable(feature = "cell_leak", issue = "69099")]
pub fn leak(mut orig: RefMut<'b, T>) -> &'b mut T {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
pub const fn leak(mut orig: RefMut<'b, T>) -> &'b mut T {
// By forgetting this BorrowRefMut we ensure that the borrow counter in the RefCell can't
// go back to UNUSED within the lifetime `'b`. Resetting the reference tracking state would
// require a unique reference to the borrowed RefCell. No further references can be created
Expand All @@ -1798,25 +1830,27 @@ struct BorrowRefMut<'b> {
borrow: &'b Cell<BorrowFlag>,
}

impl Drop for BorrowRefMut<'_> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
impl const Drop for BorrowRefMut<'_> {
#[inline]
fn drop(&mut self) {
let borrow = self.borrow.get();
debug_assert!(is_writing(borrow));
self.borrow.set(borrow + 1);
self.borrow.replace(borrow + 1);
}
}

impl<'b> BorrowRefMut<'b> {
#[inline]
fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRefMut<'b>> {
#[rustc_const_unstable(feature = "const_ref_cell", issue = "137844")]
const fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRefMut<'b>> {
// NOTE: Unlike BorrowRefMut::clone, new is called to create the initial
// mutable reference, and so there must currently be no existing
// references. Thus, while clone increments the mutable refcount, here
// we explicitly only allow going from UNUSED to UNUSED - 1.
match borrow.get() {
UNUSED => {
borrow.set(UNUSED - 1);
borrow.replace(UNUSED - 1);
Some(BorrowRefMut { borrow })
}
_ => None,
Expand Down Expand Up @@ -1855,7 +1889,8 @@ pub struct RefMut<'b, T: ?Sized + 'b> {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> Deref for RefMut<'_, T> {
#[rustc_const_unstable(feature = "const_deref", issue = "88955")]
impl<T: ?Sized> const Deref for RefMut<'_, T> {
type Target = T;

#[inline]
Expand All @@ -1866,7 +1901,8 @@ impl<T: ?Sized> Deref for RefMut<'_, T> {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> DerefMut for RefMut<'_, T> {
#[rustc_const_unstable(feature = "const_deref", issue = "88955")]
impl<T: ?Sized> const DerefMut for RefMut<'_, T> {
#[inline]
fn deref_mut(&mut self) -> &mut T {
// SAFETY: the value is accessible as long as we hold our borrow.
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@
#![feature(cfg_target_has_atomic)]
#![feature(cfg_target_has_atomic_equal_alignment)]
#![feature(cfg_ub_checks)]
#![feature(const_destruct)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@compiler-errors @lcnr @fee1-dead is this feature okay to use in libcore?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not until the syntax is finalized by T-lang. I've marked this PR as blocked

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which syntax? There is no ~const in this PR.

Copy link
Member

@RalfJung RalfJung Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this feature gate does is

  • allow impl const Drop -- but we already have a few impl const in the standard library so that syntax can't be the concern (I'm not even sure if the feature gate is truly required for this)
  • adjust the const_check logic to allow drop in const if the type in question is const-destructible

Copy link
Contributor

@oli-obk oli-obk Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

impl const Trait for Type syntax is also not clear anymore that it will get accepted. So we're putting everything on hold and not adding more things we may have to revert

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I go back to the private const_drop and as_mut helpers then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll just hold off on merging. I'm optimistic we'll get a decision soon

#![feature(const_precise_live_drops)]
#![feature(const_trait_impl)]
#![feature(decl_macro)]
Expand Down
Loading
Loading