From f04ad56df465170c1e5f5970c2a41cce96454ff3 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sun, 18 Feb 2024 03:07:48 +0000 Subject: [PATCH] implement `PyTypeMethods` (#3705) * implement `PyTypeMethods` * introduce `PyType` bound constructors * `from_type_ptr_bound` instead of `from_type_ptr_borrowed` * correct conditional code * just make `from_type_ptr_bound` create an owned `Bound` * correct docstrings Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> * Rework as `PyType::from_borrowed_type_ptr` * correct doc link to `from_borrowed_type_ptr` Co-authored-by: Lily Foote * remove unneeded lifetime name --------- Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> Co-authored-by: Lily Foote --- src/err/err_state.rs | 3 +- src/err/mod.rs | 5 +- src/instance.rs | 3 +- src/prelude.rs | 1 + src/types/any.rs | 12 +-- src/types/boolobject.rs | 5 +- src/types/mod.rs | 2 +- src/types/typeobject.rs | 171 ++++++++++++++++++++++++++++++------- tests/test_class_basics.rs | 2 +- 9 files changed, 155 insertions(+), 49 deletions(-) diff --git a/src/err/err_state.rs b/src/err/err_state.rs index eb03d8b9..9f85296f 100644 --- a/src/err/err_state.rs +++ b/src/err/err_state.rs @@ -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))] diff --git a/src/err/mod.rs b/src/err/mod.rs index cf74739d..d55c8f45 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -2,8 +2,7 @@ use crate::instance::Bound; use crate::panic::PanicException; use crate::type_object::PyTypeInfo; use crate::types::any::PyAnyMethods; -use crate::types::string::PyStringMethods; -use crate::types::{PyTraceback, PyType}; +use crate::types::{string::PyStringMethods, typeobject::PyTypeMethods, PyTraceback, PyType}; use crate::{ exceptions::{self, PyBaseException}, ffi, @@ -280,7 +279,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::(py))); + /// assert!(err.get_type_bound(py).is(&PyType::new_bound::(py))); /// }); /// ``` pub fn get_type_bound<'py>(&self, py: Python<'py>) -> Bound<'py, PyType> { diff --git a/src/instance.rs b/src/instance.rs index 821aad81..3e280c43 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -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, DowncastError, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer, diff --git a/src/prelude.rs b/src/prelude.rs index 47951aed..3ac239f4 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -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; diff --git a/src/types/any.rs b/src/types/any.rs index 33b3d964..1cb6bb77 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -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. @@ -1499,7 +1499,7 @@ pub trait PyAnyMethods<'py> { fn iter(&self) -> PyResult>; /// 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; @@ -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_borrowed_type_ptr(self.py(), ffi::Py_TYPE(self.as_ptr())) } } #[inline] @@ -2265,7 +2265,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { #[cfg(not(PyPy))] fn py_super(&self) -> PyResult> { - PySuper::new_bound(&self.get_type().as_borrowed(), self) + PySuper::new_bound(&self.get_type(), self) } } @@ -2286,7 +2286,7 @@ impl<'py> Bound<'py, PyAny> { N: IntoPy>, { 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 { diff --git a/src/types/boolobject.rs b/src/types/boolobject.rs index c0c4e4ba..01bc14b4 100644 --- a/src/types/boolobject.rs +++ b/src/types/boolobject.rs @@ -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; diff --git a/src/types/mod.rs b/src/types/mod.rs index da22b307..46909c88 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -309,4 +309,4 @@ mod slice; pub(crate) mod string; pub(crate) mod traceback; pub(crate) mod tuple; -mod typeobject; +pub(crate) mod typeobject; diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 50eeaa71..f0e4a8c0 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -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; @@ -11,38 +13,143 @@ 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(py: Python<'_>) -> &PyType { T::type_object_bound(py).into_gil_ref() } + /// Creates a new type object. + #[inline] + pub fn new_bound(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_borrowed_type_ptr`]. /// /// # 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 = "Use `PyType::from_borrowed_type_ptr` instead" + ) + )] pub unsafe fn from_type_ptr(py: Python<'_>, p: *mut ffi::PyTypeObject) -> &PyType { - py.from_borrowed_ptr(p as *mut ffi::PyObject) + Self::from_borrowed_type_ptr(py, p).into_gil_ref() + } + + /// Converts the given FFI pointer into `Bound`, to use in safe code. + /// + /// The function creates a new reference from the given pointer, and returns + /// it as a `Bound`. + /// + /// # Safety + /// - The pointer must be a valid non-null reference to a `PyTypeObject` + #[inline] + pub unsafe fn from_borrowed_type_ptr( + py: Python<'_>, + p: *mut ffi::PyTypeObject, + ) -> Bound<'_, 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 { + self.as_borrowed().qualname() + } + + /// Gets the full name, which includes the module, of the `PyType`. + pub fn name(&self) -> PyResult> { + 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 { + 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(&self) -> PyResult + where + T: PyTypeInfo, + { + self.as_borrowed().is_subclass_of::() + } +} + +/// 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>; + + /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. + fn qualname(&self) -> PyResult; + + /// Checks whether `self` is a subclass of `other`. + /// + /// Equivalent to the Python expression `issubclass(self, other)`. + fn is_subclass(&self, other: &Bound<'_, PyAny>) -> PyResult; + + /// 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(&self) -> PyResult + 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> { + Borrowed::from(self).name() + } + + fn qualname(&self) -> PyResult { #[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())? }; @@ -53,8 +160,29 @@ impl PyType { name } - /// Gets the full name, which includes the module, of the `PyType`. - pub fn name(&self) -> PyResult> { + /// Checks whether `self` is a subclass of `other`. + /// + /// Equivalent to the Python expression `issubclass(self, other)`. + fn is_subclass(&self, other: &Bound<'_, PyAny>) -> PyResult { + 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(&self) -> PyResult + where + T: PyTypeInfo, + { + self.is_subclass(&T::type_object_bound(self.py())) + } +} + +impl<'a> Borrowed<'a, '_, PyType> { + fn name(self) -> PyResult> { #[cfg(not(any(Py_LIMITED_API, PyPy)))] { let ptr = self.as_type_ptr(); @@ -79,33 +207,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 { - 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(&self) -> PyResult - where - T: PyTypeInfo, - { - self.is_subclass(T::type_object_bound(self.py()).as_gil_ref()) - } } #[cfg(test)] diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index d19d106d..1c2b2a5c 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -230,7 +230,7 @@ impl UnsendableChild { fn test_unsendable() -> PyResult<()> { let obj = Python::with_gil(|py| -> PyResult<_> { - let obj: Py = PyType::new::(py).call1((5,))?.extract()?; + let obj: Py = PyType::new_bound::(py).call1((5,))?.extract()?; // Accessing the value inside this thread should not panic let caught_panic =