# HG changeset patch # User Yuya Nishihara # Date 1568469711 -32400 # Node ID 070a38737334d0ce30f11fbc5249f2231feaf8a6 # Parent 9145abd8b96d6d8538772e80e61b65b1e2fbbdb2 rust-cpython: move py_shared_state to PySharedRefCell object The goal of this series is to encapsulate more "py_shared" thingy and reduce the size of the macro, which is hard to debug. Since py_shared_state manages the borrowing state of the object owned by PySharedRefCell, this change makes more sense. If a PyObject has more than one data to be leaked into Python world, each PySharedState should incref the parent PyObject, and keep track of the corresponding borrowing state. diff -r 9145abd8b96d -r 070a38737334 rust/hg-cpython/src/dirstate/dirs_multiset.rs --- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs Thu Oct 10 21:37:12 2019 +0200 +++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs Sat Sep 14 23:01:51 2019 +0900 @@ -16,10 +16,7 @@ Python, }; -use crate::{ - dirstate::extract_dirstate, - ref_sharing::{PySharedRefCell, PySharedState}, -}; +use crate::{dirstate::extract_dirstate, ref_sharing::PySharedRefCell}; use hg::{ utils::hg_path::{HgPath, HgPathBuf}, DirsMultiset, DirsMultisetIter, DirstateMapError, DirstateParseError, @@ -28,7 +25,6 @@ py_class!(pub class Dirs |py| { data inner: PySharedRefCell; - data py_shared_state: PySharedState; // `map` is either a `dict` or a flat iterator (usually a `set`, sometimes // a `list`) @@ -65,7 +61,6 @@ Self::create_instance( py, PySharedRefCell::new(inner), - PySharedState::default() ) } @@ -115,11 +110,7 @@ impl Dirs { pub fn from_inner(py: Python, d: DirsMultiset) -> PyResult { - Self::create_instance( - py, - PySharedRefCell::new(d), - PySharedState::default(), - ) + Self::create_instance(py, PySharedRefCell::new(d)) } fn translate_key( diff -r 9145abd8b96d -r 070a38737334 rust/hg-cpython/src/dirstate/dirstate_map.rs --- a/rust/hg-cpython/src/dirstate/dirstate_map.rs Thu Oct 10 21:37:12 2019 +0200 +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs Sat Sep 14 23:01:51 2019 +0900 @@ -21,7 +21,7 @@ use crate::{ dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator}, dirstate::{decapsule_make_dirstate_tuple, dirs_multiset::Dirs}, - ref_sharing::{PySharedRefCell, PySharedState}, + ref_sharing::PySharedRefCell, }; use hg::{ utils::hg_path::{HgPath, HgPathBuf}, @@ -44,14 +44,12 @@ // leaks, with all methods that go along for reference sharing. py_class!(pub class DirstateMap |py| { data inner: PySharedRefCell; - data py_shared_state: PySharedState; def __new__(_cls, _root: PyObject) -> PyResult { let inner = RustDirstateMap::default(); Self::create_instance( py, PySharedRefCell::new(inner), - PySharedState::default() ) } diff -r 9145abd8b96d -r 070a38737334 rust/hg-cpython/src/ref_sharing.rs --- a/rust/hg-cpython/src/ref_sharing.rs Thu Oct 10 21:37:12 2019 +0200 +++ b/rust/hg-cpython/src/ref_sharing.rs Sat Sep 14 23:01:51 2019 +0900 @@ -27,7 +27,7 @@ use std::cell::{Cell, Ref, RefCell, RefMut}; /// Manages the shared state between Python and Rust -#[derive(Default)] +#[derive(Debug, Default)] pub struct PySharedState { leak_count: Cell, mutably_borrowed: Cell, @@ -118,12 +118,14 @@ #[derive(Debug)] pub struct PySharedRefCell { inner: RefCell, + pub py_shared_state: PySharedState, // TODO: remove pub } impl PySharedRefCell { - pub const fn new(value: T) -> PySharedRefCell { + pub fn new(value: T) -> PySharedRefCell { Self { inner: RefCell::new(value), + py_shared_state: PySharedState::default(), } } @@ -193,12 +195,6 @@ /// /// # Warning /// -/// The targeted `py_class!` needs to have the -/// `data py_shared_state: PySharedState;` data attribute to compile. -/// A better, more complicated macro is needed to automatically insert it, -/// but this one is not yet really battle tested (what happens when -/// multiple references are needed?). See the example below. -/// /// TODO allow Python container types: for now, integration with the garbage /// collector does not extend to Rust structs holding references to Python /// objects. Should the need surface, `__traverse__` and `__clear__` will @@ -223,7 +219,6 @@ /// /// py_class!(pub class MyType |py| { /// data inner: PySharedRefCell; -/// data py_shared_state: PySharedState; /// }); /// /// py_shared_ref!(MyType, MyStruct, inner, MyTypeLeakedRef); @@ -244,7 +239,7 @@ // assert $data_member type use crate::ref_sharing::PySharedRefCell; let data: &PySharedRefCell<_> = self.$data_member(py); - self.py_shared_state(py) + data.py_shared_state .borrow_mut(py, unsafe { data.borrow_mut() }) } @@ -263,7 +258,7 @@ use crate::ref_sharing::PySharedRefCell; let data: &PySharedRefCell<_> = self.$data_member(py); let static_ref = - self.py_shared_state(py).leak_immutable(py, data)?; + data.py_shared_state.leak_immutable(py, data)?; let leak_handle = $leaked::new(py, self); Ok((leak_handle, static_ref)) } @@ -292,7 +287,7 @@ fn drop(&mut self) { let gil = Python::acquire_gil(); let py = gil.python(); - let state = self.inner.py_shared_state(py); + let state = &self.inner.$data_member(py).py_shared_state; unsafe { state.decrease_leak_count(py, false); } @@ -324,7 +319,6 @@ /// /// py_class!(pub class MyType |py| { /// data inner: PySharedRefCell; -/// data py_shared_state: PySharedState; /// /// def __iter__(&self) -> PyResult { /// let (leak_handle, leaked_ref) = unsafe { self.leak_immutable(py)? };