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

implement PyTypeMethods #3705

Merged
merged 12 commits into from
Feb 18, 2024
3 changes: 1 addition & 2 deletions src/err/err_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ impl PyErrStateNormalized {

#[cfg(Py_3_12)]
pub(crate) fn ptype<'py>(&self, py: Python<'py>) -> Bound<'py, PyType> {
use crate::instance::PyNativeType;
use crate::types::any::PyAnyMethods;
self.pvalue.bind(py).get_type().as_borrowed().to_owned()
self.pvalue.bind(py).get_type()
}

#[cfg(not(Py_3_12))]
Expand Down
4 changes: 2 additions & 2 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::instance::Bound;
use crate::panic::PanicException;
use crate::type_object::PyTypeInfo;
use crate::types::any::PyAnyMethods;
use crate::types::{PyTraceback, PyType};
use crate::types::{typeobject::PyTypeMethods, PyTraceback, PyType};
use crate::{
exceptions::{self, PyBaseException},
ffi,
Expand Down Expand Up @@ -270,7 +270,7 @@ impl PyErr {
///
/// Python::with_gil(|py| {
/// let err: PyErr = PyTypeError::new_err(("some type error",));
/// assert!(err.get_type_bound(py).is(PyType::new::<PyTypeError>(py)));
/// assert!(err.get_type_bound(py).is(&PyType::new_bound::<PyTypeError>(py)));
/// });
/// ```
pub fn get_type_bound<'py>(&self, py: Python<'py>) -> Bound<'py, PyType> {
Expand Down
3 changes: 1 addition & 2 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ use crate::ffi_ptr_ext::FfiPtrExt;
use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell};
use crate::pyclass::boolean_struct::{False, True};
use crate::type_object::HasPyGilRef;
use crate::types::any::PyAnyMethods;
use crate::types::string::PyStringMethods;
use crate::types::{any::PyAnyMethods, string::PyStringMethods, typeobject::PyTypeMethods};
use crate::types::{PyDict, PyString, PyTuple};
use crate::{
ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer, PyRef, PyRefMut,
Expand Down
1 change: 1 addition & 0 deletions src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,4 @@ pub use crate::types::set::PySetMethods;
pub use crate::types::string::PyStringMethods;
pub use crate::types::traceback::PyTracebackMethods;
pub use crate::types::tuple::PyTupleMethods;
pub use crate::types::typeobject::PyTypeMethods;
12 changes: 6 additions & 6 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ impl PyAny {

/// Returns the Python type object for this object's type.
pub fn get_type(&self) -> &PyType {
self.as_borrowed().get_type()
self.as_borrowed().get_type().into_gil_ref()
}

/// Returns the Python type pointer for this object.
Expand Down Expand Up @@ -1499,7 +1499,7 @@ pub trait PyAnyMethods<'py> {
fn iter(&self) -> PyResult<Bound<'py, PyIterator>>;

/// Returns the Python type object for this object's type.
fn get_type(&self) -> &'py PyType;
fn get_type(&self) -> Bound<'py, PyType>;

/// Returns the Python type pointer for this object.
fn get_type_ptr(&self) -> *mut ffi::PyTypeObject;
Expand Down Expand Up @@ -2107,8 +2107,8 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
PyIterator::from_bound_object(self)
}

fn get_type(&self) -> &'py PyType {
unsafe { PyType::from_type_ptr(self.py(), ffi::Py_TYPE(self.as_ptr())) }
fn get_type(&self) -> Bound<'py, PyType> {
unsafe { PyType::from_type_ptr_bound(self.py(), ffi::Py_TYPE(self.as_ptr())) }
}

#[inline]
Expand Down Expand Up @@ -2265,7 +2265,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {

#[cfg(not(PyPy))]
fn py_super(&self) -> PyResult<Bound<'py, PySuper>> {
PySuper::new_bound(&self.get_type().as_borrowed(), self)
PySuper::new_bound(&self.get_type(), self)
}
}

