Merge pull request #3600 from davidhewitt/pytypecheck

replace `PyTryFrom` by splitting `PyTypeInfo`

Author: davidhewitt

guide/src/class.md

@@ -1108,8 +1108,10 @@ struct MyClass {
     # #[allow(dead_code)]
     num: i32,
 }
-unsafe impl pyo3::type_object::PyTypeInfo for MyClass {
+unsafe impl pyo3::type_object::HasPyGilRef for MyClass {
     type AsRefTarget = pyo3::PyCell<Self>;
+}
+unsafe impl pyo3::type_object::PyTypeInfo for MyClass {
     const NAME: &'static str = "MyClass";
     const MODULE: ::std::option::Option<&'static str> = ::std::option::Option::None;
     #[inline]

guide/src/migration.md

@@ -5,6 +5,42 @@ For a detailed list of all changes, see the [CHANGELOG](changelog.md).
 
 ## from 0.20.* to 0.21
 
+### `PyTypeInfo` and `PyTryFrom` have been adjusted
+
+The `PyTryFrom` trait has aged poorly, its [`try_from`] method now conflicts with `try_from` in the 2021 edition prelude. A lot of its functionality was also duplicated with `PyTypeInfo`.
+
+To tighten up the PyO3 traits ahead of [a proposed upcoming API change](https://github.com/PyO3/pyo3/issues/3382) the `PyTypeInfo` trait has had a simpler companion `PyTypeCheck`. The methods [`PyAny::downcast`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.downcast) and [`PyAny::downcast_exact`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html#method.downcast_exact) no longer use `PyTryFrom` as a bound, instead using `PyTypeCheck` and `PyTypeInfo` respectively.
+
+To migrate, switch all type casts to use `obj.downcast()` instead of `try_from(obj)` (and similar for `downcast_exact`).
+
+Before:
+
+```rust
+# use pyo3::prelude::*;
+# use pyo3::types::{PyInt, PyList};
+# fn main() -> PyResult<()> {
+Python::with_gil(|py| {
+    let list = PyList::new(py, 0..5);
+    let b = <PyInt as PyTryFrom>::try_from(list.get_item(0).unwrap())?;
+    Ok(())
+})
+# }
+```
+
+After:
+
+```rust
+# use pyo3::prelude::*;
+# use pyo3::types::{PyInt, PyList};
+# fn main() -> PyResult<()> {
+Python::with_gil(|py| {
+    let list = PyList::new(py, 0..5);
+    let b = list.get_item(0).unwrap().downcast::<PyInt>()?;
+    Ok(())
+})
+# }
+```
+
 ### `py.None()`, `py.NotImplemented()` and `py.Ellipsis()` now return typed singletons
 
 Previously `py.None()`, `py.NotImplemented()` and `py.Ellipsis()` would return `PyObject`. This had a few downsides:

newsfragments/3600.changed.md

@@ -0,0 +1 @@
+Split some `PyTypeInfo` functionality into new traits `HasPyGilRef` and `PyTypeCheck`.

pyo3-macros-backend/src/pyclass.rs

@@ -741,9 +741,11 @@ fn impl_pytypeinfo(
     };
 
     quote! {
-        unsafe impl _pyo3::type_object::PyTypeInfo for #cls {
+        unsafe impl _pyo3::type_object::HasPyGilRef for #cls {
             type AsRefTarget = _pyo3::PyCell<Self>;
+        }
 
+        unsafe impl _pyo3::type_object::PyTypeInfo for #cls {
             const NAME: &'static str = #cls_name;
             const MODULE: ::std::option::Option<&'static str> = #module;
 

src/conversion.rs

@@ -304,7 +304,7 @@ where
     T: PyClass,
 {
     fn extract(obj: &'a PyAny) -> PyResult<Self> {
-        PyTryFrom::try_from(obj).map_err(Into::into)
+        obj.downcast().map_err(Into::into)
     }
 }
 
@@ -313,7 +313,7 @@ where
     T: PyClass + Clone,
 {
     fn extract(obj: &'a PyAny) -> PyResult<Self> {
-        let cell: &PyCell<Self> = PyTryFrom::try_from(obj)?;
+        let cell: &PyCell<Self> = obj.downcast()?;
         Ok(unsafe { cell.try_borrow_unguarded()?.clone() })
     }
 }
@@ -323,7 +323,7 @@ where
     T: PyClass,
 {
     fn extract(obj: &'a PyAny) -> PyResult<Self> {
-        let cell: &PyCell<T> = PyTryFrom::try_from(obj)?;
+        let cell: &PyCell<T> = obj.downcast()?;
         cell.try_borrow().map_err(Into::into)
     }
 }
@@ -333,7 +333,7 @@ where
     T: PyClass<Frozen = False>,
 {
     fn extract(obj: &'a PyAny) -> PyResult<Self> {
-        let cell: &PyCell<T> = PyTryFrom::try_from(obj)?;
+        let cell: &PyCell<T> = obj.downcast()?;
         cell.try_borrow_mut().map_err(Into::into)
     }
 }
@@ -381,78 +381,61 @@ pub trait PyTryInto<T>: Sized {
     fn try_into_exact(&self) -> Result<&T, PyDowncastError<'_>>;
 }
 
-// TryFrom implies TryInto
-impl<U> PyTryInto<U> for PyAny
-where
-    U: for<'v> PyTryFrom<'v>,
-{
-    fn try_into(&self) -> Result<&U, PyDowncastError<'_>> {
-        <U as PyTryFrom<'_>>::try_from(self)
-    }
-    fn try_into_exact(&self) -> Result<&U, PyDowncastError<'_>> {
-        U::try_from_exact(self)
-    }
-}
+mod implementations {
+    use super::*;
 
-impl<'v, T> PyTryFrom<'v> for T
-where
-    T: PyTypeInfo + PyNativeType,
-{
-    fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
-        let value = value.into();
-        unsafe {
-            if T::is_type_of(value) {
-                Ok(Self::try_from_unchecked(value))
-            } else {
-                Err(PyDowncastError::new(value, T::NAME))
-            }
+    // TryFrom implies TryInto
+    impl<U> PyTryInto<U> for PyAny
+    where
+        U: for<'v> PyTryFrom<'v>,
+    {
+        fn try_into(&self) -> Result<&U, PyDowncastError<'_>> {
+            <U as PyTryFrom<'_>>::try_from(self)
+        }
+        fn try_into_exact(&self) -> Result<&U, PyDowncastError<'_>> {
+            U::try_from_exact(self)
         }
     }
 
-    fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
-        let value = value.into();
-        unsafe {
-            if T::is_exact_type_of(value) {
-                Ok(Self::try_from_unchecked(value))
-            } else {
-                Err(PyDowncastError::new(value, T::NAME))
-            }
+    impl<'v, T> PyTryFrom<'v> for T
+    where
+        T: PyTypeInfo<AsRefTarget = Self> + PyNativeType,
+    {
+        fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
+            value.into().downcast()
         }
-    }
 
-    #[inline]
-    unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self {
-        Self::unchecked_downcast(value.into())
-    }
-}
+        fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
+            value.into().downcast_exact()
+        }
 
