Skip to content

Commit aec00f9

Browse files
committed
Auto merge of #51466 - joshlf:ref-split, r=dtolnay
Add Ref/RefMut map_split method As proposed [here](https://internals.rust-lang.org/t/make-refcell-support-slice-splitting/7707). TLDR: Add a `map_split` method that allows multiple `RefMut`s to exist simultaneously so long as they refer to non-overlapping regions of the original `RefCell`. This is useful for things like the slice `split_at_mut` method.
2 parents 0f8f490 + 2a999b4 commit aec00f9

File tree

3 files changed

+185
-19
lines changed

3 files changed

+185
-19
lines changed

src/libcore/cell.rs

+126-19
Original file line numberDiff line numberDiff line change
@@ -570,11 +570,20 @@ impl Display for BorrowMutError {
570570
}
571571
}
572572

573-
// Values [1, MAX-1] represent the number of `Ref` active
574-
// (will not outgrow its range since `usize` is the size of the address space)
573+
// Values [1, MIN_WRITING-1] represent the number of `Ref` active. Values in
574+
// [MIN_WRITING, MAX-1] represent the number of `RefMut` active. Multiple
575+
// `RefMut`s can only be active at a time if they refer to distinct,
576+
// nonoverlapping components of a `RefCell` (e.g., different ranges of a slice).
577+
//
578+
// `Ref` and `RefMut` are both two words in size, and so there will likely never
579+
// be enough `Ref`s or `RefMut`s in existence to overflow half of the `usize`
580+
// range. Thus, a `BorrowFlag` will probably never overflow. However, this is
581+
// not a guarantee, as a pathological program could repeatedly create and then
582+
// mem::forget `Ref`s or `RefMut`s. Thus, all code must explicitly check for
583+
// overflow in order to avoid unsafety.
575584
type BorrowFlag = usize;
576585
const UNUSED: BorrowFlag = 0;
577-
const WRITING: BorrowFlag = !0;
586+
const MIN_WRITING: BorrowFlag = (!0)/2 + 1; // 0b1000...
578587

