Merge pull request #3776 from davidhewitt/bound-extract

migrate `FromPyObject` for `Bound` and `Py` to new APIs

Author: davidhewitt

guide/src/migration.md

@@ -248,6 +248,7 @@ Because the new `Bound<T>` API brings ownership out of the PyO3 framework and in
 - 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)
 - `Bound<PyList>` and `Bound<PyTuple>` cannot support indexing with `list[0]`, you should use `list.get_item(0)` instead.
 - `Bound<PyTuple>::iter_borrowed` is slightly more efficient than `Bound<PyTuple>::iter`. The default iteration of `Bound<PyTuple>` cannot return borrowed references because Rust does not (yet) have "lending iterators". Similarly `Bound<PyTuple>::get_borrowed_item` is more efficient than `Bound<PyTuple>::get_item` for the same reason.
+- `&Bound<T>` does not implement `FromPyObject` (although it might be possible to do this in the future once the GIL Refs API is completely removed). Use `bound_any.downcast::<T>()` instead of `bound_any.extract::<&Bound<T>>()`.
 
 #### Migrating `FromPyObject` implementations
 

newsfragments/3776.changed.md

@@ -0,0 +1 @@
+Relax bound of `FromPyObject` for `Py<T>` to just `T: PyTypeCheck`.

src/impl_/extract_argument.rs

@@ -3,7 +3,7 @@ use crate::{
     ffi,
     pyclass::boolean_struct::False,
     types::{PyDict, PyString, PyTuple},
-    FromPyObject, PyAny, PyClass, PyErr, PyRef, PyRefMut, PyResult, Python,
+    Bound, FromPyObject, PyAny, PyClass, PyErr, PyRef, PyRefMut, PyResult, PyTypeCheck, Python,
 };
 
 /// A trait which is used to help PyO3 macros extract function arguments.
@@ -31,6 +31,18 @@ where
     }
 }
 
+impl<'a, 'py, T> PyFunctionArgument<'a, 'py> for &'a Bound<'py, T>
+where
+    T: PyTypeCheck,
+{
+    type Holder = Option<Bound<'py, T>>;
+
+    #[inline]
+    fn extract(obj: &'py PyAny, holder: &'a mut Option<Bound<'py, T>>) -> PyResult<Self> {
+        Ok(&*holder.insert(obj.extract()?))
+    }
+}
+
 /// Trait for types which can be a function argument holder - they should
 /// to be able to const-initialize to an empty value.
 pub trait FunctionArgumentHolder: Sized {

src/instance.rs

@@ -1419,27 +1419,22 @@ impl<T> Drop for Py<T> {
     }
 }
 
-impl<'a, T> FromPyObject<'a> for Py<T>
+impl<T> FromPyObject<'_> for Py<T>
 where
-    T: PyTypeInfo,
-    &'a T::AsRefTarget: FromPyObject<'a>,
-    T::AsRefTarget: 'a + AsPyPointer,
+    T: PyTypeCheck,
 {
     /// Extracts `Self` from the source `PyObject`.
-    fn extract(ob: &'a PyAny) -> PyResult<Self> {
-        unsafe {
-            ob.extract::<&T::AsRefTarget>()
-                .map(|val| Py::from_borrowed_ptr(ob.py(), val.as_ptr()))
-        }
+    fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<Self> {
+        ob.extract::<Bound<'_, T>>().map(Bound::unbind)
     }
 }
 
-impl<'a, T> FromPyObject<'a> for Bound<'a, T>
+impl<'py, T> FromPyObject<'py> for Bound<'py, T>
 where
-    T: PyTypeInfo,
+    T: PyTypeCheck,
 {
     /// Extracts `Self` from the source `PyObject`.
-    fn extract_bound(ob: &Bound<'a, PyAny>) -> PyResult<Self> {
+    fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
         ob.downcast().map(Clone::clone).map_err(Into::into)
     }
 }

tests/test_pyfunction.rs

@@ -529,3 +529,20 @@ fn test_some_wrap_arguments() {
         py_assert!(py, function, "function() == [1, 2, None, None]");
     })
 }
+
+#[test]
+fn test_reference_to_bound_arguments() {
+    #[pyfunction]
+    fn reference_args<'py>(
+        x: &Bound<'py, PyAny>,
+        y: Option<&Bound<'py, PyAny>>,
+    ) -> PyResult<Bound<'py, PyAny>> {
+        y.map_or_else(|| Ok(x.clone()), |y| y.add(x))
+    }
+
+    Python::with_gil(|py| {
+        let function = wrap_pyfunction!(reference_args, py).unwrap();
+        py_assert!(py, function, "function(1) == 1");
+        py_assert!(py, function, "function(1, 2) == 3");
+    })
+}

tests/ui/invalid_argument_attributes.stderr

@@ -93,6 +93,7 @@ error[E0277]: the trait bound `CancelHandle: Clone` is not satisfied
    |                                                  ^^^^ the trait `Clone` is not implemented for `CancelHandle`
    |
    = help: the following other types implement trait `PyFunctionArgument<'a, 'py>`:
+             &'a pyo3::Bound<'py, T>
              &'a pyo3::coroutine::Coroutine
              &'a mut pyo3::coroutine::Coroutine
    = note: required for `CancelHandle` to implement `FromPyObject<'_>`