Mercurial > hg

changeset 44203:2a24ead003f0

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
rust-cpython: add panicking version of borrow_mut() and use it The original borrow_mut() is renamed to try_borrow_mut(). Since leak_immutable() no longer incref the borrow count, the caller should know if the underlying value is borrowed or not. No Python world is involved. That's why we can simply use the panicking borrow_mut().
author Yuya Nishihara <yuya@tcha.org>
date Sat, 12 Oct 2019 23:34:05 +0900
parents a7f8160cc4e4
children bafdaf4858d8
files rust/hg-cpython/src/dirstate/dirs_multiset.rs rust/hg-cpython/src/dirstate/dirstate_map.rs rust/hg-cpython/src/ref_sharing.rs
diffstat 3 files changed, 54 insertions(+), 33 deletions(-) [+]
line wrap: on
line diff
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs	Tue Jan 28 22:27:30 2020 -0500
+++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs	Sat Oct 12 23:34:05 2019 +0900
@@ -72,7 +72,7 @@
     }
 
     def addpath(&self, path: PyObject) -> PyResult<PyObject> {
-        self.inner_shared(py).borrow_mut()?.add_path(
+        self.inner_shared(py).borrow_mut().add_path(
             HgPath::new(path.extract::<PyBytes>(py)?.data(py)),
         ).and(Ok(py.None())).or_else(|e| {
             match e {
@@ -90,7 +90,7 @@
     }
 
     def delpath(&self, path: PyObject) -> PyResult<PyObject> {
-        self.inner_shared(py).borrow_mut()?.delete_path(
+        self.inner_shared(py).borrow_mut().delete_path(
             HgPath::new(path.extract::<PyBytes>(py)?.data(py)),
         )
             .and(Ok(py.None()))
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs	Tue Jan 28 22:27:30 2020 -0500
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs	Sat Oct 12 23:34:05 2019 +0900
@@ -53,7 +53,7 @@
     }
 
     def clear(&self) -> PyResult<PyObject> {
-        self.inner_shared(py).borrow_mut()?.clear();
+        self.inner_shared(py).borrow_mut().clear();
         Ok(py.None())
     }
 
@@ -80,7 +80,7 @@
         size: PyObject,
         mtime: PyObject
     ) -> PyResult<PyObject> {
-        self.inner_shared(py).borrow_mut()?.add_file(
+        self.inner_shared(py).borrow_mut().add_file(
             HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
             oldstate.extract::<PyBytes>(py)?.data(py)[0]
                 .try_into()
@@ -108,7 +108,7 @@
         oldstate: PyObject,
         size: PyObject
     ) -> PyResult<PyObject> {
-        self.inner_shared(py).borrow_mut()?
+        self.inner_shared(py).borrow_mut()
             .remove_file(
                 HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
                 oldstate.extract::<PyBytes>(py)?.data(py)[0]
@@ -132,7 +132,7 @@
         f: PyObject,
         oldstate: PyObject
     ) -> PyResult<PyBool> {
-        self.inner_shared(py).borrow_mut()?
+        self.inner_shared(py).borrow_mut()
             .drop_file(
                 HgPath::new(f.extract::<PyBytes>(py)?.data(py)),
                 oldstate.extract::<PyBytes>(py)?.data(py)[0]
@@ -163,7 +163,7 @@
                 ))
             })
             .collect();
-        self.inner_shared(py).borrow_mut()?
+        self.inner_shared(py).borrow_mut()
             .clear_ambiguous_times(files?, now.extract(py)?);
         Ok(py.None())
     }
@@ -198,7 +198,7 @@
 
     def hastrackeddir(&self, d: PyObject) -> PyResult<PyBool> {
         let d = d.extract::<PyBytes>(py)?;
-        Ok(self.inner_shared(py).borrow_mut()?
+        Ok(self.inner_shared(py).borrow_mut()
             .has_tracked_dir(HgPath::new(d.data(py)))
             .map_err(|e| {
                 PyErr::new::<exc::ValueError, _>(py, e.to_string())
@@ -208,7 +208,7 @@
 
     def hasdir(&self, d: PyObject) -> PyResult<PyBool> {
         let d = d.extract::<PyBytes>(py)?;
-        Ok(self.inner_shared(py).borrow_mut()?
+        Ok(self.inner_shared(py).borrow_mut()
             .has_dir(HgPath::new(d.data(py)))
             .map_err(|e| {
                 PyErr::new::<exc::ValueError, _>(py, e.to_string())
@@ -217,7 +217,7 @@
     }
 
     def parents(&self, st: PyObject) -> PyResult<PyTuple> {
-        self.inner_shared(py).borrow_mut()?
+        self.inner_shared(py).borrow_mut()
             .parents(st.extract::<PyBytes>(py)?.data(py))
             .and_then(|d| {
                 Ok((PyBytes::new(py, &d.p1), PyBytes::new(py, &d.p2))
@@ -235,13 +235,13 @@
         let p1 = extract_node_id(py, &p1)?;
         let p2 = extract_node_id(py, &p2)?;
 
-        self.inner_shared(py).borrow_mut()?
+        self.inner_shared(py).borrow_mut()
             .set_parents(&DirstateParents { p1, p2 });
         Ok(py.None())
     }
 
     def read(&self, st: PyObject) -> PyResult<Option<PyObject>> {
-        match self.inner_shared(py).borrow_mut()?
+        match self.inner_shared(py).borrow_mut()
             .read(st.extract::<PyBytes>(py)?.data(py))
         {
             Ok(Some(parents)) => Ok(Some(
@@ -268,7 +268,7 @@
             p2: extract_node_id(py, &p2)?,
         };
 
-        match self.inner_shared(py).borrow_mut()?.pack(parents, now) {
+        match self.inner_shared(py).borrow_mut().pack(parents, now) {
             Ok(packed) => Ok(PyBytes::new(py, &packed)),
             Err(_) => Err(PyErr::new::<exc::OSError, _>(
                 py,
@@ -280,7 +280,7 @@
     def filefoldmapasdict(&self) -> PyResult<PyDict> {
         let dict = PyDict::new(py);
         for (key, value) in
-            self.inner_shared(py).borrow_mut()?.build_file_fold_map().iter()
+            self.inner_shared(py).borrow_mut().build_file_fold_map().iter()
         {
             dict.set_item(py, key.as_ref().to_vec(), value.as_ref().to_vec())?;
         }
@@ -336,7 +336,7 @@
 
     def getdirs(&self) -> PyResult<Dirs> {
         // TODO don't copy, share the reference
-        self.inner_shared(py).borrow_mut()?.set_dirs()
+        self.inner_shared(py).borrow_mut().set_dirs()
             .map_err(|e| {
                 PyErr::new::<exc::ValueError, _>(py, e.to_string())
             })?;
@@ -353,7 +353,7 @@
     }
     def getalldirs(&self) -> PyResult<Dirs> {
         // TODO don't copy, share the reference
-        self.inner_shared(py).borrow_mut()?.set_all_dirs()
+        self.inner_shared(py).borrow_mut().set_all_dirs()
             .map_err(|e| {
                 PyErr::new::<exc::ValueError, _>(py, e.to_string())
             })?;
@@ -431,7 +431,7 @@
     ) -> PyResult<PyObject> {
         let key = key.extract::<PyBytes>(py)?;
         let value = value.extract::<PyBytes>(py)?;
-        self.inner_shared(py).borrow_mut()?.copy_map.insert(
+        self.inner_shared(py).borrow_mut().copy_map.insert(
             HgPathBuf::from_bytes(key.data(py)),
             HgPathBuf::from_bytes(value.data(py)),
         );
@@ -445,7 +445,7 @@
         let key = key.extract::<PyBytes>(py)?;
         match self
             .inner_shared(py)
-            .borrow_mut()?
+            .borrow_mut()
             .copy_map
             .remove(HgPath::new(key.data(py)))
         {
--- a/rust/hg-cpython/src/ref_sharing.rs	Tue Jan 28 22:27:30 2020 -0500
+++ b/rust/hg-cpython/src/ref_sharing.rs	Sat Oct 12 23:34:05 2019 +0900
@@ -202,7 +202,19 @@
         self.data.borrow(self.py)
     }
 
-    pub fn borrow_mut(&self) -> PyResult<RefMut<'a, T>> {
+    /// Mutably borrows the wrapped value.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the value is currently borrowed through `PySharedRef`
+    /// or `PyLeaked`.
+    pub fn borrow_mut(&self) -> RefMut<'a, T> {
+        self.try_borrow_mut().expect("already borrowed")
+    }
+
+    /// Mutably borrows the wrapped value, returning an error if the value
+    /// is currently borrowed.
+    pub fn try_borrow_mut(&self) -> PyResult<RefMut<'a, T>> {
         self.data.try_borrow_mut(self.py)
     }
 
@@ -572,7 +584,7 @@
         let (gil, owner) = prepare_env();
         let py = gil.python();
         let leaked = owner.string_shared(py).leak_immutable();
-        owner.string_shared(py).borrow_mut().unwrap().clear();
+        owner.string_shared(py).borrow_mut().clear();
         assert!(leaked.try_borrow(py).is_err());
     }
 
@@ -582,7 +594,7 @@
         let py = gil.python();
         let leaked = owner.string_shared(py).leak_immutable();
         let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) };
-        owner.string_shared(py).borrow_mut().unwrap().clear();
+        owner.string_shared(py).borrow_mut().clear();
         assert!(leaked_iter.try_borrow_mut(py).is_err());
     }
 
@@ -592,40 +604,40 @@
         let (gil, owner) = prepare_env();
         let py = gil.python();
         let leaked = owner.string_shared(py).leak_immutable();
-        owner.string_shared(py).borrow_mut().unwrap().clear();
+        owner.string_shared(py).borrow_mut().clear();
         let _leaked_iter = unsafe { leaked.map(py, |s| s.chars()) };
     }
 
     #[test]
-    fn test_borrow_mut_while_leaked_ref() {
+    fn test_try_borrow_mut_while_leaked_ref() {
         let (gil, owner) = prepare_env();
         let py = gil.python();
-        assert!(owner.string_shared(py).borrow_mut().is_ok());
+        assert!(owner.string_shared(py).try_borrow_mut().is_ok());
         let leaked = owner.string_shared(py).leak_immutable();
         {
             let _leaked_ref = leaked.try_borrow(py).unwrap();
-            assert!(owner.string_shared(py).borrow_mut().is_err());
+            assert!(owner.string_shared(py).try_borrow_mut().is_err());
             {
                 let _leaked_ref2 = leaked.try_borrow(py).unwrap();
-                assert!(owner.string_shared(py).borrow_mut().is_err());
+                assert!(owner.string_shared(py).try_borrow_mut().is_err());
             }
-            assert!(owner.string_shared(py).borrow_mut().is_err());
+            assert!(owner.string_shared(py).try_borrow_mut().is_err());
         }
-        assert!(owner.string_shared(py).borrow_mut().is_ok());
+        assert!(owner.string_shared(py).try_borrow_mut().is_ok());
     }
 
     #[test]
-    fn test_borrow_mut_while_leaked_ref_mut() {
+    fn test_try_borrow_mut_while_leaked_ref_mut() {
         let (gil, owner) = prepare_env();
         let py = gil.python();
-        assert!(owner.string_shared(py).borrow_mut().is_ok());
+        assert!(owner.string_shared(py).try_borrow_mut().is_ok());
         let leaked = owner.string_shared(py).leak_immutable();
         let mut leaked_iter = unsafe { leaked.map(py, |s| s.chars()) };
         {
             let _leaked_ref = leaked_iter.try_borrow_mut(py).unwrap();
-            assert!(owner.string_shared(py).borrow_mut().is_err());
+            assert!(owner.string_shared(py).try_borrow_mut().is_err());
         }
-        assert!(owner.string_shared(py).borrow_mut().is_ok());
+        assert!(owner.string_shared(py).try_borrow_mut().is_ok());
     }
 
     #[test]
@@ -638,10 +650,19 @@
     }
 
     #[test]
+    fn test_try_borrow_mut_while_borrow() {
+        let (gil, owner) = prepare_env();
+        let py = gil.python();
+        let _ref = owner.string_shared(py).borrow();
+        assert!(owner.string_shared(py).try_borrow_mut().is_err());
+    }
+
+    #[test]
+    #[should_panic(expected = "already borrowed")]
     fn test_borrow_mut_while_borrow() {
         let (gil, owner) = prepare_env();
         let py = gil.python();
         let _ref = owner.string_shared(py).borrow();
-        assert!(owner.string_shared(py).borrow_mut().is_err());
+        owner.string_shared(py).borrow_mut();
     }
 }