Expand All @@ -2286,7 +2286,7 @@ impl<'py> Bound<'py, PyAny> {
N: IntoPy<Py<PyString>>,
{
let py = self.py();
let self_type = self.get_type().as_borrowed();
let self_type = self.get_type();
let attr = if let Ok(attr) = self_type.getattr(attr_name) {
attr
} else {
Expand Down
5 changes: 3 additions & 2 deletions src/types/boolobject.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
use crate::{
exceptions::PyTypeError, ffi, ffi_ptr_ext::FfiPtrExt, instance::Bound, Borrowed, FromPyObject,
IntoPy, PyAny, PyNativeType, PyObject, PyResult, Python, ToPyObject,
exceptions::PyTypeError, ffi, ffi_ptr_ext::FfiPtrExt, instance::Bound,
types::typeobject::PyTypeMethods, Borrowed, FromPyObject, IntoPy, PyAny, PyNativeType,
PyObject, PyResult, Python, ToPyObject,
};

use super::any::PyAnyMethods;
Expand Down
2 changes: 1 addition & 1 deletion src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,4 +309,4 @@ mod slice;
pub(crate) mod string;
pub(crate) mod traceback;
pub(crate) mod tuple;
mod typeobject;
pub(crate) mod typeobject;
172 changes: 140 additions & 32 deletions src/types/typeobject.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::err::{self, PyResult};
use crate::{ffi, PyAny, PyTypeInfo, Python};
use crate::instance::Borrowed;
use crate::types::any::PyAnyMethods;
use crate::{ffi, Bound, PyAny, PyNativeType, PyTypeInfo, Python};
use std::borrow::Cow;
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
use std::ffi::CStr;
Expand All @@ -11,38 +13,144 @@ pub struct PyType(PyAny);
pyobject_native_type_core!(PyType, pyobject_native_static_type_object!(ffi::PyType_Type), #checkfunction=ffi::PyType_Check);

impl PyType {
/// Creates a new type object.
/// Deprecated form of [`PyType::new_bound`].
#[inline]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`PyType::new` will be replaced by `PyType::new_bound` in a future PyO3 version"
)
)]
pub fn new<T: PyTypeInfo>(py: Python<'_>) -> &PyType {
T::type_object_bound(py).into_gil_ref()
}

/// Creates a new type object.
#[inline]
pub fn new_bound<T: PyTypeInfo>(py: Python<'_>) -> Bound<'_, PyType> {
T::type_object_bound(py)
}

/// Retrieves the underlying FFI pointer associated with this Python object.
#[inline]
pub fn as_type_ptr(&self) -> *mut ffi::PyTypeObject {
self.as_ptr() as *mut ffi::PyTypeObject
self.as_borrowed().as_type_ptr()
}

/// Retrieves the `PyType` instance for the given FFI pointer.
/// Deprecated form of [`PyType::from_type_ptr_bound`].
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
///
/// # Safety
/// - The pointer must be non-null.
/// - The pointer must be valid for the entire of the lifetime for which the reference is used.
///
/// - The pointer must a valid non-null reference to a `PyTypeObject`.
#[inline]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`from_type_ptr` will be replaced by `from_type_ptr_bound` in a future PyO3 version"
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
)
)]
pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType {
py.from_borrowed_ptr(p as *mut ffi::PyObject)
Self::from_type_ptr_bound(py, p).into_gil_ref()
}

/// Converts the given FFI pointer into `&Bound<PyType>`, to use in safe code.
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
///
/// This does not take ownership of the FFI pointer's "reference". The target type
/// object will have its reference count increased by 1, which will be released when
/// the `Bound` is dropped.
Copy link
Contributor

Choose a reason for hiding this comment

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

This does now take ownership, or did I misunderstand what you meant here?

Copy link
Member Author

@davidhewitt davidhewitt Feb 17, 2024

Choose a reason for hiding this comment

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

Uff, it's assuming that the reference is a borrowed pointer and creates a new reference. I agree my wording here was not the best.

Maybe it's better if we go a little bit inconsistent here and introduce this method with the new clearer name from_borrowed_type_ptr (forget the bound bit)?

That'd then match the from_borrowed methods which I think I'm about to introduce in a new PR...

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed that as a new commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

This reads clearer to me 👍

///
/// # Safety
/// - The pointer must be a valid non-null reference to a `PyTypeObject`
#[inline]
pub unsafe fn from_type_ptr_bound<'py>(
py: Python<'py>,
p: *mut ffi::PyTypeObject,
) -> Bound<'py, PyType> {
Borrowed::from_ptr_unchecked(py, p.cast())
.downcast_unchecked()
.to_owned()
}

/// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
pub fn qualname(&self) -> PyResult<String> {
self.as_borrowed().qualname()
}

/// Gets the full name, which includes the module, of the `PyType`.
pub fn name(&self) -> PyResult<Cow<'_, str>> {
self.as_borrowed().name()
}

/// Checks whether `self` is a subclass of `other`.
///
/// Equivalent to the Python expression `issubclass(self, other)`.
pub fn is_subclass(&self, other: &PyAny) -> PyResult<bool> {
self.as_borrowed().is_subclass(&other.as_borrowed())
}

/// Checks whether `self` is a subclass of type `T`.
///
/// Equivalent to the Python expression `issubclass(self, T)`, if the type
/// `T` is known at compile time.
pub fn is_subclass_of<T>(&self) -> PyResult<bool>
where
T: PyTypeInfo,
{
self.as_borrowed().is_subclass_of::<T>()
}
}

