From fdf407e045419bb3c316e1149877a75f0ddab70d Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Mon, 13 Jan 2020 22:56:16 +0000 Subject: [PATCH] FromPyObject for #[pyclass] with T: Clone --- CHANGELOG.md | 1 + pyo3-derive-backend/src/pyclass.rs | 12 +++++ src/conversion.rs | 81 +++++++++++++++++++++++++----- src/lib.rs | 2 +- src/types/mod.rs | 4 ++ tests/test_class_conversion.rs | 25 +++++++++ tests/test_compile_error.rs | 3 +- tests/ui/missing_clone.rs | 16 ++++++ tests/ui/missing_clone.stderr | 8 +++ 9 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 tests/test_class_conversion.rs create mode 100644 tests/ui/missing_clone.rs create mode 100644 tests/ui/missing_clone.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 087624aea8c..5495829096b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added +* `FromPyObject` is now automatically implemented for `T: Clone` pyclasses. [#730](https://github.com/PyO3/pyo3/pull/730) * Implemented `IntoIterator` for `PySet` and `PyFrozenSet`. [#716](https://github.com/PyO3/pyo3/pull/716) * `PyClass`, `PyClassShell`, `PyObjectLayout`, `PyClassInitializer` [#683](https://github.com/PyO3/pyo3/pull/683) diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs index 5c845bb8f5e..e70694ebca6 100644 --- a/pyo3-derive-backend/src/pyclass.rs +++ b/pyo3-derive-backend/src/pyclass.rs @@ -395,6 +395,18 @@ fn impl_class( #weakref } + impl pyo3::conversion::FromPyObjectImpl for #cls { + type Impl = pyo3::conversion::extract_impl::Cloned; + } + + impl pyo3::conversion::FromPyObjectImpl for &'_ #cls { + type Impl = pyo3::conversion::extract_impl::Reference; + } + + impl pyo3::conversion::FromPyObjectImpl for &'_ mut #cls { + type Impl = pyo3::conversion::extract_impl::MutReference; + } + #into_pyobject #inventory_impl diff --git a/src/conversion.rs b/src/conversion.rs index d24e9f48ab5..7a9259ec398 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -244,25 +244,80 @@ where } } -/// Extract reference to instance from `PyObject` -impl<'a, T> FromPyObject<'a> for &'a T -where - T: PyTryFrom<'a>, -{ - #[inline] - fn extract(ob: &'a PyAny) -> PyResult<&'a T> { - Ok(T::try_from(ob)?) +#[doc(hidden)] +pub mod extract_impl { + use super::*; + + pub trait ExtractImpl<'a, Target>: Sized { + fn extract(source: &'a PyAny) -> PyResult; + } + + pub struct Cloned; + pub struct Reference; + pub struct MutReference; + + impl<'a, T: 'a> ExtractImpl<'a, T> for Cloned + where + T: Clone, + Reference: ExtractImpl<'a, &'a T>, + { + fn extract(source: &'a PyAny) -> PyResult { + Ok(Reference::extract(source)?.clone()) + } + } + + impl<'a, T> ExtractImpl<'a, &'a T> for Reference + where + T: PyTryFrom<'a>, + { + fn extract(source: &'a PyAny) -> PyResult<&'a T> { + Ok(T::try_from(source)?) + } + } + + impl<'a, T> ExtractImpl<'a, &'a mut T> for MutReference + where + T: PyTryFrom<'a>, + { + fn extract(source: &'a PyAny) -> PyResult<&'a mut T> { + Ok(T::try_from_mut(source)?) + } } } -/// Extract mutable reference to instance from `PyObject` -impl<'a, T> FromPyObject<'a> for &'a mut T +use extract_impl::ExtractImpl; + +/// This is an internal trait for re-using `FromPyObject` implementations for many pyo3 types. +/// +/// Users should implement `FromPyObject` directly instead of via this trait. +pub trait FromPyObjectImpl { + // Implement this trait with to specify the implementor of `extract_impl::ExtractImpl` to use for + // extracting this type from Python objects. + // + // Example valid implementations are `extract_impl::Cloned`, `extract_impl::Reference`, and + // `extract_impl::MutReference`, which are for extracting `T`, `&T` and `&mut T` respectively via + // PyTryFrom. + // + // We deliberately don't require Impl: ExtractImpl here because we allow #[pyclass] + // to specify an Impl which doesn't satisfy the ExtractImpl constraints. + // + // e.g. non-clone #[pyclass] can still have Impl: Cloned. + // + // We catch invalid Impls in the blanket impl for FromPyObject, which only + // complains when .extract() is actually used. + + /// The type which implements `ExtractImpl`. + type Impl; +} + +impl<'a, T> FromPyObject<'a> for T where - T: PyTryFrom<'a>, + T: FromPyObjectImpl, + ::Impl: ExtractImpl<'a, Self>, { #[inline] - fn extract(ob: &'a PyAny) -> PyResult<&'a mut T> { - Ok(T::try_from_mut(ob)?) + fn extract(ob: &'a PyAny) -> PyResult { + ::Impl::extract(ob) } } diff --git a/src/lib.rs b/src/lib.rs index c670c20c986..14126a5f4de 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -152,7 +152,7 @@ pub mod buffer; #[doc(hidden)] pub mod callback; pub mod class; -mod conversion; +pub mod conversion; #[doc(hidden)] pub mod derive_utils; mod err; diff --git a/src/types/mod.rs b/src/types/mod.rs index 6677a1b9baf..3616b718d2d 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -142,6 +142,10 @@ macro_rules! pyobject_native_type_convert( } } + impl<$($type_param,)*> $crate::conversion::FromPyObjectImpl for &'_ $name { + type Impl = $crate::conversion::extract_impl::Reference; + } + impl<$($type_param,)*> ::std::fmt::Debug for $name { fn fmt(&self, f: &mut ::std::fmt::Formatter) -> Result<(), ::std::fmt::Error> diff --git a/tests/test_class_conversion.rs b/tests/test_class_conversion.rs new file mode 100644 index 00000000000..ee3678ddd48 --- /dev/null +++ b/tests/test_class_conversion.rs @@ -0,0 +1,25 @@ +use pyo3::prelude::*; + +#[pyclass] +#[derive(Clone, Debug, PartialEq)] +struct Cloneable { + x: i32, +} + +#[test] +fn test_cloneable_pyclass() { + let c = Cloneable { x: 10 }; + + let gil = Python::acquire_gil(); + let py = gil.python(); + + let py_c = Py::new(py, c.clone()).unwrap().to_object(py); + + let c2: Cloneable = py_c.extract(py).unwrap(); + let rc: &Cloneable = py_c.extract(py).unwrap(); + let mrc: &mut Cloneable = py_c.extract(py).unwrap(); + + assert_eq!(c, c2); + assert_eq!(&c, rc); + assert_eq!(&c, mrc); +} diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 634b4932c40..661176c0e40 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -1,7 +1,8 @@ #[test] fn test_compile_errors() { let t = trybuild::TestCases::new(); + t.compile_fail("tests/ui/invalid_pymethod_names.rs"); + t.compile_fail("tests/ui/missing_clone.rs"); t.compile_fail("tests/ui/reject_generics.rs"); t.compile_fail("tests/ui/too_many_args_to_getter.rs"); - t.compile_fail("tests/ui/invalid_pymethod_names.rs"); } diff --git a/tests/ui/missing_clone.rs b/tests/ui/missing_clone.rs new file mode 100644 index 00000000000..d577bdfcdf9 --- /dev/null +++ b/tests/ui/missing_clone.rs @@ -0,0 +1,16 @@ +use pyo3::prelude::*; + +#[pyclass] +struct TestClass { + num: u32, +} + +fn main() { + let t = TestClass { num: 10 }; + + let gil = Python::acquire_gil(); + let py = gil.python(); + + let pyvalue = Py::new(py, t).unwrap().to_object(py); + let t: TestClass = pyvalue.extract(py).unwrap(); +} diff --git a/tests/ui/missing_clone.stderr b/tests/ui/missing_clone.stderr new file mode 100644 index 00000000000..54452e43bc2 --- /dev/null +++ b/tests/ui/missing_clone.stderr @@ -0,0 +1,8 @@ +error[E0277]: the trait bound `TestClass: std::clone::Clone` is not satisfied + --> $DIR/missing_clone.rs:15:32 + | +15 | let t: TestClass = pyvalue.extract(py).unwrap(); + | ^^^^^^^ the trait `std::clone::Clone` is not implemented for `TestClass` + | + = note: required because of the requirements on the impl of `pyo3::conversion::extract_impl::ExtractImpl<'_, TestClass>` for `pyo3::conversion::extract_impl::Cloned` + = note: required because of the requirements on the impl of `pyo3::conversion::FromPyObject<'_>` for `TestClass`