Patchwork [6,of,7] rust-cpython: make PySharedRef::try_borrow_mut() return BorrowMutError

login
register
mail settings
Submitter Yuya Nishihara
Date Oct. 22, 2019, 8:11 a.m.
Message ID <29399437267567957a97.1571731917@mimosa>
Download mbox | patch
Permalink /patch/42525/
State New
Headers show

Comments

Yuya Nishihara - Oct. 22, 2019, 8:11 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1571472088 -32400
#      Sat Oct 19 17:01:28 2019 +0900
# Node ID 29399437267567957a97dc9d924704ca748e3cec
# Parent  253a5f9f8ed37f696073bcfc9be87bf5fddbfd57
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.

Patch

diff --git a/rust/hg-cpython/src/exceptions.rs b/rust/hg-cpython/src/exceptions.rs
--- a/rust/hg-cpython/src/exceptions.rs
+++ b/rust/hg-cpython/src/exceptions.rs
@@ -68,5 +68,3 @@  impl PatternFileError {
         }
     }
 }
-
-py_exception!(shared_ref, AlreadyBorrowed, RuntimeError);
diff --git a/rust/hg-cpython/src/ref_sharing.rs b/rust/hg-cpython/src/ref_sharing.rs
--- a/rust/hg-cpython/src/ref_sharing.rs
+++ b/rust/hg-cpython/src/ref_sharing.rs
@@ -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 @@  impl<T> PySharedRefCell<T> {
     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 @@  impl<'a, T> PySharedRef<'a, T> {
 
     /// 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)
     }