Merge pull request #930 from davidhewitt/remove-managed-py-ref

Remove ManagedPyRef
This commit is contained in:
Yuji Kanagawa 2020-05-16 13:31:46 +09:00 committed by GitHub
commit 8e6398029d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 4 additions and 136 deletions

View File

@ -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.1] - 2020-05-14
### Fixed

View File

@ -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.

View File

@ -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);
}
}
}

View File

@ -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;