From a3eb328378e17d7fa79f0876393269dd4e511ab8 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Mon, 29 Jan 2024 13:31:04 +0000 Subject: [PATCH 1/2] migrate `FromPyObject` for `Bound` and `Py` to `extract_bound` --- newsfragments/3776.changed.md | 1 + src/instance.rs | 19 +++++++------------ 2 files changed, 8 insertions(+), 12 deletions(-) create mode 100644 newsfragments/3776.changed.md diff --git a/newsfragments/3776.changed.md b/newsfragments/3776.changed.md new file mode 100644 index 00000000..71ffd893 --- /dev/null +++ b/newsfragments/3776.changed.md @@ -0,0 +1 @@ +Relax bound of `FromPyObject` for `Py` to just `T: PyTypeCheck`. diff --git a/src/instance.rs b/src/instance.rs index d7edfe06..e29f4635 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1405,27 +1405,22 @@ impl Drop for Py { } } -impl<'a, T> FromPyObject<'a> for Py +impl FromPyObject<'_> for Py 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 { - unsafe { - ob.extract::<&T::AsRefTarget>() - .map(|val| Py::from_borrowed_ptr(ob.py(), val.as_ptr())) - } + fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult { + ob.extract::>().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 { + fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult { ob.downcast().map(Clone::clone).map_err(Into::into) } } From a60c1821afb80f7775c2067b6210ceb765dfd1fb Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Mon, 29 Jan 2024 13:31:21 +0000 Subject: [PATCH 2/2] implement `PyFunctionArgument` for `&Bound` --- guide/src/migration.md | 1 + src/impl_/extract_argument.rs | 14 +++++++++++++- tests/test_pyfunction.rs | 17 +++++++++++++++++ tests/ui/invalid_argument_attributes.stderr | 1 + 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/guide/src/migration.md b/guide/src/migration.md index 9fb5adba..0174d0dc 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -247,6 +247,7 @@ Because the new `Bound` 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` to pass these types around (or use `.clone()` at the very small cost of increasing the Python reference count) - `Bound` and `Bound` cannot support indexing with `list[0]`, you should use `list.get_item(0)` instead. - `Bound::iter_borrowed` is slightly more efficient than `Bound::iter`. The default iteration of `Bound` cannot return borrowed references because Rust does not (yet) have "lending iterators". Similarly `Bound::get_borrowed_item` is more efficient than `Bound::get_item` for the same reason. +- `&Bound` 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::()` instead of `bound_any.extract::<&Bound>()`. #### Migrating `FromPyObject` implementations diff --git a/src/impl_/extract_argument.rs b/src/impl_/extract_argument.rs index a18c8d4b..cd63fe27 100644 --- a/src/impl_/extract_argument.rs +++ b/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>; + + #[inline] + fn extract(obj: &'py PyAny, holder: &'a mut Option>) -> PyResult { + 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 { diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index a2f44be6..f1116363 100644 --- a/tests/test_pyfunction.rs +++ b/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> { + 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"); + }) +} diff --git a/tests/ui/invalid_argument_attributes.stderr b/tests/ui/invalid_argument_attributes.stderr index ef8b59a8..c122dd25 100644 --- a/tests/ui/invalid_argument_attributes.stderr +++ b/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<'_>`