diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs --- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs +++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs @@ -72,7 +72,7 @@ py_class!(pub class Dirs |py| { } def addpath(&self, path: PyObject) -> PyResult { - self.inner_shared(py).borrow_mut()?.add_path( + self.inner_shared(py).borrow_mut().add_path( HgPath::new(path.extract::(py)?.data(py)), ).and(Ok(py.None())).or_else(|e| { match e { @@ -90,7 +90,7 @@ py_class!(pub class Dirs |py| { } def delpath(&self, path: PyObject) -> PyResult { - self.inner_shared(py).borrow_mut()?.delete_path( + self.inner_shared(py).borrow_mut().delete_path( HgPath::new(path.extract::(py)?.data(py)), ) .and(Ok(py.None())) diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs --- a/rust/hg-cpython/src/dirstate/dirstate_map.rs +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs @@ -53,7 +53,7 @@ py_class!(pub class DirstateMap |py| { } def clear(&self) -> PyResult { - self.inner_shared(py).borrow_mut()?.clear(); + self.inner_shared(py).borrow_mut().clear(); Ok(py.None()) } @@ -80,7 +80,7 @@ py_class!(pub class DirstateMap |py| { size: PyObject, mtime: PyObject ) -> PyResult { - self.inner_shared(py).borrow_mut()?.add_file( + self.inner_shared(py).borrow_mut().add_file( HgPath::new(f.extract::(py)?.data(py)), oldstate.extract::(py)?.data(py)[0] .try_into() @@ -108,7 +108,7 @@ py_class!(pub class DirstateMap |py| { oldstate: PyObject, size: PyObject ) -> PyResult { - self.inner_shared(py).borrow_mut()? + self.inner_shared(py).borrow_mut() .remove_file( HgPath::new(f.extract::(py)?.data(py)), oldstate.extract::(py)?.data(py)[0] @@ -132,7 +132,7 @@ py_class!(pub class DirstateMap |py| { f: PyObject, oldstate: PyObject ) -> PyResult { - self.inner_shared(py).borrow_mut()? + self.inner_shared(py).borrow_mut() .drop_file( HgPath::new(f.extract::(py)?.data(py)), oldstate.extract::(py)?.data(py)[0] @@ -163,7 +163,7 @@ py_class!(pub class DirstateMap |py| { )) }) .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 @@ py_class!(pub class DirstateMap |py| { def hastrackeddir(&self, d: PyObject) -> PyResult { let d = d.extract::(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::(py, e.to_string()) @@ -208,7 +208,7 @@ py_class!(pub class DirstateMap |py| { def hasdir(&self, d: PyObject) -> PyResult { let d = d.extract::(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::(py, e.to_string()) @@ -217,7 +217,7 @@ py_class!(pub class DirstateMap |py| { } def parents(&self, st: PyObject) -> PyResult { - self.inner_shared(py).borrow_mut()? + self.inner_shared(py).borrow_mut() .parents(st.extract::(py)?.data(py)) .and_then(|d| { Ok((PyBytes::new(py, &d.p1), PyBytes::new(py, &d.p2)) @@ -235,13 +235,13 @@ py_class!(pub class DirstateMap |py| { 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> { - match self.inner_shared(py).borrow_mut()? + match self.inner_shared(py).borrow_mut() .read(st.extract::(py)?.data(py)) { Ok(Some(parents)) => Ok(Some( @@ -268,7 +268,7 @@ py_class!(pub class DirstateMap |py| { 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::( py, @@ -280,7 +280,7 @@ py_class!(pub class DirstateMap |py| { def filefoldmapasdict(&self) -> PyResult { 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 @@ py_class!(pub class DirstateMap |py| { def getdirs(&self) -> PyResult { // 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::(py, e.to_string()) })?; @@ -353,7 +353,7 @@ py_class!(pub class DirstateMap |py| { } def getalldirs(&self) -> PyResult { // 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::(py, e.to_string()) })?; @@ -431,7 +431,7 @@ py_class!(pub class DirstateMap |py| { ) -> PyResult { let key = key.extract::(py)?; let value = value.extract::(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 @@ py_class!(pub class DirstateMap |py| { let key = key.extract::(py)?; match self .inner_shared(py) - .borrow_mut()? + .borrow_mut() .copy_map .remove(HgPath::new(key.data(py))) { 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 @@ -202,7 +202,19 @@ impl<'a, T> PySharedRef<'a, T> { self.data.borrow(self.py) } - pub fn borrow_mut(&self) -> PyResult> { + /// 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> { self.data.try_borrow_mut(self.py) } @@ -572,7 +584,7 @@ mod test { 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 @@ mod test { 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 @@ mod test { 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 @@ mod test { } #[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(); } }