Skip to content

Commit

Permalink
Add 'take', 'replace', 'replace_with' from std::cell::RefCell
Browse files Browse the repository at this point in the history
This changeset also clarifies the language around mutable borrows.  The
message accountable_refcell paniced with when trying to take a mutable
borrow was not entirely accurate ("RefCell is already immutably borrowed"),
as panics can also occur when trying to mutably borrow a RefCell that is
already mutably borrowed.

Technically, at the time of this writing, the `take` method is not in
stable Rust, but will be in the next version (1.49) rust-lang/rust#71395

A rust-toolchain file has been added codifying the expectation that this
crate targets stable Rust.
  • Loading branch information
twilco committed Dec 11, 2020
1 parent be1ad50 commit 7e18479
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 5 deletions.
1 change: 1 addition & 0 deletions rust-toolchain
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
stable
108 changes: 103 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use backtrace::Backtrace;
use std::cell::{
BorrowError, BorrowMutError, Ref as StdRef, RefCell as StdRefCell, RefMut as StdRefMut,
};
use std::env;
use std::{env, mem};
use std::fmt::{Debug, Display, Error, Formatter};
use std::ops::{Deref, DerefMut};

Expand Down Expand Up @@ -203,8 +203,8 @@ impl<T: ?Sized> RefCell<T> {
})
}

/// Borrow the value stored in this cell mutably. Panics if any outstanding immutable
/// borrows of the same cell exist.
/// Borrow the value stored in this cell mutably. Panics if there are any other outstanding
/// borrows of this cell (mutable borrows are unique, i.e. there can only be one).
pub fn borrow_mut(&self) -> RefMut<T> {
if let Ok(r) = self.inner.try_borrow_mut() {
let id = self.borrows.borrow_mut().record();
Expand All @@ -225,7 +225,7 @@ impl<T: ?Sized> RefCell<T> {
}
}
}
panic!("RefCell is already immutably borrowed.");
panic!("RefCell is already borrowed.");
}
}

Expand All @@ -251,6 +251,20 @@ impl<T: ?Sized> RefCell<T> {
}
}

impl <T> RefCell<T> {
/// Corresponds to https://doc.rust-lang.org/std/cell/struct.RefCell.html#method.replace.
pub fn replace(&self, t: T) -> T {
mem::replace(&mut *self.borrow_mut(), t)
}

/// Corresponds to https://doc.rust-lang.org/std/cell/struct.RefCell.html#method.replace_with.
pub fn replace_with<F: FnOnce(&mut T) -> T>(&self, f: F) -> T {
let mut_borrow = &mut *self.borrow_mut();
let replacement = f(mut_borrow);
mem::replace(mut_borrow, replacement)
}
}

/// Print a backtrace without any frames from the backtrace library.
fn print_filtered_backtrace(backtrace: &Backtrace) {
let mut idx = 1;
Expand Down Expand Up @@ -284,6 +298,13 @@ impl<T: Clone> Clone for RefCell<T> {
}
}

impl <T: Default> RefCell<T> {
/// Corresponds to https://doc.rust-lang.org/std/cell/struct.RefCell.html#method.take.
pub fn take(&self) -> T {
self.replace(Default::default())
}
}

impl<T: Default> Default for RefCell<T> {
fn default() -> RefCell<T> {
RefCell::new(Default::default())
Expand Down Expand Up @@ -319,7 +340,7 @@ mod tests {
use super::{Ref, RefCell};

#[test]
#[should_panic(expected = "RefCell is already immutably borrowed")]
#[should_panic(expected = "RefCell is already borrowed")]
fn cannot_borrow_mutably() {
let c = RefCell::new(5);
let _b = c.borrow();
Expand Down Expand Up @@ -358,4 +379,81 @@ mod tests {
};
let _b2 = c.borrow_mut();
}

#[test]
fn take_refcell_returns_correct_value() {
let c: RefCell<i32> = RefCell::new(5);
assert_eq!(5, c.take());
assert_eq!(i32::default(), *c.borrow());
}

#[test]
#[should_panic(expected = "RefCell is already borrowed")]
fn cannot_take_borrowed_refcell() {
let c = RefCell::new(5);
let _b = c.borrow();
c.take();
}

#[test]
#[should_panic(expected = "RefCell is already borrowed")]
fn cannot_take_mut_borrowed_refcell() {
let c = RefCell::new(5);
let _b = c.borrow_mut();
c.take();
}

#[test]
fn replace_refcell_properly_replaces_contents() {
let c = RefCell::new(5);
c.replace(12);
assert_eq!(12, *c.borrow());
}

#[test]
#[should_panic(expected = "RefCell is already borrowed")]
fn cannot_replace_borrowed_refcell() {
let c = RefCell::new(5);
let _b = c.borrow();
c.replace(12);
}

#[test]
#[should_panic(expected = "RefCell is already borrowed")]
fn cannot_replace_mut_borrowed_refcell() {
let c = RefCell::new(5);
let _b = c.borrow_mut();
c.replace(12);
}

#[test]
fn replace_with_refcell_properly_replaces_contents() {
let c = RefCell::new(5);
c.replace_with(|&mut old_value| old_value + 1);
assert_eq!(6, *c.borrow());
}

#[test]
#[should_panic(expected = "RefCell is already borrowed")]
fn cannot_replace_with_borrowed_refcell() {
let c = RefCell::new(5);
let _b = c.borrow();
c.replace_with(|&mut old_val| { old_val + 1 });
}

#[test]
#[should_panic(expected = "RefCell is already borrowed")]
fn cannot_replace_with_mut_borrowed_refcell() {
let c = RefCell::new(5);
let _b = c.borrow_mut();
c.replace_with(|&mut old_val| { old_val + 1 });
}

#[test]
#[should_panic(expected = "RefCell is already borrowed")]
fn test() {
let c = RefCell::new(5);
let _b = c.borrow_mut();
let _b2 = c.borrow_mut();
}
}

0 comments on commit 7e18479

Please sign in to comment.