Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Bound::as_any and Bound::into_any (and same for Py) #3785

Merged
merged 3 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/3785.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `Py::as_any` and `Py::into_any`.
152 changes: 92 additions & 60 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
};
use crate::{gil, PyTypeCheck};
use std::marker::PhantomData;
use std::mem::{self, ManuallyDrop};
use std::mem::ManuallyDrop;
use std::ops::Deref;
use std::ptr::NonNull;

Expand All @@ -35,10 +35,15 @@ pub unsafe trait PyNativeType: Sized {
///
/// This is available as a migration tool to adjust code from the deprecated "GIL Refs"
/// API to the `Bound` smart pointer API.
#[inline]
fn as_borrowed(&self) -> Borrowed<'_, '_, Self::AsRefSource> {
// Safety: &'py Self is expected to be a Python pointer,
// so has the same layout as Borrowed<'py, 'py, T>
unsafe { std::mem::transmute(self) }
Borrowed(
unsafe { NonNull::new_unchecked(self as *const Self as *mut _) },
PhantomData,
self.py(),
)
}

/// Returns a GIL marker constrained to the lifetime of this type.
Expand Down Expand Up @@ -96,14 +101,6 @@ impl<'py> Bound<'py, PyAny> {
}
}

impl<'py, T> Bound<'py, T> {
/// Helper to cast to Bound<'py, PyAny>
pub(crate) fn as_any(&self) -> &Bound<'py, PyAny> {
// Safety: all Bound<T> have the same memory layout, and all Bound<T> are valid Bound<PyAny>
unsafe { std::mem::transmute(self) }
}
}

impl<'py, T> std::fmt::Debug for Bound<'py, T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
let any = self.as_any();
Expand Down Expand Up @@ -150,25 +147,29 @@ impl<'py, T> AsRef<Bound<'py, PyAny>> for Bound<'py, T>
where
T: AsRef<PyAny>,
{
#[inline]
fn as_ref(&self) -> &Bound<'py, PyAny> {
self.as_any()
}
}

impl<T> Clone for Bound<'_, T> {
#[inline]
fn clone(&self) -> Self {
Self(self.0, ManuallyDrop::new(self.1.clone_ref(self.0)))
}
}

impl<T> Drop for Bound<'_, T> {
#[inline]
fn drop(&mut self) {
unsafe { ffi::Py_DECREF(self.1.as_ptr()) }
unsafe { ffi::Py_DECREF(self.as_ptr()) }
}
}

impl<'py, T> Bound<'py, T> {
/// Returns the GIL token associated with this object.
#[inline]
pub fn py(&self) -> Python<'py> {
self.0
}
Expand All @@ -194,10 +195,26 @@ impl<'py, T> Bound<'py, T> {
/// of the pointer or decrease the reference count (e.g. with [`pyo3::ffi::Py_DecRef`](crate::ffi::Py_DecRef)).
#[inline]
pub fn into_ptr(self) -> *mut ffi::PyObject {
self.into_non_null().as_ptr()
ManuallyDrop::new(self).as_ptr()
}

/// Helper to cast to `Bound<'py, PyAny>`.
#[inline]
pub fn as_any(&self) -> &Bound<'py, PyAny> {
// Safety: all Bound<T> have the same memory layout, and all Bound<T> are valid
// Bound<PyAny>, so pointer casting is valid.
unsafe { &*(self as *const Self).cast::<Bound<'py, PyAny>>() }
}

/// Helper to cast to `Bound<'py, PyAny>`, transferring ownership.
#[inline]
pub fn into_any(self) -> Bound<'py, PyAny> {
// Safety: all Bound<T> are valid Bound<PyAny>
Bound(self.0, ManuallyDrop::new(self.unbind().into_any()))
}

