Merge pull request #3782 from davidhewitt/type-check-bound

add `bound` method variants for `PyTypeInfo`

guide/src/class/numeric.md

@@ -388,7 +388,7 @@ fn my_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
 # fn main() -> PyResult<()> {
 #     Python::with_gil(|py| -> PyResult<()> {
 #         let globals = PyModule::import(py, "__main__")?.dict();
-#         globals.set_item("Number", Number::type_object(py))?;
+#         globals.set_item("Number", Number::type_object_bound(py))?;
 #
 #         py.run(SCRIPT, Some(globals), None)?;
 #         Ok(())

guide/src/migration.md

@@ -210,6 +210,7 @@ To minimise breakage of code using the GIL-Refs API, the `Bound<T>` smart pointe
 For example, the following APIs have gained updated variants:
 - `PyList::new`, `PyTyple::new` and similar constructors have replacements `PyList::new_bound`, `PyTuple::new_bound` etc.
 - `FromPyObject::extract` has a new `FromPyObject::extract_bound` (see the section below)
+- The `PyTypeInfo` trait has had new `_bound` methods added to accept / return `Bound<T>`.
 
 Because the new `Bound<T>` API brings ownership out of the PyO3 framework and into user code, there are a few places where user code is expected to need to adjust while switching to the new API:
 - Code will need to add the occasional `&` to borrow the new smart pointer as `&Bound<T>` to pass these types around (or use `.clone()` at the very small cost of increasing the Python reference count)
@@ -245,6 +246,8 @@ impl<'py> FromPyObject<'py> for MyType {
 
 The expectation is that in 0.22 `extract_bound` will have the default implementation removed and in 0.23 `extract` will be removed.
 
+
+
 ## from 0.19.* to 0.20
 
 ### Drop support for older technologies
@@ -656,7 +659,7 @@ To migrate, update trait bounds and imports from `PyTypeObject` to `PyTypeInfo`.
 
 Before:
 
-```rust,compile_fail
+```rust,ignore
 use pyo3::Python;
 use pyo3::type_object::PyTypeObject;
 use pyo3::types::PyType;
@@ -668,7 +671,7 @@ fn get_type_object<T: PyTypeObject>(py: Python<'_>) -> &PyType {
 
 After
 
-```rust
+```rust,ignore
 use pyo3::{Python, PyTypeInfo};
 use pyo3::types::PyType;
 
@@ -995,13 +998,13 @@ makes it possible to interact with Python exception objects.
 
 The new types also have names starting with the "Py" prefix. For example, before:
 
-```rust,compile_fail
+```rust,ignore
 let err: PyErr = TypeError::py_err("error message");
 ```
 
 After:
 
-```rust,compile_fail
+```rust,ignore
 # use pyo3::{PyErr, PyResult, Python, type_object::PyTypeObject};
 # use pyo3::exceptions::{PyBaseException, PyTypeError};
 # Python::with_gil(|py| -> PyResult<()> {

src/conversions/chrono_tz.rs

@@ -35,8 +35,7 @@
 //! ```
 use crate::exceptions::PyValueError;
 use crate::sync::GILOnceCell;
-use crate::types::any::PyAnyMethods;
-use crate::types::PyType;
+use crate::types::{any::PyAnyMethods, PyType};
 use crate::{
     intern, Bound, FromPyObject, IntoPy, Py, PyAny, PyObject, PyResult, Python, ToPyObject,
 };
@@ -51,7 +50,7 @@ impl ToPyObject for Tz {
             .unwrap()
             .call1((self.name(),))
             .unwrap()
-            .into()
+            .unbind()
     }
 }
 

src/conversions/std/ipaddr.rs

@@ -36,7 +36,7 @@ impl ToPyObject for Ipv4Addr {
             .expect("failed to load ipaddress.IPv4Address")
             .call1((u32::from_be_bytes(self.octets()),))
             .expect("failed to construct ipaddress.IPv4Address")
-            .to_object(py)
+            .unbind()
     }
 }
 
@@ -48,7 +48,7 @@ impl ToPyObject for Ipv6Addr {
             .expect("failed to load ipaddress.IPv6Address")
             .call1((u128::from_be_bytes(self.octets()),))
             .expect("failed to construct ipaddress.IPv6Address")
-            .to_object(py)
+            .unbind()
     }
 }
 