579588
impl<T> RefCell<T> {
580589
/// Creates a new `RefCell` containing `value`.
@@ -775,8 +784,9 @@ impl<T: ?Sized> RefCell<T> {
775784

776785
/// Mutably borrows the wrapped value.
777786
///
778-
/// The borrow lasts until the returned `RefMut` exits scope. The value
779-
/// cannot be borrowed while this borrow is active.
787+
/// The borrow lasts until the returned `RefMut` or all `RefMut`s derived
788+
/// from it exit scope. The value cannot be borrowed while this borrow is
789+
/// active.
780790
///
781791
/// # Panics
782792
///
@@ -818,8 +828,9 @@ impl<T: ?Sized> RefCell<T> {
818828

819829
/// Mutably borrows the wrapped value, returning an error if the value is currently borrowed.
820830
///
821-
/// The borrow lasts until the returned `RefMut` exits scope. The value cannot be borrowed
822-
/// while this borrow is active.
831+
/// The borrow lasts until the returned `RefMut` or all `RefMut`s derived
832+
/// from it exit scope. The value cannot be borrowed while this borrow is
833+
/// active.
823834
///
824835
/// This is the non-panicking variant of [`borrow_mut`](#method.borrow_mut).
825836
///
@@ -1010,12 +1021,15 @@ struct BorrowRef<'b> {
10101021
impl<'b> BorrowRef<'b> {
10111022
#[inline]
10121023
fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRef<'b>> {
1013-
match borrow.get() {
1014-
WRITING => None,
1015-
b => {
1016-
borrow.set(b + 1);
1017-
Some(BorrowRef { borrow: borrow })
1018-
},
1024+
let b = borrow.get();
1025+
if b >= MIN_WRITING {
1026+
None
1027+
} else {
1028+
// Prevent the borrow counter from overflowing into
1029+
// a writing borrow.
1030+
assert!(b < MIN_WRITING - 1);
1031+
borrow.set(b + 1);
1032+
Some(BorrowRef { borrow })
10191033
}
10201034
}
10211035
}
@@ -1024,7 +1038,7 @@ impl<'b> Drop for BorrowRef<'b> {
10241038
#[inline]
10251039
fn drop(&mut self) {
10261040
let borrow = self.borrow.get();
1027-
debug_assert!(borrow != WRITING && borrow != UNUSED);
1041+
debug_assert!(borrow < MIN_WRITING && borrow != UNUSED);
10281042
self.borrow.set(borrow - 1);
10291043
}
10301044
}
@@ -1036,8 +1050,9 @@ impl<'b> Clone for BorrowRef<'b> {
10361050
// is not set to WRITING.
10371051
let borrow = self.borrow.get();
10381052
debug_assert!(borrow != UNUSED);
1039-
// Prevent the borrow counter from overflowing.
1040-
assert!(borrow != WRITING);
1053+
// Prevent the borrow counter from overflowing into
1054+
// a writing borrow.
1055+
assert!(borrow < MIN_WRITING - 1);
10411056
self.borrow.set(borrow + 1);
10421057
BorrowRef { borrow: self.borrow }
10431058
}
@@ -1109,6 +1124,37 @@ impl<'b, T: ?Sized> Ref<'b, T> {
11091124
borrow: orig.borrow,
11101125
}
11111126
}
1127+
1128+
/// Split a `Ref` into multiple `Ref`s for different components of the
1129+
/// borrowed data.
1130+
///
1131+
/// The `RefCell` is already immutably borrowed, so this cannot fail.
1132+
///
1133+
/// This is an associated function that needs to be used as
1134+
/// `Ref::map_split(...)`. A method would interfere with methods of the same
1135+
/// name on the contents of a `RefCell` used through `Deref`.
1136+
///
1137+
/// # Examples
1138+
///
1139+
/// ```
1140+
/// #![feature(refcell_map_split)]
1141+
/// use std::cell::{Ref, RefCell};
1142+
///
1143+
/// let cell = RefCell::new([1, 2, 3, 4]);
1144+
/// let borrow = cell.borrow();
1145+
/// let (begin, end) = Ref::map_split(borrow, |slice| slice.split_at(2));
1146+
/// assert_eq!(*begin, [1, 2]);
1147+
/// assert_eq!(*end, [3, 4]);
1148+
/// ```
1149+
#[unstable(feature = "refcell_map_split", issue = "51476")]
1150+
#[inline]
1151+
pub fn map_split<U: ?Sized, V: ?Sized, F>(orig: Ref<'b, T>, f: F) -> (Ref<'b, U>, Ref<'b, V>)
1152+
where F: FnOnce(&T) -> (&U, &V)
1153+
{
1154+
let (a, b) = f(orig.value);
1155+
let borrow = orig.borrow.clone();
1156+
(Ref { value: a, borrow }, Ref { value: b, borrow: orig.borrow })
1157+
}
11121158
}
11131159

11141160
#[unstable(feature = "coerce_unsized", issue = "27732")]
@@ -1157,6 +1203,44 @@ impl<'b, T: ?Sized> RefMut<'b, T> {
11571203
borrow: borrow,
11581204
}
11591205
}
1206+
1207+
/// Split a `RefMut` into multiple `RefMut`s for different components of the
1208+
/// borrowed data.
1209+
///
1210+
/// The underlying `RefCell` will remain mutably borrowed until both
1211+
/// returned `RefMut`s go out of scope.
1212+
///
1213+
/// The `RefCell` is already mutably borrowed, so this cannot fail.
1214+
///
1215+
/// This is an associated function that needs to be used as
1216+
/// `RefMut::map_split(...)`. A method would interfere with methods of the
1217+
/// same name on the contents of a `RefCell` used through `Deref`.
1218+
///
1219+
/// # Examples
1220+
///
1221+
/// ```
1222+
/// #![feature(refcell_map_split)]
1223+
/// use std::cell::{RefCell, RefMut};
1224+
///
1225+
/// let cell = RefCell::new([1, 2, 3, 4]);
1226+
/// let borrow = cell.borrow_mut();
1227+
/// let (mut begin, mut end) = RefMut::map_split(borrow, |slice| slice.split_at_mut(2));
1228+
/// assert_eq!(*begin, [1, 2]);
1229+
/// assert_eq!(*end, [3, 4]);
1230+
/// begin.copy_from_slice(&[4, 3]);
1231+
/// end.copy_from_slice(&[2, 1]);
1232+
/// ```
1233+
#[unstable(feature = "refcell_map_split", issue = "51476")]
1234+
#[inline]
1235+
pub fn map_split<U: ?Sized, V: ?Sized, F>(
1236+
orig: RefMut<'b, T>, f: F
1237+
) -> (RefMut<'b, U>, RefMut<'b, V>)
1238+
where F: FnOnce(&mut T) -> (&mut U, &mut V)
1239+
{
1240+
let (a, b) = f(orig.value);
1241+
let borrow = orig.borrow.clone();
1242+
(RefMut { value: a, borrow }, RefMut { value: b, borrow: orig.borrow })
1243+
}
11601244
}
11611245

11621246
struct BorrowRefMut<'b> {
@@ -1167,22 +1251,45 @@ impl<'b> Drop for BorrowRefMut<'b> {
11671251
#[inline]
11681252
fn drop(&mut self) {
11691253
let borrow = self.borrow.get();
1170-
debug_assert!(borrow == WRITING);
1171-
self.borrow.set(UNUSED);
1254+
debug_assert!(borrow >= MIN_WRITING);
1255+
self.borrow.set(if borrow == MIN_WRITING {
1256+
UNUSED
1257+
} else {
1258+
borrow - 1
1259+
});
11721260
}
11731261
}
11741262