/// Casts this `Bound<T>` to a `Borrowed<T>` smart pointer.
#[inline]
pub fn as_borrowed<'a>(&'a self) -> Borrowed<'a, 'py, T> {
Borrowed(
unsafe { NonNull::new_unchecked(self.as_ptr()) },
Expand All @@ -208,15 +225,18 @@ impl<'py, T> Bound<'py, T> {

/// Removes the connection for this `Bound<T>` from the GIL, allowing
/// it to cross thread boundaries.
#[inline]
pub fn unbind(self) -> Py<T> {
// Safety: the type T is known to be correct and the ownership of the
// pointer is transferred to the new Py<T> instance.
unsafe { Py::from_non_null(self.into_non_null()) }
let non_null = (ManuallyDrop::new(self).1).0;
unsafe { Py::from_non_null(non_null) }
}

/// Casts this `Bound<T>` as the corresponding "GIL Ref" type.
///
/// This is a helper to be used for migration from the deprecated "GIL Refs" API.
#[inline]
pub fn as_gil_ref(&'py self) -> &'py T::AsRefTarget
where
T: HasPyGilRef,
Expand All @@ -228,20 +248,13 @@ impl<'py, T> Bound<'py, T> {
/// [release pool](Python::from_owned_ptr).
///
/// This is a helper to be used for migration from the deprecated "GIL Refs" API.
#[inline]
pub fn into_gil_ref(self) -> &'py T::AsRefTarget
where
T: HasPyGilRef,
{
unsafe { self.py().from_owned_ptr(self.into_ptr()) }
}

// Internal helper to convert `self` into a `NonNull` which owns the
// Python reference.
pub(crate) fn into_non_null(self) -> NonNull<ffi::PyObject> {
// wrap in ManuallyDrop to avoid running Drop for self and decreasing
// the reference count
ManuallyDrop::new(self).1 .0
}
}

unsafe impl<T> AsPyPointer for Bound<'_, T> {
Expand All @@ -262,11 +275,7 @@ pub struct Borrowed<'a, 'py, T>(NonNull<ffi::PyObject>, PhantomData<&'a Py<T>>,
impl<'py, T> Borrowed<'_, 'py, T> {
/// Creates a new owned `Bound` from this borrowed reference by increasing the reference count.
pub(crate) fn to_owned(self) -> Bound<'py, T> {
unsafe { ffi::Py_INCREF(self.as_ptr()) };
Bound(
self.py(),
ManuallyDrop::new(unsafe { Py::from_non_null(self.0) }),
)
(*self).clone()
}
}

Expand Down Expand Up @@ -348,6 +357,7 @@ impl<'py, T> Deref for Borrowed<'_, 'py, T> {
}

impl<T> Clone for Borrowed<'_, '_, T> {
#[inline]
fn clone(&self) -> Self {
*self
}
Expand All @@ -357,13 +367,15 @@ impl<T> Copy for Borrowed<'_, '_, T> {}

impl<T> ToPyObject for Borrowed<'_, '_, T> {
/// Converts `Py` instance -> PyObject.
#[inline]
fn to_object(&self, py: Python<'_>) -> PyObject {
(*self).into_py(py)
}
}

impl<T> IntoPy<PyObject> for Borrowed<'_, '_, T> {
/// Converts `Py` instance -> PyObject.
#[inline]
fn into_py(self, py: Python<'_>) -> PyObject {
self.to_owned().into_py(py)
}
Expand Down Expand Up @@ -709,9 +721,22 @@ impl<T> Py<T> {
/// of the pointer or decrease the reference count (e.g. with [`pyo3::ffi::Py_DecRef`](crate::ffi::Py_DecRef)).
#[inline]
pub fn into_ptr(self) -> *mut ffi::PyObject {
let ptr = self.0.as_ptr();
std::mem::forget(self);
ptr
ManuallyDrop::new(self).0.as_ptr()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice idiom of using ManuallyDrop for conciseness instead of safety, compared to using forget!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it only works when pulling a Copy field out of the ManuallyDrop, but fortunately that's what we're doing here! 😂

}

/// Helper to cast to `Py<PyAny>`.
#[inline]
pub fn as_any(&self) -> &Py<PyAny> {
// Safety: all Py<T> have the same memory layout, and all Py<T> are valid
// Py<PyAny>, so pointer casting is valid.
unsafe { &*(self as *const Self).cast::<Py<PyAny>>() }
}

/// Helper to cast to `Py<PyAny>`, transferring ownership.
#[inline]
pub fn into_any(self) -> Py<PyAny> {
// Safety: all Py<T> are valid Py<PyAny>
unsafe { Py::from_non_null(ManuallyDrop::new(self).0) }
}
}

Expand Down Expand Up @@ -868,17 +893,20 @@ where

impl<T> Py<T> {
/// Attaches this `Py` to the given Python context, allowing access to further Python APIs.
#[inline]
pub fn bind<'py>(&self, _py: Python<'py>) -> &Bound<'py, T> {
// Safety: `Bound` has the same layout as `Py`
unsafe { &*(self as *const Py<T>).cast() }
}

/// Same as `bind` but takes ownership of `self`.
#[inline]
pub fn into_bound(self, py: Python<'_>) -> Bound<'_, T> {
Bound(py, ManuallyDrop::new(self))
}

/// Same as `bind` but produces a `Borrowed<T>` instead of a `Bound<T>`.
#[inline]
pub fn bind_borrowed<'a, 'py>(&'a self, py: Python<'py>) -> Borrowed<'a, 'py, T> {
Borrowed(self.0, PhantomData, py)
}
Expand Down Expand Up @@ -1243,24 +1271,16 @@ impl<T> Py<T> {
///
/// # Safety
/// `ptr` must point to a Python object of type T.
#[inline]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed inline here because it's only used in this module, I figure it'll be inlined anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is available for inlining (just because it is in the same crate), #[inline] will still increase the likelyhood that it actually is inlined, i.e. it will bias the heuristics towards using that option, i.e. I would not remove it since this should be compiled away completed only after inlining which should happen immediately in the first pass of LLVM over the resulting MIR.

pub(crate) unsafe fn from_non_null(ptr: NonNull<ffi::PyObject>) -> Self {
unsafe fn from_non_null(ptr: NonNull<ffi::PyObject>) -> Self {
Self(ptr, PhantomData)
}

/// Returns the inner pointer without decreasing the refcount.
#[inline]
fn into_non_null(self) -> NonNull<ffi::PyObject> {
let pointer = self.0;
mem::forget(self);
pointer
}
}

impl<T> ToPyObject for Py<T> {
/// Converts `Py` instance -> PyObject.
#[inline]
fn to_object(&self, py: Python<'_>) -> PyObject {
unsafe { PyObject::from_borrowed_ptr(py, self.as_ptr()) }
self.clone_ref(py).into_any()
}
}

Expand All @@ -1269,7 +1289,7 @@ impl<T> IntoPy<PyObject> for Py<T> {
/// Consumes `self` without calling `Py_DECREF()`.
#[inline]
fn into_py(self, _py: Python<'_>) -> PyObject {
unsafe { PyObject::from_non_null(self.into_non_null()) }
self.into_any()
}
}

Expand All @@ -1281,27 +1301,26 @@ impl<T> IntoPy<PyObject> for &'_ Py<T> {
}

impl<T> ToPyObject for Bound<'_, T> {
/// Converts `Py` instance -> PyObject.
/// Converts `&Bound` instance -> PyObject, increasing the reference count.
#[inline]
fn to_object(&self, py: Python<'_>) -> PyObject {
unsafe { PyObject::from_borrowed_ptr(py, self.as_ptr()) }
self.clone().into_py(py)
}
}

impl<T> IntoPy<PyObject> for Bound<'_, T> {
/// Converts a `Py` instance to `PyObject`.
/// Consumes `self` without calling `Py_DECREF()`.
/// Converts a `Bound` instance to `PyObject`.
#[inline]
fn into_py(self, _py: Python<'_>) -> PyObject {
unsafe { PyObject::from_non_null(self.into_non_null()) }
self.into_any().unbind()
}
}

impl<T> IntoPy<PyObject> for &Bound<'_, T> {
/// Converts a `Py` instance to `PyObject`.
/// Consumes `self` without calling `Py_DECREF()`.
/// Converts `&Bound` instance -> PyObject, increasing the reference count.
#[inline]
fn into_py(self, _py: Python<'_>) -> PyObject {
unsafe { PyObject::from_non_null(self.clone().into_non_null()) }
fn into_py(self, py: Python<'_>) -> PyObject {
self.to_object(py)
}
}

Expand All @@ -1313,18 +1332,13 @@ unsafe impl<T> crate::AsPyPointer for Py<T> {
}
}

impl std::convert::From<&'_ PyAny> for PyObject {
fn from(obj: &PyAny) -> Self {
unsafe { Py::from_borrowed_ptr(obj.py(), obj.as_ptr()) }
}
}

