[PATCH 1 of 9] rust-cpython: rename PyLeakedRef to PyLeaked
Raphaël Gomès
raphael.gomes at octobus.net
Sat Oct 19 16:28:55 EDT 2019
Thanks a lot for this series Yuya. This reference-sharing business gets
a lot better every time.
We should maybe think about bringing Georges and Mark along for the ride
for the (near) future upstreaming, don't you think?
On 10/19/19 12:07 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1570875051 -32400
> # Sat Oct 12 19:10:51 2019 +0900
> # Node ID da6ae3bc571609faeb2bd6696efc7869b0420f6a
> # Parent cfe3e9159c6c4eb5b7775a7b813f7f22f55a1f88
> rust-cpython: rename PyLeakedRef to PyLeaked
>
> This series will make PyLeaked* behave more like a Python iterator, which
> means mutation of the owner object will be allowed and the leaked reference
> (i.e. the iterator) will be invalidated instead.
>
> I'll add PyLeakedRef/PyLeakedRefMut structs which will represent a "borrowed"
> state, and prevent the underlying value from being mutably borrowed while the
> leaked reference is in use:
>
> let shared = self.inner_shared(py);
> let leaked = shared.leak_immutable();
> {
> let leaked_ref: PyLeakedRef<_> = leaked.borrow(py);
> shared.borrow_mut(); // panics since the underlying value is borrowed
> }
> shared.borrow_mut(); // allowed
>
> The relation between PyLeaked* structs is quite similar to RefCell/Ref/RefMut,
> but the implementation can't be reused because the borrowing state will have
> to be shared across objects having no lifetime relation.
>
> PyLeaked isn't named as PyLeakedCell since it isn't actually a cell in that
> leaked.borrow_mut() will require &mut self.
>
> diff --git a/rust/hg-cpython/src/dirstate/copymap.rs b/rust/hg-cpython/src/dirstate/copymap.rs
> --- a/rust/hg-cpython/src/dirstate/copymap.rs
> +++ b/rust/hg-cpython/src/dirstate/copymap.rs
> @@ -12,7 +12,7 @@ use cpython::{PyBytes, PyClone, PyDict,
> use std::cell::RefCell;
>
> use crate::dirstate::dirstate_map::DirstateMap;
> -use crate::ref_sharing::PyLeakedRef;
> +use crate::ref_sharing::PyLeaked;
> use hg::{utils::hg_path::HgPathBuf, CopyMapIter};
>
> py_class!(pub class CopyMap |py| {
> @@ -104,14 +104,14 @@ impl CopyMap {
>
> py_shared_iterator!(
> CopyMapKeysIterator,
> - PyLeakedRef<CopyMapIter<'static>>,
> + PyLeaked<CopyMapIter<'static>>,
> CopyMap::translate_key,
> Option<PyBytes>
> );
>
> py_shared_iterator!(
> CopyMapItemsIterator,
> - PyLeakedRef<CopyMapIter<'static>>,
> + PyLeaked<CopyMapIter<'static>>,
> CopyMap::translate_key_value,
> Option<(PyBytes, PyBytes)>
> );
> 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
> @@ -17,7 +17,7 @@ use cpython::{
> };
>
> use crate::dirstate::extract_dirstate;
> -use crate::ref_sharing::{PyLeakedRef, PySharedRefCell};
> +use crate::ref_sharing::{PyLeaked, PySharedRefCell};
> use hg::{
> utils::hg_path::{HgPath, HgPathBuf},
> DirsMultiset, DirsMultisetIter, DirstateMapError, DirstateParseError,
> @@ -123,7 +123,7 @@ impl Dirs {
>
> py_shared_iterator!(
> DirsMultisetKeysIterator,
> - PyLeakedRef<DirsMultisetIter<'static>>,
> + PyLeaked<DirsMultisetIter<'static>>,
> Dirs::translate_key,
> Option<PyBytes>
> );
> 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
> @@ -20,7 +20,7 @@ use cpython::{
> use crate::{
> dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator},
> dirstate::{dirs_multiset::Dirs, make_dirstate_tuple},
> - ref_sharing::{PyLeakedRef, PySharedRefCell},
> + ref_sharing::{PyLeaked, PySharedRefCell},
> };
> use hg::{
> utils::hg_path::{HgPath, HgPathBuf},
> @@ -483,14 +483,14 @@ py_shared_ref!(DirstateMap, RustDirstate
>
> py_shared_iterator!(
> DirstateMapKeysIterator,
> - PyLeakedRef<StateMapIter<'static>>,
> + PyLeaked<StateMapIter<'static>>,
> DirstateMap::translate_key,
> Option<PyBytes>
> );
>
> py_shared_iterator!(
> DirstateMapItemsIterator,
> - PyLeakedRef<StateMapIter<'static>>,
> + PyLeaked<StateMapIter<'static>>,
> DirstateMap::translate_key_value,
> Option<(PyBytes, PyObject)>
> );
> 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
> @@ -186,12 +186,12 @@ impl<'a, T> PySharedRef<'a, T> {
> }
>
> /// Returns a leaked reference.
> - pub fn leak_immutable(&self) -> PyResult<PyLeakedRef<&'static T>> {
> + pub fn leak_immutable(&self) -> PyResult<PyLeaked<&'static T>> {
> let state = &self.data.py_shared_state;
> unsafe {
> let (static_ref, static_state_ref) =
> state.leak_immutable(self.py, self.data)?;
> - Ok(PyLeakedRef::new(
> + Ok(PyLeaked::new(
> self.py,
> self.owner,
> static_ref,
> @@ -307,16 +307,16 @@ macro_rules! py_shared_ref {
> }
>
> /// Manage immutable references to `PyObject` leaked into Python iterators.
> -pub struct PyLeakedRef<T> {
> +pub struct PyLeaked<T> {
> inner: PyObject,
> data: Option<T>,
> py_shared_state: &'static PySharedState,
> }
>
> -// DO NOT implement Deref for PyLeakedRef<T>! Dereferencing PyLeakedRef
> +// DO NOT implement Deref for PyLeaked<T>! Dereferencing PyLeaked
> // without taking Python GIL wouldn't be safe.
>
> -impl<T> PyLeakedRef<T> {
> +impl<T> PyLeaked<T> {
> /// # Safety
> ///
> /// The `py_shared_state` must be owned by the `inner` Python object.
> @@ -355,19 +355,19 @@ impl<T> PyLeakedRef<T> {
> ///
> /// The lifetime of the object passed in to the function `f` is cheated.
> /// It's typically a static reference, but is valid only while the
> - /// corresponding `PyLeakedRef` is alive. Do not copy it out of the
> + /// corresponding `PyLeaked` is alive. Do not copy it out of the
> /// function call.
> pub unsafe fn map<U>(
> mut self,
> py: Python,
> f: impl FnOnce(T) -> U,
> - ) -> PyLeakedRef<U> {
> + ) -> PyLeaked<U> {
> // f() could make the self.data outlive. That's why map() is unsafe.
> // In order to make this function safe, maybe we'll need a way to
> // temporarily restrict the lifetime of self.data and translate the
> // returned object back to Something<'static>.
> let new_data = f(self.data.take().unwrap());
> - PyLeakedRef {
> + PyLeaked {
> inner: self.inner.clone_ref(py),
> data: Some(new_data),
> py_shared_state: self.py_shared_state,
> @@ -375,7 +375,7 @@ impl<T> PyLeakedRef<T> {
> }
> }
>
> -impl<T> Drop for PyLeakedRef<T> {
> +impl<T> Drop for PyLeaked<T> {
> fn drop(&mut self) {
> // py_shared_state should be alive since we do have
> // a Python reference to the owner object. Taking GIL makes
> @@ -383,7 +383,7 @@ impl<T> Drop for PyLeakedRef<T> {
> let gil = Python::acquire_gil();
> let py = gil.python();
> if self.data.is_none() {
> - return; // moved to another PyLeakedRef
> + return; // moved to another PyLeaked
> }
> self.py_shared_state.decrease_leak_count(py, false);
> }
> @@ -439,7 +439,7 @@ impl<T> Drop for PyLeakedRef<T> {
> ///
> /// py_shared_iterator!(
> /// MyTypeItemsIterator,
> -/// PyLeakedRef<HashMap<'static, Vec<u8>, Vec<u8>>>,
> +/// PyLeaked<HashMap<'static, Vec<u8>, Vec<u8>>>,
> /// MyType::translate_key_value,
> /// Option<(PyBytes, PyBytes)>
> /// );
More information about the Mercurial-devel
mailing list