From deed093a38243ced1f52927ebf7511c099a3bf36 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 1 Feb 2015 18:53:47 -0800 Subject: [PATCH] std: Deprecate RefCell::{try_borrow, try_borrow_mut} The existence of these two functions is at odds with our current [error conventions][conventions] which recommend that panicking and `Result`-like variants should not be provided together. [conventions]: https://github.com/rust-lang/rfcs/blob/master/text/0236-error-conventions.md#do-not-provide-both-result-and-fail-variants This commit adds a new `borrow_state` function returning a `BorrowState` enum to `RefCell` which serves as a replacemnt for the `try_borrow` and `try_borrow_mut` functions. --- src/libcore/cell.rs | 55 +++++++++++++++++++++++++++++++++-------- src/libcore/fmt/mod.rs | 10 +++++--- src/libcoretest/cell.rs | 4 +++ 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 65dccc8c244..15e745b0e71 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -266,6 +266,18 @@ pub struct RefCell { borrow: Cell, } +/// An enumeration of values returned from the `state` method on a `RefCell`. +#[derive(Copy, Clone, PartialEq)] +#[unstable(feature = "std_misc")] +pub enum BorrowState { + /// The cell is currently being read, there is at least one active `borrow`. + Reading, + /// The cell is currently being written to, there is an active `borrow_mut`. + Writing, + /// There are no outstanding borrows on this cell. + Unused, +} + // Values [1, MAX-1] represent the number of `Ref` active // (will not outgrow its range since `uint` is the size of the address space) type BorrowFlag = uint; @@ -310,6 +322,19 @@ impl RefCell { unsafe { self.value.into_inner() } } + /// Query the current state of this `RefCell` + /// + /// The returned value can be dispatched on to determine if a call to + /// `borrow` or `borrow_mut` would succeed. + #[unstable(feature = "std_misc")] + pub fn borrow_state(&self) -> BorrowState { + match self.borrow.get() { + WRITING => BorrowState::Writing, + UNUSED => BorrowState::Unused, + _ => BorrowState::Reading, + } + } + /// Attempts to immutably borrow the wrapped value. /// /// The borrow lasts until the returned `Ref` exits scope. Multiple @@ -317,6 +342,8 @@ impl RefCell { /// /// Returns `None` if the value is currently mutably borrowed. #[unstable(feature = "core", reason = "may be renamed or removed")] + #[deprecated(since = "1.0.0", + reason = "dispatch on `cell.borrow_state()` instead")] pub fn try_borrow<'a>(&'a self) -> Option> { match BorrowRef::new(&self.borrow) { Some(b) => Some(Ref { _value: unsafe { &*self.value.get() }, _borrow: b }), @@ -326,8 +353,8 @@ impl RefCell { /// Immutably borrows the wrapped value. /// - /// The borrow lasts until the returned `Ref` exits scope. Multiple immutable borrows can be - /// taken out at the same time. + /// The borrow lasts until the returned `Ref` exits scope. Multiple + /// immutable borrows can be taken out at the same time. /// /// # Panics /// @@ -361,9 +388,12 @@ impl RefCell { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn borrow<'a>(&'a self) -> Ref<'a, T> { - match self.try_borrow() { - Some(ptr) => ptr, - None => panic!("RefCell already mutably borrowed") + match BorrowRef::new(&self.borrow) { + Some(b) => Ref { + _value: unsafe { &*self.value.get() }, + _borrow: b, + }, + None => panic!("RefCell already mutably borrowed"), } } @@ -374,6 +404,8 @@ impl RefCell { /// /// Returns `None` if the value is currently borrowed. #[unstable(feature = "core", reason = "may be renamed or removed")] + #[deprecated(since = "1.0.0", + reason = "dispatch on `cell.borrow_state()` instead")] pub fn try_borrow_mut<'a>(&'a self) -> Option> { match BorrowRefMut::new(&self.borrow) { Some(b) => Some(RefMut { _value: unsafe { &mut *self.value.get() }, _borrow: b }), @@ -383,8 +415,8 @@ impl RefCell { /// Mutably borrows the wrapped value. /// - /// The borrow lasts until the returned `RefMut` exits scope. The value cannot be borrowed - /// while this borrow is active. + /// The borrow lasts until the returned `RefMut` exits scope. The value + /// cannot be borrowed while this borrow is active. /// /// # Panics /// @@ -417,9 +449,12 @@ impl RefCell { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn borrow_mut<'a>(&'a self) -> RefMut<'a, T> { - match self.try_borrow_mut() { - Some(ptr) => ptr, - None => panic!("RefCell already borrowed") + match BorrowRefMut::new(&self.borrow) { + Some(b) => RefMut { + _value: unsafe { &mut *self.value.get() }, + _borrow: b, + }, + None => panic!("RefCell already borrowed"), } } diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 2ff67ebd550..a82246ccb33 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -13,7 +13,7 @@ #![stable(feature = "rust1", since = "1.0.0")] use any; -use cell::{Cell, RefCell, Ref, RefMut}; +use cell::{Cell, RefCell, Ref, RefMut, BorrowState}; use char::CharExt; use iter::{Iterator, IteratorExt}; use marker::{Copy, Sized}; @@ -973,9 +973,11 @@ impl Debug for Cell { #[stable(feature = "rust1", since = "1.0.0")] impl Debug for RefCell { fn fmt(&self, f: &mut Formatter) -> Result { - match self.try_borrow() { - Some(val) => write!(f, "RefCell {{ value: {:?} }}", val), - None => write!(f, "RefCell {{ }}") + match self.borrow_state() { + BorrowState::Unused | BorrowState::Reading => { + write!(f, "RefCell {{ value: {:?} }}", self.borrow()) + } + BorrowState::Writing => write!(f, "RefCell {{ }}"), } } } diff --git a/src/libcoretest/cell.rs b/src/libcoretest/cell.rs index 5815dbc0acc..8939bd61fe4 100644 --- a/src/libcoretest/cell.rs +++ b/src/libcoretest/cell.rs @@ -60,6 +60,7 @@ fn no_mut_then_imm_borrow() { let x = RefCell::new(0); let _b1 = x.borrow_mut(); assert!(x.try_borrow().is_none()); + assert_eq!(x.borrow_state(), BorrowState::Writing); } #[test] @@ -67,13 +68,16 @@ fn no_imm_then_borrow_mut() { let x = RefCell::new(0); let _b1 = x.borrow(); assert!(x.try_borrow_mut().is_none()); + assert_eq!(x.borrow_state(), BorrowState::Reading); } #[test] fn no_double_borrow_mut() { let x = RefCell::new(0); + assert_eq!(x.borrow_state(), BorrowState::Unused); let _b1 = x.borrow_mut(); assert!(x.try_borrow_mut().is_none()); + assert_eq!(x.borrow_state(), BorrowState::Writing); } #[test]