impl<T> std::convert::From<&'_ T> for PyObject
where
T: PyNativeType + AsRef<PyAny>,
T: PyNativeType,
{
#[inline]
fn from(obj: &T) -> Self {
unsafe { Py::from_borrowed_ptr(obj.py(), obj.as_ref().as_ptr()) }
obj.as_borrowed().to_owned().into_any().unbind()
}
}

Expand All @@ -1334,7 +1348,7 @@ where
{
#[inline]
fn from(other: Py<T>) -> Self {
unsafe { Self::from_non_null(other.into_non_null()) }
other.into_any()
}
}

Expand Down Expand Up @@ -1362,7 +1376,7 @@ where
T: PyClass,
{
fn from(cell: &PyCell<T>) -> Self {
unsafe { Py::from_borrowed_ptr(cell.py(), cell.as_ptr()) }
cell.as_borrowed().to_owned().unbind()
}
}

Expand Down Expand Up @@ -1724,6 +1738,24 @@ a = A()
});
}

#[test]
fn test_bound_as_any() {
Python::with_gil(|py| {
let obj = PyString::new_bound(py, "hello world");
let any = obj.as_any();
assert_eq!(any.as_ptr(), obj.as_ptr());
});
}

#[test]
fn test_bound_into_any() {
Python::with_gil(|py| {
let obj = PyString::new_bound(py, "hello world");
let any = obj.clone().into_any();
assert_eq!(any.as_ptr(), obj.as_ptr());
});
}

#[cfg(feature = "macros")]
mod using_macros {
use crate::PyCell;
Expand Down
Loading