src/err/mod.rs

@@ -159,7 +159,7 @@ impl PyErr {
     {
         PyErr::from_state(PyErrState::Lazy(Box::new(move |py| {
             PyErrStateLazyFnOutput {
-                ptype: T::type_object(py).into(),
+                ptype: T::type_object_bound(py).into(),
                 pvalue: args.arguments(py),
             }
         })))
@@ -540,7 +540,7 @@ impl PyErr {
     where
         T: PyTypeInfo,
     {
-        self.is_instance(py, T::type_object(py))
+        self.is_instance(py, T::type_object_bound(py).as_gil_ref())
     }
 
     /// Writes the error back to the Python interpreter's global state.
@@ -1077,18 +1077,24 @@ mod tests {
     fn test_pyerr_matches() {
         Python::with_gil(|py| {
             let err = PyErr::new::<PyValueError, _>("foo");
-            assert!(err.matches(py, PyValueError::type_object(py)));
+            assert!(err.matches(py, PyValueError::type_object_bound(py)));
 
             assert!(err.matches(
                 py,
-                (PyValueError::type_object(py), PyTypeError::type_object(py))
+                (
+                    PyValueError::type_object_bound(py),
+                    PyTypeError::type_object_bound(py)
+                )
             ));
 
-            assert!(!err.matches(py, PyTypeError::type_object(py)));
+            assert!(!err.matches(py, PyTypeError::type_object_bound(py)));
 
             // String is not a valid exception class, so we should get a TypeError
-            let err: PyErr = PyErr::from_type(crate::types::PyString::type_object(py), "foo");
-            assert!(err.matches(py, PyTypeError::type_object(py)));
+            let err: PyErr = PyErr::from_type(
+                crate::types::PyString::type_object_bound(py).as_gil_ref(),
+                "foo",
+            );
+            assert!(err.matches(py, PyTypeError::type_object_bound(py)));
         })
     }
 

src/marker.rs

@@ -717,7 +717,7 @@ impl<'py> Python<'py> {
     where
         T: PyTypeInfo,
     {
-        T::type_object(self)
+        T::type_object_bound(self).into_gil_ref()
     }
 
     /// Imports the Python module with the specified name.

src/pycell.rs

@@ -207,7 +207,7 @@ use crate::{
     type_object::get_tp_free,
     PyTypeInfo,
 };
-use crate::{ffi, IntoPy, PyErr, PyNativeType, PyObject, PyResult, PyTypeCheck, Python};
+use crate::{ffi, Bound, IntoPy, PyErr, PyNativeType, PyObject, PyResult, PyTypeCheck, Python};
 use std::cell::UnsafeCell;
 use std::fmt;
 use std::mem::ManuallyDrop;
@@ -553,7 +553,7 @@ where
 {
     const NAME: &'static str = <T as PyTypeCheck>::NAME;
 
-    fn type_check(object: &PyAny) -> bool {
+    fn type_check(object: &Bound<'_, PyAny>) -> bool {
         <T as PyTypeCheck>::type_check(object)
     }
 }

src/sync.rs

@@ -1,5 +1,5 @@
 //! Synchronization mechanisms based on the Python GIL.
-use crate::{instance::Bound, types::PyString, types::PyType, Py, PyResult, PyVisit, Python};
+use crate::{types::PyString, types::PyType, Bound, Py, PyResult, PyVisit, Python};
 use std::cell::UnsafeCell;
 
 /// Value with concurrent access protected by the GIL.
@@ -196,9 +196,9 @@ impl GILOnceCell<Py<PyType>> {
         py: Python<'py>,
         module_name: &str,
         attr_name: &str,
-    ) -> PyResult<&'py PyType> {
+    ) -> PyResult<&Bound<'py, PyType>> {
         self.get_or_try_init(py, || py.import(module_name)?.getattr(attr_name)?.extract())
-            .map(|ty| ty.as_ref(py))
+            .map(|ty| ty.bind(py))
     }
 }
 

src/tests/common.rs

@@ -138,11 +138,11 @@ mod inner {
         ($py:expr, $body:expr, [$(($category:ty, $message:literal)),+] $(,)? ) => {{
             $crate::tests::common::CatchWarnings::enter($py, |w| {
                 $body;
-                let expected_warnings = [$((<$category as $crate::type_object::PyTypeInfo>::type_object($py), $message)),+];
+                let expected_warnings = [$((<$category as $crate::type_object::PyTypeInfo>::type_object_bound($py), $message)),+];
                 assert_eq!(w.len(), expected_warnings.len());
                 for (warning, (category, message)) in w.iter().zip(expected_warnings) {
 
-                    assert!(warning.getattr("category").unwrap().is(category));
+                    assert!(warning.getattr("category").unwrap().is(&category));
                     assert_eq!(
                         warning.getattr("message").unwrap().str().unwrap().to_string_lossy(),
                         message

src/type_object.rs

@@ -1,7 +1,9 @@
 //! Python type object information
 
+use crate::ffi_ptr_ext::FfiPtrExt;
+use crate::types::any::PyAnyMethods;
 use crate::types::{PyAny, PyType};
-use crate::{ffi, PyNativeType, Python};
+use crate::{ffi, Bound, PyNativeType, Python};
 
 /// `T: PyLayout<U>` represents that `T` is a concrete representation of `U` in the Python heap.
 /// E.g., `PyCell` is a concrete representation of all `pyclass`es, and `ffi::PyObject`
@@ -64,19 +66,71 @@ pub unsafe trait PyTypeInfo: Sized + HasPyGilRef {
 
     /// Returns the safe abstraction over the type object.
     #[inline]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PyTypeInfo::type_object` will be replaced by `PyTypeInfo::type_object_bound` in a future PyO3 version"
+        )
+    )]
     fn type_object(py: Python<'_>) -> &PyType {
+        // This isn't implemented in terms of `type_object_bound` because this just borrowed the
+        // object, for legacy reasons.
         unsafe { py.from_borrowed_ptr(Self::type_object_raw(py) as _) }
     }
 
+    /// Returns the safe abstraction over the type object.
+    #[inline]
+    fn type_object_bound(py: Python<'_>) -> Bound<'_, PyType> {
+        // Making the borrowed object `Bound` is necessary for soundness reasons. It's an extreme
+        // edge case, but arbitrary Python code _could_ change the __class__ of an object and cause
+        // the type object to be freed.
+        //
+        // By making `Bound` we assume ownership which is then safe against races.
+        unsafe {
+            Self::type_object_raw(py)
+                .cast::<ffi::PyObject>()
+                .assume_borrowed_unchecked(py)
+                .to_owned()
+                .downcast_into_unchecked()
+        }
+    }
+
     /// Checks if `object` is an instance of this type or a subclass of this type.
     #[inline]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PyTypeInfo::is_type_of` will be replaced by `PyTypeInfo::is_type_of_bound` in a future PyO3 version"
+        )
+    )]
     fn is_type_of(object: &PyAny) -> bool {
+        Self::is_type_of_bound(&object.as_borrowed())
+    }
+
+    /// Checks if `object` is an instance of this type or a subclass of this type.
+    #[inline]
+    fn is_type_of_bound(object: &Bound<'_, PyAny>) -> bool {
         unsafe { ffi::PyObject_TypeCheck(object.as_ptr(), Self::type_object_raw(object.py())) != 0 }
     }
 
     /// Checks if `object` is an instance of this type.
     #[inline]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PyTypeInfo::is_exact_type_of` will be replaced by `PyTypeInfo::is_exact_type_of_bound` in a future PyO3 version"
+        )
+    )]
     fn is_exact_type_of(object: &PyAny) -> bool {
+        Self::is_exact_type_of_bound(&object.as_borrowed())
+    }
+
+    /// Checks if `object` is an instance of this type.
+    #[inline]
+    fn is_exact_type_of_bound(object: &Bound<'_, PyAny>) -> bool {
         unsafe { ffi::Py_TYPE(object.as_ptr()) == Self::type_object_raw(object.py()) }
     }
 }
@@ -89,7 +143,7 @@ pub trait PyTypeCheck: HasPyGilRef {
     /// Checks if `object` is an instance of `Self`, which may include a subtype.
     ///
     /// This should be equivalent to the Python expression `isinstance(object, Self)`.
-    fn type_check(object: &PyAny) -> bool;
+    fn type_check(object: &Bound<'_, PyAny>) -> bool;
 }
 
 impl<T> PyTypeCheck for T
@@ -99,8 +153,8 @@ where
     const NAME: &'static str = <T as PyTypeInfo>::NAME;
 
     #[inline]
-    fn type_check(object: &PyAny) -> bool {
-        <T as PyTypeInfo>::is_type_of(object)
+    fn type_check(object: &Bound<'_, PyAny>) -> bool {
+        T::is_type_of_bound(object)
     }
 }
 

src/types/any.rs

@@ -733,7 +733,7 @@ impl PyAny {
     where
         T: PyTypeCheck<AsRefTarget = T>,
     {
-        if T::type_check(self) {
+        if T::type_check(&self.as_borrowed()) {
             // Safety: type_check is responsible for ensuring that the type is correct
             Ok(unsafe { self.downcast_unchecked() })
         } else {
@@ -776,7 +776,7 @@ impl PyAny {
     where
         T: PyTypeInfo<AsRefTarget = T>,
     {
-        if T::is_exact_type_of(self) {
+        if T::is_exact_type_of_bound(&self.as_borrowed()) {
             // Safety: type_check is responsible for ensuring that the type is correct
             Ok(unsafe { self.downcast_unchecked() })
         } else {
@@ -2100,7 +2100,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
     where
         T: PyTypeCheck,
     {
-        if T::type_check(self.as_gil_ref()) {
+        if T::type_check(self) {
             // Safety: type_check is responsible for ensuring that the type is correct
             Ok(unsafe { self.downcast_unchecked() })
         } else {
@@ -2113,7 +2113,7 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
     where
         T: PyTypeCheck,
     {
-        if T::type_check(self.as_gil_ref()) {
+        if T::type_check(&self) {
             // Safety: type_check is responsible for ensuring that the type is correct
             Ok(unsafe { self.downcast_into_unchecked() })
         } else {
@@ -2218,12 +2218,12 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
 
     #[inline]
     fn is_instance_of<T: PyTypeInfo>(&self) -> bool {
-        T::is_type_of(self.as_gil_ref())
+        T::is_type_of_bound(self)
     }
 
     #[inline]
     fn is_exact_instance_of<T: PyTypeInfo>(&self) -> bool {
-        T::is_exact_type_of(self.as_gil_ref())
+        T::is_exact_type_of_bound(self)
     }
 
     fn contains<V>(&self, value: V) -> PyResult<bool>

src/types/ellipsis.rs

@@ -1,4 +1,7 @@
-use crate::{ffi, ffi_ptr_ext::FfiPtrExt, Borrowed, PyAny, PyTypeInfo, Python};
+use crate::{
+    ffi, ffi_ptr_ext::FfiPtrExt, types::any::PyAnyMethods, Borrowed, Bound, PyAny, PyTypeInfo,
+    Python,
+};
 
 /// Represents the Python `Ellipsis` object.
 #[repr(transparent)]
@@ -38,14 +41,14 @@ unsafe impl PyTypeInfo for PyEllipsis {
     }
 
     #[inline]
-    fn is_type_of(object: &PyAny) -> bool {
+    fn is_type_of_bound(object: &Bound<'_, PyAny>) -> bool {
         // ellipsis is not usable as a base type
-        Self::is_exact_type_of(object)
+        Self::is_exact_type_of_bound(object)
     }
 
     #[inline]
-    fn is_exact_type_of(object: &PyAny) -> bool {
-        object.is(Self::get_bound(object.py()).as_ref())
+    fn is_exact_type_of_bound(object: &Bound<'_, PyAny>) -> bool {
+        object.is(&**Self::get_bound(object.py()))
     }
 }
 
@@ -68,7 +71,7 @@ mod tests {
         Python::with_gil(|py| {
             assert!(PyEllipsis::get_bound(py)
                 .get_type()
-                .is(PyEllipsis::type_object(py)));
+                .is(&PyEllipsis::type_object_bound(py)));
         })
     }