From 6f810e43581ba332d9f936d13aa584e05dd45a6b Mon Sep 17 00:00:00 2001 From: Tyler Wilcock Date: Fri, 11 Dec 2020 10:34:36 -0600 Subject: [PATCH] Add 'take', 'replace', 'replace_with' from std::cell::RefCell 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) https://github.com/rust-lang/rust/issues/71395 A rust-toolchain file has been added codifying the expectation that this crate targets stable Rust. --- rust-toolchain | 1 + src/lib.rs | 108 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 rust-toolchain diff --git a/rust-toolchain b/rust-toolchain new file mode 100644 index 0000000..870bbe4 --- /dev/null +++ b/rust-toolchain @@ -0,0 +1 @@ +stable \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index db66e66..118eadc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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}; @@ -203,8 +203,8 @@ impl RefCell { }) } - /// 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 { if let Ok(r) = self.inner.try_borrow_mut() { let id = self.borrows.borrow_mut().record(); @@ -225,7 +225,7 @@ impl RefCell { } } } - panic!("RefCell is already immutably borrowed."); + panic!("RefCell is already borrowed."); } } @@ -251,6 +251,20 @@ impl RefCell { } } +impl RefCell { + /// 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 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; @@ -284,6 +298,13 @@ impl Clone for RefCell { } } +impl RefCell { + /// Corresponds to https://doc.rust-lang.org/std/cell/struct.RefCell.html#method.take. + pub fn take(&self) -> T { + self.replace(Default::default()) + } +} + impl Default for RefCell { fn default() -> RefCell { RefCell::new(Default::default()) @@ -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(); @@ -358,4 +379,81 @@ mod tests { }; let _b2 = c.borrow_mut(); } + + #[test] + fn take_refcell_returns_correct_value() { + let c = 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(); + } }