/// Implementation of functionality for [`PyType`].
///
/// These methods are defined for the `Bound<'py, PyType>` smart pointer, so to use method call
/// syntax these methods are separated into a trait, because stable Rust does not yet support
/// `arbitrary_self_types`.
#[doc(alias = "PyType")]
pub trait PyTypeMethods<'py> {
/// Retrieves the underlying FFI pointer associated with this Python object.
fn as_type_ptr(&self) -> *mut ffi::PyTypeObject;

/// Gets the full name, which includes the module, of the `PyType`.
fn name(&self) -> PyResult<Cow<'_, str>>;

/// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
fn qualname(&self) -> PyResult<String>;

/// Checks whether `self` is a subclass of `other`.
///
/// Equivalent to the Python expression `issubclass(self, other)`.
fn is_subclass(&self, other: &Bound<'_, PyAny>) -> PyResult<bool>;

/// Checks whether `self` is a subclass of type `T`.
///
/// Equivalent to the Python expression `issubclass(self, T)`, if the type
/// `T` is known at compile time.
fn is_subclass_of<T>(&self) -> PyResult<bool>
where
T: PyTypeInfo;
}

impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> {
/// Retrieves the underlying FFI pointer associated with this Python object.
#[inline]
fn as_type_ptr(&self) -> *mut ffi::PyTypeObject {
self.as_ptr() as *mut ffi::PyTypeObject
}

/// Gets the name of the `PyType`.
fn name(&self) -> PyResult<Cow<'_, str>> {
Borrowed::from(self).name()
}

fn qualname(&self) -> PyResult<String> {
#[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))]
let name = self.getattr(intern!(self.py(), "__qualname__"))?.extract();

#[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))]
let name = {
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::types::any::PyAnyMethods;

let obj = unsafe {
ffi::PyType_GetQualName(self.as_type_ptr()).assume_owned_or_err(self.py())?
};
Expand All @@ -53,8 +161,29 @@ impl PyType {
name
}

/// Gets the full name, which includes the module, of the `PyType`.
pub fn name(&self) -> PyResult<Cow<'_, str>> {
/// Checks whether `self` is a subclass of `other`.
///
/// Equivalent to the Python expression `issubclass(self, other)`.
fn is_subclass(&self, other: &Bound<'_, PyAny>) -> PyResult<bool> {
let result = unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), other.as_ptr()) };
err::error_on_minusone(self.py(), result)?;
Ok(result == 1)
}

/// Checks whether `self` is a subclass of type `T`.
///
/// Equivalent to the Python expression `issubclass(self, T)`, if the type
/// `T` is known at compile time.
fn is_subclass_of<T>(&self) -> PyResult<bool>
where
T: PyTypeInfo,
{
self.is_subclass(&T::type_object_bound(self.py()))
}
}

impl<'a> Borrowed<'a, '_, PyType> {
fn name(self) -> PyResult<Cow<'a, str>> {
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
{
let ptr = self.as_type_ptr();
Expand All @@ -79,33 +208,12 @@ impl PyType {
#[cfg(Py_3_11)]
let name = {
use crate::ffi_ptr_ext::FfiPtrExt;

unsafe { ffi::PyType_GetName(self.as_type_ptr()).assume_owned_or_err(self.py())? }
};

Ok(Cow::Owned(format!("{}.{}", module, name)))
}
}

/// Checks whether `self` is a subclass of `other`.
///
/// Equivalent to the Python expression `issubclass(self, other)`.
pub fn is_subclass(&self, other: &PyAny) -> PyResult<bool> {
let result = unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), other.as_ptr()) };
err::error_on_minusone(self.py(), result)?;
Ok(result == 1)
}

/// Checks whether `self` is a subclass of type `T`.
///
/// Equivalent to the Python expression `issubclass(self, T)`, if the type
/// `T` is known at compile time.
pub fn is_subclass_of<T>(&self) -> PyResult<bool>
where
T: PyTypeInfo,
{
self.is_subclass(T::type_object_bound(self.py()).as_gil_ref())
}
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion tests/test_class_basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl UnsendableChild {

fn test_unsendable<T: PyClass + 'static>() -> PyResult<()> {
let obj = Python::with_gil(|py| -> PyResult<_> {
let obj: Py<T> = PyType::new::<T>(py).call1((5,))?.extract()?;
let obj: Py<T> = PyType::new_bound::<T>(py).call1((5,))?.extract()?;

// Accessing the value inside this thread should not panic
let caught_panic =
Expand Down
Loading