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 <[email protected]> * Rework as `PyType::from_borrowed_type_ptr` * correct doc link to `from_borrowed_type_ptr` Co-authored-by: Lily Foote <[email protected]> * remove unneeded lifetime name --------- Co-authored-by: Icxolu <[email protected]> Co-authored-by: Lily Foote <[email protected]>
PyO3 · Feb 18, 2024 · f04ad56 · f04ad56
1 parent 1d295a1
commit f04ad56
Show file tree

Hide file tree

Showing 9 changed files with 155 additions and 49 deletions.
diff --git 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
@@ -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::<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> {

diff --git 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
@@ -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
@@ -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<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;
@@ -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<Bound<'py, PySuper>> {
-        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<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 {

diff --git 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
@@ -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
@@ -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<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_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<PyType>`, to use in safe code.
+    ///
+    /// The function creates a new reference from the given pointer, and returns
+    /// it as a `Bound<PyType>`.
+    ///
+    /// # 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<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())?
             };
@@ -53,8 +160,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();
@@ -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<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)]

diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs
@@ -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 =