Patchwork [02,of,10] rust-cpython: move borrow_mut() to PySharedRefCell

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 22, 2019, 6:41 a.m.
Message ID <34a355c69f643f15ff46.1569134499@mimosa>
Download mbox | patch
Permalink /patch/41721/
State Accepted
Headers show

Comments

Yuya Nishihara - Sept. 22, 2019, 6:41 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1568470639 -32400
#      Sat Sep 14 23:17:19 2019 +0900
# Node ID 34a355c69f643f15ff46f4524c75137c08a8de3b
# Parent  e7f643679476b013e86b9a2b75f704d64eec4293
rust-cpython: move borrow_mut() to PySharedRefCell

PySharedRefCell() will host almost all py_shared public functions. This change
is the first step.

borrow_mut() can be safely implemented since PySharedRefCell knows its inner
object is managed by its own py_shared_state.
Yuya Nishihara - Oct. 5, 2019, 3:05 p.m.
On Sun, 22 Sep 2019 15:41:39 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1568470639 -32400
> #      Sat Sep 14 23:17:19 2019 +0900
> # Node ID 34a355c69f643f15ff46f4524c75137c08a8de3b
> # Parent  e7f643679476b013e86b9a2b75f704d64eec4293
> rust-cpython: move borrow_mut() to PySharedRefCell

> 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
> @@ -125,9 +125,15 @@ impl<T> PySharedRefCell<T> {
>          self.inner.as_ptr()
>      }
>  
> -    pub unsafe fn borrow_mut(&self) -> RefMut<T> {
> -        // must be borrowed by self.py_shared_state(py).borrow_mut().
> -        self.inner.borrow_mut()
> +    // TODO: maybe this should be named as try_borrow_mut(), and use
> +    // inner.try_borrow_mut(). The current implementation panics if
> +    // self.inner has been borrowed, but returns error if py_shared_state
> +    // refuses to borrow.
> +    pub fn borrow_mut<'a>(
> +        &'a self,
> +        py: Python<'a>,
> +    ) -> PyResult<PyRefMut<'a, T>> {

Actually this will be changed to borrow_mut() -> RefMut<'a, T> since
borrow_mut() will just mark leaked references poisoned. Mutation while
leaked will be just allowed.

Patch

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
@@ -125,9 +125,15 @@  impl<T> PySharedRefCell<T> {
         self.inner.as_ptr()
     }
 
-    pub unsafe fn borrow_mut(&self) -> RefMut<T> {
-        // must be borrowed by self.py_shared_state(py).borrow_mut().
-        self.inner.borrow_mut()
+    // TODO: maybe this should be named as try_borrow_mut(), and use
+    // inner.try_borrow_mut(). The current implementation panics if
+    // self.inner has been borrowed, but returns error if py_shared_state
+    // refuses to borrow.
+    pub fn borrow_mut<'a>(
+        &'a self,
+        py: Python<'a>,
+    ) -> PyResult<PyRefMut<'a, T>> {
+        self.py_shared_state.borrow_mut(py, self.inner.borrow_mut())
     }
 }
 
@@ -216,6 +222,7 @@  macro_rules! py_shared_ref {
         $leaked: ident,
     ) => {
         impl $name {
+            // TODO: remove this function in favor of inner(py).borrow_mut()
             fn borrow_mut<'a>(
                 &'a self,
                 py: Python<'a>,
@@ -224,8 +231,7 @@  macro_rules! py_shared_ref {
                 // assert $data_member type
                 use crate::ref_sharing::PySharedRefCell;
                 let data: &PySharedRefCell<_> = self.$data_member(py);
-                data.py_shared_state
-                    .borrow_mut(py, unsafe { data.borrow_mut() })
+                data.borrow_mut(py)
             }
 
             /// Returns a leaked reference and its management object.