# HG changeset patch # User Yuya Nishihara # Date 1567942015 -32400 # Node ID 5cb8867c9e2b598b2b3faecb701341d75994a296 # Parent aaec70a5f9a860141e7e0f0b205ce394b2c180a8 rust-cpython: move $leaked struct out of macro It wasn't easy to hack the $leaked struct since errors in macro would generate lots of compile errors. Let's make it a plain struct so we can easily extend it. PyLeakedRef keeps a more generic PyObject instead of the $name struct since it no longer has to call any specific methods implemented by the $name class. $leaked parameter in py_shared_iterator!() is kept for future change. diff -r aaec70a5f9a8 -r 5cb8867c9e2b rust/hg-cpython/src/dirstate/copymap.rs --- a/rust/hg-cpython/src/dirstate/copymap.rs Sun Sep 15 16:04:45 2019 +0900 +++ b/rust/hg-cpython/src/dirstate/copymap.rs Sun Sep 08 20:26:55 2019 +0900 @@ -11,7 +11,8 @@ use cpython::{PyBytes, PyClone, PyDict, PyObject, PyResult, Python}; use std::cell::RefCell; -use crate::dirstate::dirstate_map::{DirstateMap, DirstateMapLeakedRef}; +use crate::dirstate::dirstate_map::DirstateMap; +use crate::ref_sharing::PyLeakedRef; use hg::{utils::hg_path::HgPathBuf, CopyMapIter}; py_class!(pub class CopyMap |py| { @@ -103,7 +104,7 @@ py_shared_iterator!( CopyMapKeysIterator, - DirstateMapLeakedRef, + PyLeakedRef, CopyMapIter<'static>, CopyMap::translate_key, Option @@ -111,7 +112,7 @@ py_shared_iterator!( CopyMapItemsIterator, - DirstateMapLeakedRef, + PyLeakedRef, CopyMapIter<'static>, CopyMap::translate_key_value, Option<(PyBytes, PyBytes)> diff -r aaec70a5f9a8 -r 5cb8867c9e2b rust/hg-cpython/src/dirstate/dirs_multiset.rs --- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs Sun Sep 15 16:04:45 2019 +0900 +++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs Sun Sep 08 20:26:55 2019 +0900 @@ -16,7 +16,8 @@ Python, }; -use crate::{dirstate::extract_dirstate, ref_sharing::PySharedRefCell}; +use crate::dirstate::extract_dirstate; +use crate::ref_sharing::{PyLeakedRef, PySharedRefCell}; use hg::{ utils::hg_path::{HgPath, HgPathBuf}, DirsMultiset, DirsMultisetIter, DirstateMapError, DirstateParseError, @@ -106,7 +107,7 @@ } }); -py_shared_ref!(Dirs, DirsMultiset, inner, DirsMultisetLeakedRef,); +py_shared_ref!(Dirs, DirsMultiset, inner); impl Dirs { pub fn from_inner(py: Python, d: DirsMultiset) -> PyResult { @@ -123,7 +124,7 @@ py_shared_iterator!( DirsMultisetKeysIterator, - DirsMultisetLeakedRef, + PyLeakedRef, DirsMultisetIter<'static>, Dirs::translate_key, Option diff -r aaec70a5f9a8 -r 5cb8867c9e2b rust/hg-cpython/src/dirstate/dirstate_map.rs --- a/rust/hg-cpython/src/dirstate/dirstate_map.rs Sun Sep 15 16:04:45 2019 +0900 +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs Sun Sep 08 20:26:55 2019 +0900 @@ -21,7 +21,7 @@ use crate::{ dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator}, dirstate::{decapsule_make_dirstate_tuple, dirs_multiset::Dirs}, - ref_sharing::PySharedRefCell, + ref_sharing::{PyLeakedRef, PySharedRefCell}, }; use hg::{ utils::hg_path::{HgPath, HgPathBuf}, @@ -501,11 +501,11 @@ } } -py_shared_ref!(DirstateMap, RustDirstateMap, inner, DirstateMapLeakedRef,); +py_shared_ref!(DirstateMap, RustDirstateMap, inner); py_shared_iterator!( DirstateMapKeysIterator, - DirstateMapLeakedRef, + PyLeakedRef, StateMapIter<'static>, DirstateMap::translate_key, Option @@ -513,7 +513,7 @@ py_shared_iterator!( DirstateMapItemsIterator, - DirstateMapLeakedRef, + PyLeakedRef, StateMapIter<'static>, DirstateMap::translate_key_value, Option<(PyBytes, PyObject)> diff -r aaec70a5f9a8 -r 5cb8867c9e2b rust/hg-cpython/src/ref_sharing.rs --- a/rust/hg-cpython/src/ref_sharing.rs Sun Sep 15 16:04:45 2019 +0900 +++ b/rust/hg-cpython/src/ref_sharing.rs Sun Sep 08 20:26:55 2019 +0900 @@ -23,7 +23,7 @@ //! Macros for use in the `hg-cpython` bridge library. use crate::exceptions::AlreadyBorrowed; -use cpython::{PyResult, Python}; +use cpython::{PyClone, PyObject, PyResult, Python}; use std::cell::{Cell, Ref, RefCell, RefMut}; /// Manages the shared state between Python and Rust @@ -219,9 +219,6 @@ /// * `$inner_struct` is the identifier of the underlying Rust struct /// * `$data_member` is the identifier of the data member of `$inner_struct` /// that will be shared. -/// * `$leaked` is the identifier to give to the struct that will manage -/// references to `$name`, to be used for example in other macros like -/// `py_shared_iterator`. /// /// # Example /// @@ -234,14 +231,13 @@ /// data inner: PySharedRefCell; /// }); /// -/// py_shared_ref!(MyType, MyStruct, inner, MyTypeLeakedRef); +/// py_shared_ref!(MyType, MyStruct, inner); /// ``` macro_rules! py_shared_ref { ( $name: ident, $inner_struct: ident, - $data_member: ident, - $leaked: ident, + $data_member: ident ) => { impl $name { // TODO: remove this function in favor of inner(py).borrow_mut() @@ -266,59 +262,59 @@ unsafe fn leak_immutable<'a>( &'a self, py: Python<'a>, - ) -> PyResult<($leaked, &'static $inner_struct)> { + ) -> PyResult<(PyLeakedRef, &'static $inner_struct)> { + use cpython::PythonObject; // assert $data_member type use crate::ref_sharing::PySharedRefCell; let data: &PySharedRefCell<_> = self.$data_member(py); let (static_ref, static_state_ref) = data.py_shared_state.leak_immutable(py, data)?; - let leak_handle = $leaked::new(py, self, static_state_ref); + let leak_handle = + PyLeakedRef::new(py, self.as_object(), static_state_ref); Ok((leak_handle, static_ref)) } } + }; +} - /// Manage immutable references to `$name` leaked into Python - /// iterators. - /// - /// In truth, this does not represent leaked references themselves; - /// it is instead useful alongside them to manage them. - pub struct $leaked { - _inner: $name, - py_shared_state: &'static crate::ref_sharing::PySharedState, - } +/// Manage immutable references to `PyObject` leaked into Python iterators. +/// +/// In truth, this does not represent leaked references themselves; +/// it is instead useful alongside them to manage them. +pub struct PyLeakedRef { + _inner: PyObject, + py_shared_state: &'static PySharedState, +} - impl $leaked { - /// # Safety - /// - /// The `py_shared_state` must be owned by the `inner` Python - /// object. - // Marked as unsafe so client code wouldn't construct $leaked - // struct by mistake. Its drop() is unsafe. - unsafe fn new( - py: Python, - inner: &$name, - py_shared_state: &'static crate::ref_sharing::PySharedState, - ) -> Self { - Self { - _inner: inner.clone_ref(py), - py_shared_state, - } - } +impl PyLeakedRef { + /// # Safety + /// + /// The `py_shared_state` must be owned by the `inner` Python object. + // Marked as unsafe so client code wouldn't construct PyLeakedRef + // struct by mistake. Its drop() is unsafe. + pub unsafe fn new( + py: Python, + inner: &PyObject, + py_shared_state: &'static PySharedState, + ) -> Self { + Self { + _inner: inner.clone_ref(py), + py_shared_state, } + } +} - impl Drop for $leaked { - fn drop(&mut self) { - // py_shared_state should be alive since we do have - // a Python reference to the owner object. Taking GIL makes - // sure that the state is only accessed by this thread. - let gil = Python::acquire_gil(); - let py = gil.python(); - unsafe { - self.py_shared_state.decrease_leak_count(py, false); - } - } +impl Drop for PyLeakedRef { + fn drop(&mut self) { + // py_shared_state should be alive since we do have + // a Python reference to the owner object. Taking GIL makes + // sure that the state is only accessed by this thread. + let gil = Python::acquire_gil(); + let py = gil.python(); + unsafe { + self.py_shared_state.decrease_leak_count(py, false); } - }; + } } /// Defines a `py_class!` that acts as a Python iterator over a Rust iterator. @@ -372,7 +368,7 @@ /// /// py_shared_iterator!( /// MyTypeItemsIterator, -/// MyTypeLeakedRef, +/// PyLeakedRef, /// HashMap<'static, Vec, Vec>, /// MyType::translate_key_value, /// Option<(PyBytes, PyBytes)> @@ -381,7 +377,7 @@ macro_rules! py_shared_iterator { ( $name: ident, - $leaked: ident, + $leaked: ty, $iterator_type: ty, $success_func: expr, $success_type: ty