11751263
impl<'b> BorrowRefMut<'b> {
11761264
#[inline]
11771265
fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRefMut<'b>> {
1266+
// NOTE: Unlike BorrowRefMut::clone, new is called to create the initial
1267+
// mutable reference, and so there must currently be no existing
1268+
// references. Thus, while clone increments the mutable refcount, here
1269+
// we simply go directly from UNUSED to MIN_WRITING.
11781270
match borrow.get() {
11791271
UNUSED => {
1180-
borrow.set(WRITING);
1272+
borrow.set(MIN_WRITING);
11811273
Some(BorrowRefMut { borrow: borrow })
11821274
},
11831275
_ => None,
11841276
}
11851277
}
1278+
1279+
// Clone a `BorrowRefMut`.
1280+
//
1281+
// This is only valid if each `BorrowRefMut` is used to track a mutable
1282+
// reference to a distinct, nonoverlapping range of the original object.
1283+
// This isn't in a Clone impl so that code doesn't call this implicitly.
1284+
#[inline]
1285+
fn clone(&self) -> BorrowRefMut<'b> {
1286+
let borrow = self.borrow.get();
1287+
debug_assert!(borrow >= MIN_WRITING);
1288+
// Prevent the borrow counter from overflowing.
1289+
assert!(borrow != !0);
1290+
self.borrow.set(borrow + 1);
1291+
BorrowRefMut { borrow: self.borrow }
1292+
}
11861293
}
11871294

11881295
/// A wrapper type for a mutably borrowed value from a `RefCell<T>`.

src/libcore/tests/cell.rs

+58
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,64 @@ fn ref_map_does_not_update_flag() {
165165
assert!(x.try_borrow_mut().is_ok());
166166
}
167167

168+
#[test]
169+
fn ref_map_split_updates_flag() {
170+
let x = RefCell::new([1, 2]);
171+
{
172+
let b1 = x.borrow();
173+
assert!(x.try_borrow().is_ok());
174+
assert!(x.try_borrow_mut().is_err());
175+
{
176+
let (_b2, _b3) = Ref::map_split(b1, |slc| slc.split_at(1));
177+
assert!(x.try_borrow().is_ok());
178+
assert!(x.try_borrow_mut().is_err());
179+
}
180+
assert!(x.try_borrow().is_ok());
181+
assert!(x.try_borrow_mut().is_ok());
182+
}
183+
assert!(x.try_borrow().is_ok());
184+
assert!(x.try_borrow_mut().is_ok());
185+
186+
{
187+
let b1 = x.borrow_mut();
188+
assert!(x.try_borrow().is_err());
189+
assert!(x.try_borrow_mut().is_err());
190+
{
191+
let (_b2, _b3) = RefMut::map_split(b1, |slc| slc.split_at_mut(1));
192+
assert!(x.try_borrow().is_err());
193+
assert!(x.try_borrow_mut().is_err());
194+
drop(_b2);
195+
assert!(x.try_borrow().is_err());
196+
assert!(x.try_borrow_mut().is_err());
197+
}
198+
assert!(x.try_borrow().is_ok());
199+
assert!(x.try_borrow_mut().is_ok());
200+
}
201+
assert!(x.try_borrow().is_ok());
202+
assert!(x.try_borrow_mut().is_ok());
203+
}
204+
205+
#[test]
206+
fn ref_map_split() {
207+
let x = RefCell::new([1, 2]);
208+
let (b1, b2) = Ref::map_split(x.borrow(), |slc| slc.split_at(1));
209+
assert_eq!(*b1, [1]);
210+
assert_eq!(*b2, [2]);
211+
}
212+
213+
#[test]
214+
fn ref_mut_map_split() {
215+
let x = RefCell::new([1, 2]);
216+
{
217+
let (mut b1, mut b2) = RefMut::map_split(x.borrow_mut(), |slc| slc.split_at_mut(1));
218+
assert_eq!(*b1, [1]);
219+
assert_eq!(*b2, [2]);
220+
b1[0] = 2;
221+
b2[0] = 1;
222+
}
223+
assert_eq!(*x.borrow(), [2, 1]);
224+
}
225+
168226
#[test]
169227
fn ref_map_accessor() {
170228
struct X(RefCell<(u32, char)>);

src/libcore/tests/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#![feature(pattern)]
2828
#![feature(range_is_empty)]
2929
#![feature(raw)]
30+
#![feature(refcell_map_split)]
3031
#![feature(refcell_replace_swap)]
3132
#![feature(slice_patterns)]
3233
#![feature(slice_rotate)]

0 commit comments

Comments
 (0)