Remove ManagedPyRef

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

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.

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<T>,
-    _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<Self>);
-}
-
-/// 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<T: ToPyObject + ?Sized> 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<Self>) {
-        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<T: ToPyObject + AsPyPointer + ?Sized> 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<T>) {}
-}
-
-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);
-        }
-    }
 }

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;