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.

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
Open
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
120 changes: 78 additions & 42 deletions library/core/src/cell.rs
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do that in a separate PR first.

Copy link
Author

Choose a reason for hiding this comment

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

Apologies, I don't quite understand. Are you saying to move this const version of clone into its own PR? It depends on a good number of other methods being const, so I can do that but I'm not sure it's the cleanest approach.

Copy link
Author

Choose a reason for hiding this comment

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

Or are you saying to add const_trait to Clone in a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea, Let's make Clone const before landing this PR, then this PR doesn't need a const hack

#[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
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)]
#![feature(const_precise_live_drops)]
#![feature(const_trait_impl)]
#![feature(decl_macro)]
Expand Down
Loading
Loading