Merge pull request #730 from davidhewitt/extract-clone

FromPyObject for #[pyclass] with T: Clone

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)
 

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

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<Target>;
+    }
+
+    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<T> {
+            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,
+    <T as 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<T> {
+        <T as FromPyObjectImpl>::Impl::extract(ob)
     }
 }
 

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;

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>

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);
+}

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");
 }

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();
+}

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`