-impl<'v, T> PyTryFrom<'v> for PyCell<T>
-where
-    T: 'v + PyClass,
-{
-    fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
-        let value = value.into();
-        unsafe {
-            if T::is_type_of(value) {
-                Ok(Self::try_from_unchecked(value))
-            } else {
-                Err(PyDowncastError::new(value, T::NAME))
-            }
+        #[inline]
+        unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self {
+            value.into().downcast_unchecked()
         }
     }
-    fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
-        let value = value.into();
-        unsafe {
-            if T::is_exact_type_of(value) {
-                Ok(Self::try_from_unchecked(value))
-            } else {
-                Err(PyDowncastError::new(value, T::NAME))
+
+    impl<'v, T> PyTryFrom<'v> for PyCell<T>
+    where
+        T: 'v + PyClass,
+    {
+        fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
+            value.into().downcast()
+        }
+        fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> {
+            let value = value.into();
+            unsafe {
+                if T::is_exact_type_of(value) {
+                    Ok(Self::try_from_unchecked(value))
+                } else {
+                    Err(PyDowncastError::new(value, T::NAME))
+                }
             }
         }
-    }
-    #[inline]
-    unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self {
-        Self::unchecked_downcast(value.into())
+        #[inline]
+        unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self {
+            value.into().downcast_unchecked()
+        }
     }
 }
 
@@ -572,10 +555,11 @@ mod test_no_clone {}
 
 #[cfg(test)]
 mod tests {
-    use crate::types::{IntoPyDict, PyAny, PyDict, PyList};
-    use crate::{PyObject, Python, ToPyObject};
+    use crate::PyObject;
 
-    use super::PyTryFrom;
+    use super::super::PyTryFrom;
+    use crate::types::{IntoPyDict, PyAny, PyDict, PyList};
+    use crate::{Python, ToPyObject};
 
     #[test]
     fn test_try_from() {

src/instance.rs

@@ -1,14 +1,14 @@
-use crate::conversion::PyTryFrom;
 use crate::err::{self, PyDowncastError, PyErr, PyResult};
-use crate::gil;
 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::{PyDict, PyString, PyTuple};
 use crate::{
     ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer, PyRef, PyRefMut,
     PyTypeInfo, Python, ToPyObject,
 };
+use crate::{gil, PyTypeCheck};
 use std::marker::PhantomData;
 use std::mem::{self, ManuallyDrop};
 use std::ops::Deref;
@@ -185,7 +185,7 @@ impl<'py, T> Py2<'py, T> {
     #[doc(hidden)] // public and doc(hidden) to use in examples and tests for now
     pub fn as_gil_ref(&'py self) -> &'py T::AsRefTarget
     where
-        T: PyTypeInfo,
+        T: HasPyGilRef,
     {
         unsafe { self.py().from_borrowed_ptr(self.as_ptr()) }
     }
@@ -194,7 +194,7 @@ impl<'py, T> Py2<'py, T> {
     #[doc(hidden)] // public but hidden, to use for tests for now
     pub fn into_gil_ref(self) -> &'py T::AsRefTarget
     where
-        T: PyTypeInfo,
+        T: HasPyGilRef,
     {
         unsafe { self.py().from_owned_ptr(self.into_ptr()) }
     }
@@ -437,7 +437,7 @@ where
 
 impl<T> Py<T>
 where
-    T: PyTypeInfo,
+    T: HasPyGilRef,
 {
     /// Borrows a GIL-bound reference to the contained `T`.
     ///
@@ -1314,11 +1314,11 @@ impl PyObject {
     /// # }
     /// ```
     #[inline]
-    pub fn downcast<'p, T>(&'p self, py: Python<'p>) -> Result<&T, PyDowncastError<'_>>
+    pub fn downcast<'py, T>(&'py self, py: Python<'py>) -> Result<&'py T, PyDowncastError<'py>>
     where
-        T: PyTryFrom<'p>,
+        T: PyTypeCheck<AsRefTarget = T>,
     {
-        <T as PyTryFrom<'_>>::try_from(self.as_ref(py))
+        self.as_ref(py).downcast()
     }
 
     /// Casts the PyObject to a concrete Python object type without checking validity.
@@ -1329,9 +1329,9 @@ impl PyObject {
     #[inline]
     pub unsafe fn downcast_unchecked<'p, T>(&'p self, py: Python<'p>) -> &T
     where
-        T: PyTryFrom<'p>,
+        T: HasPyGilRef<AsRefTarget = T>,
     {
-        <T as PyTryFrom<'_>>::try_from_unchecked(self.as_ref(py))
+        self.as_ref(py).downcast_unchecked()
     }
 }
 
@@ -1509,6 +1509,8 @@ a = A()
 
     #[cfg(feature = "macros")]
     mod using_macros {
+        use crate::{PyCell, PyTryInto};
+
         use super::*;
 
         #[crate::pyclass]
@@ -1533,5 +1535,15 @@ a = A()
                 assert_eq!(instance.try_borrow_mut(py).unwrap().0, 123);
             })
         }
+
+        #[test]
+        fn cell_tryfrom() {
+            // More detailed tests of the underlying semantics in pycell.rs
+            Python::with_gil(|py| {
+                let instance: &PyAny = Py::new(py, SomeClass(0)).unwrap().into_ref(py);
+                let _: &PyCell<SomeClass> = PyTryInto::try_into(instance).unwrap();
+                let _: &PyCell<SomeClass> = PyTryInto::try_into_exact(instance).unwrap();
+            })
+        }
     }
 }

src/lib.rs

@@ -294,9 +294,8 @@
 //! [Features chapter of the guide]: https://pyo3.rs/latest/features.html#features-reference "Features Reference - PyO3 user guide"
 //! [`Ungil`]: crate::marker::Ungil
 pub use crate::class::*;
-pub use crate::conversion::{
-    AsPyPointer, FromPyObject, FromPyPointer, IntoPy, PyTryFrom, PyTryInto, ToPyObject,
-};
+pub use crate::conversion::{AsPyPointer, FromPyObject, FromPyPointer, IntoPy, ToPyObject};
+pub use crate::conversion::{PyTryFrom, PyTryInto};
 pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult};
 pub use crate::gil::GILPool;
 #[cfg(not(PyPy))]
@@ -306,7 +305,7 @@ pub use crate::marker::Python;
 pub use crate::pycell::{PyCell, PyRef, PyRefMut};
 pub use crate::pyclass::PyClass;
 pub use crate::pyclass_init::PyClassInitializer;
-pub use crate::type_object::PyTypeInfo;
+pub use crate::type_object::{PyTypeCheck, PyTypeInfo};
 pub use crate::types::PyAny;
 pub use crate::version::PythonVersionInfo;