rust-cpython: make PySharedRef::try_borrow_mut() return BorrowMutError
authorYuya Nishihara <yuya@tcha.org>
Sat, 19 Oct 2019 17:01:28 +0900
changeset 44206 9804badd5970
parent 44205 f015d679f08c
child 44207 e960c30d7e50
rust-cpython: make PySharedRef::try_borrow_mut() return BorrowMutError As I said, it shouldn't be an error of Python layer, but is something like a coding error. Returning BorrowMutError makes more sense. There's a weird hack to propagate the borrow-by-leaked state to RefCell to obtain BorrowMutError. If we don't like it, maybe we can add our own BorrowMutError.
rust/hg-cpython/src/exceptions.rs
rust/hg-cpython/src/ref_sharing.rs
--- a/rust/hg-cpython/src/exceptions.rs	Sat Oct 19 16:48:34 2019 +0900
+++ b/rust/hg-cpython/src/exceptions.rs	Sat Oct 19 17:01:28 2019 +0900
@@ -40,5 +40,3 @@
 }
 
 py_exception!(rustext, HgPathPyError, RuntimeError);
-
-py_exception!(shared_ref, AlreadyBorrowed, RuntimeError);
--- a/rust/hg-cpython/src/ref_sharing.rs	Sat Oct 19 16:48:34 2019 +0900
+++ b/rust/hg-cpython/src/ref_sharing.rs	Sat Oct 19 17:01:28 2019 +0900
@@ -22,10 +22,10 @@
 
 //! Macros for use in the `hg-cpython` bridge library.
 
-use crate::exceptions::AlreadyBorrowed;
 use cpython::{exc, PyClone, PyErr, PyObject, PyResult, Python};
-use std::cell::{Ref, RefCell, RefMut};
+use std::cell::{BorrowMutError, Ref, RefCell, RefMut};
 use std::ops::{Deref, DerefMut};
+use std::result;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
 /// Manages the shared state between Python and Rust
@@ -139,17 +139,14 @@
     fn try_borrow_mut<'a>(
         &'a self,
         py: Python<'a>,
-    ) -> PyResult<RefMut<'a, T>> {
+    ) -> result::Result<RefMut<'a, T>, BorrowMutError> {
         if self.py_shared_state.current_borrow_count(py) > 0 {
-            return Err(AlreadyBorrowed::new(
-                py,
-                "Cannot borrow mutably while immutably borrowed",
-            ));
+            // propagate borrow-by-leaked state to inner to get BorrowMutError
+            let _dummy = self.inner.borrow();
+            self.inner.try_borrow_mut()?;
+            unreachable!("BorrowMutError must be returned");
         }
-        let inner_ref = self
-            .inner
-            .try_borrow_mut()
-            .map_err(|e| AlreadyBorrowed::new(py, e.to_string()))?;
+        let inner_ref = self.inner.try_borrow_mut()?;
         self.py_shared_state.increment_generation(py);
         Ok(inner_ref)
     }
@@ -191,7 +188,9 @@
 
     /// Mutably borrows the wrapped value, returning an error if the value
     /// is currently borrowed.
-    pub fn try_borrow_mut(&self) -> PyResult<RefMut<'a, T>> {
+    pub fn try_borrow_mut(
+        &self,
+    ) -> result::Result<RefMut<'a, T>, BorrowMutError> {
         self.data.try_borrow_mut(self.py)
     }