diff --git a/CHANGELOG.md b/CHANGELOG.md index de82f92b..ec2fb2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Removed +- Remove `ManagedPyRef` (unused, and needs specialization) [#930](https://github.com/PyO3/pyo3/pull/930) ## [0.10.0] ### Added diff --git a/src/conversion.rs b/src/conversion.rs index b1e18003..051d0b8d 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -87,8 +87,6 @@ pub trait ToPyObject { /// all [ToPyObject] and creates a new object using [ToPyObject::to_object], /// while the fast one is only implemented for AsPyPointer (we know /// that every AsPyPointer is also ToPyObject) and uses [AsPyPointer::as_ptr()] -/// -/// This trait should eventually be replaced with [ManagedPyRef](crate::ManagedPyRef). pub trait ToBorrowedObject: ToPyObject { /// Converts self into a Python object and calls the specified closure /// on the native FFI pointer underlying the Python object. diff --git a/src/instance.rs b/src/instance.rs index 8b4911a2..8cc6ddfa 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -323,108 +323,9 @@ where } } -/// Reference to a converted [ToPyObject]. -/// -/// Many methods want to take anything that can be converted into a Python object. This type -/// takes care of both types types that are already Python object (i.e. implement -/// [AsPyPointer]) and those that don't (i.e. [ToPyObject] types). -/// For the [AsPyPointer] types, we just use the borrowed pointer, which is a lot faster -/// and simpler than creating a new extra object. The remaning [ToPyObject] types are -/// converted to Python objects, the owned pointer is stored and decref'd on drop. -/// -/// # Example -/// -/// ``` -/// use pyo3::ffi; -/// use pyo3::{ToPyObject, AsPyPointer, PyNativeType, ManagedPyRef}; -/// use pyo3::types::{PyDict, PyAny}; -/// -/// pub fn get_dict_item<'p>(dict: &'p PyDict, key: &impl ToPyObject) -> Option<&'p PyAny> { -/// let key = ManagedPyRef::from_to_pyobject(dict.py(), key); -/// unsafe { -/// dict.py().from_borrowed_ptr_or_opt(ffi::PyDict_GetItem(dict.as_ptr(), key.as_ptr())) -/// } -/// } -/// ``` -#[repr(transparent)] -pub struct ManagedPyRef<'p, T: ToPyObject + ?Sized> { - data: *mut ffi::PyObject, - data_type: PhantomData, - _py: Python<'p>, -} - -/// This should eventually be replaced with a generic `IntoPy` trait impl by figuring -/// out the correct lifetime annotation to make the compiler happy. -impl<'p, T: ToPyObject> ManagedPyRef<'p, T> { - pub fn from_to_pyobject(py: Python<'p>, to_pyobject: &T) -> Self { - to_pyobject.to_managed_py_ref(py) - } -} - -impl<'p, T: ToPyObject> AsPyPointer for ManagedPyRef<'p, T> { - fn as_ptr(&self) -> *mut ffi::PyObject { - self.data - } -} - -/// Helper trait to choose the right implementation for [ManagedPyRef]. -pub trait ManagedPyRefDispatch: ToPyObject { - /// Optionally converts into a Python object and stores the pointer to the python heap. - fn to_managed_py_ref<'p>(&self, py: Python<'p>) -> ManagedPyRef<'p, Self>; - - /// Dispatch over a xdecref and a noop drop impl - fn drop_impl(borrowed: &mut ManagedPyRef); -} - -/// Case 1: It's a Rust object which still needs to be converted to a Python object. -/// This means we're storing the owned pointer that into_ptr() has given us -/// and therefore need to xdecref when we're done. -/// -/// Note that the actual implementations are part of the trait declaration to avoid -/// a specialization error -impl ManagedPyRefDispatch for T { - /// Contains the case 1 impl (with to_object) to avoid a specialization error - default fn to_managed_py_ref<'p>(&self, py: Python<'p>) -> ManagedPyRef<'p, Self> { - ManagedPyRef { - data: self.to_object(py).into_ptr(), - data_type: PhantomData, - _py: py, - } - } - - /// Contains the case 1 impl (decref) to avoid a specialization error - default fn drop_impl(borrowed: &mut ManagedPyRef) { - unsafe { ffi::Py_DECREF(borrowed.data) }; - } -} - -/// Case 2: It's an object on the Python heap, we're just storing a borrowed pointer. -/// The object we're getting is an owned pointer, it might have it's own drop impl. -impl ManagedPyRefDispatch for T { - /// Use AsPyPointer to copy the pointer and store it as borrowed pointer - fn to_managed_py_ref<'p>(&self, py: Python<'p>) -> ManagedPyRef<'p, Self> { - ManagedPyRef { - data: self.as_ptr(), - data_type: PhantomData, - _py: py, - } - } - - /// We have a borrowed pointer, so nothing to do here - fn drop_impl(_: &mut ManagedPyRef) {} -} - -impl<'p, T: ToPyObject + ?Sized> Drop for ManagedPyRef<'p, T> { - /// Uses the internal [ManagedPyRefDispatch] trait to get the right drop impl without causing - /// a specialization error - fn drop(&mut self) { - ManagedPyRefDispatch::drop_impl(self); - } -} - #[cfg(test)] mod test { - use super::{ManagedPyRef, Py}; + use super::Py; use crate::ffi; use crate::types::PyDict; use crate::{AsPyPointer, Python}; @@ -439,37 +340,4 @@ mod test { }; assert_eq!(unsafe { ffi::Py_REFCNT(dict.as_ptr()) }, 1); } - - #[test] - fn borrowed_py_ref_with_to_pointer() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let native = PyDict::new(py); - let ref_count = unsafe { ffi::Py_REFCNT(native.as_ptr()) }; - let borrowed = ManagedPyRef::from_to_pyobject(py, native); - assert_eq!(native.as_ptr(), borrowed.data); - assert_eq!(ref_count, unsafe { ffi::Py_REFCNT(borrowed.data) }); - drop(borrowed); - assert_eq!(ref_count, unsafe { ffi::Py_REFCNT(native.as_ptr()) }); - } - - #[test] - fn borrowed_py_ref_with_to_object() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let convertible = (1, 2, 3); - let borrowed = ManagedPyRef::from_to_pyobject(py, &convertible); - let ptr = borrowed.data; - // The refcountwould become 0 after dropping, which means the gc can free the pointer - // and getting the refcount would be UB. This incref ensures that it remains 1 - unsafe { - ffi::Py_INCREF(ptr); - } - assert_eq!(2, unsafe { ffi::Py_REFCNT(ptr) }); - drop(borrowed); - assert_eq!(1, unsafe { ffi::Py_REFCNT(ptr) }); - unsafe { - ffi::Py_DECREF(ptr); - } - } } diff --git a/src/lib.rs b/src/lib.rs index edb28423..b7825803 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -140,7 +140,7 @@ pub use crate::conversion::{ }; pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyErrValue, PyResult}; pub use crate::gil::{GILGuard, GILPool}; -pub use crate::instance::{AsPyRef, ManagedPyRef, Py, PyNativeType}; +pub use crate::instance::{AsPyRef, Py, PyNativeType}; pub use crate::object::PyObject; pub use crate::pycell::{PyCell, PyRef, PyRefMut}; pub use crate::pyclass::PyClass;