FromPyObject for #[pyclass] with T: Clone

CHANGELOG.md

@@ -15,6 +15,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::FromPyObjectImpl for #cls {
+            type Impl = pyo3::extract_impl::Cloned;
+        }
+
+        impl pyo3::FromPyObjectImpl for &'_ #cls {
+            type Impl = pyo3::extract_impl::Reference;
+        }
+
+        impl pyo3::FromPyObjectImpl for &'_ mut #cls {
+            type Impl = pyo3::extract_impl::MutReference;
+        }
+
         #into_pyobject
 
         #inventory_impl

src/conversion.rs

@@ -6,7 +6,7 @@ use crate::object::PyObject;
 use crate::type_object::{PyObjectLayout, PyTypeInfo};
 use crate::types::PyAny;
 use crate::types::PyTuple;
-use crate::{ffi, gil, Py, Python};
+use crate::{ffi, gil, Py, PyNativeType, Python};
 use std::ptr::NonNull;
 
 /// This trait represents that, **we can do zero-cost conversion from the object to FFI pointer**.
@@ -244,25 +244,77 @@ 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)?)
+pub mod extract_impl {
+    use super::*;
+
+    pub trait ExtractImpl<'a, Target>: Sized {
+        fn extract(source: &'a PyAny, py: Python<'a>) -> 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, py: Python<'a>) -> PyResult<T> {
+            Ok(Reference::extract(source, py)?.clone())
+        }
+    }
+
+    impl<'a, T> ExtractImpl<'a, &'a T> for Reference
+    where
+        T: PyTryFrom<'a>,
+    {
+        fn extract(source: &'a PyAny, _: Python<'a>) -> 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, _: Python<'a>) -> 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;
+
+/// Implement this trait with to specify the implementor of ExtractImpl to use for extracting
+/// this type from Python objects.
+///
+/// Example valid implementations are `Cloned`, `Reference`, and `MutReference`, which are for
+/// extracting `T`, `&T` and `&mut T` respectively via PyTryFrom
+///
+/// This is an internal trait mostly for re-using FromPyObject implementations for many pyo3
+/// types.
+///
+/// Most users should implement `FromPyObject` directly instead of via this trait..
+pub trait FromPyObjectImpl {
+    // We deliberately don't require Policy: ExtractImpl here because we allow #[pyclass]
+    // to specify Policies when they don't satisfy the ExtractImpl constraints.
+    //
+    // e.g. non-clone #[pyclass] can still have Policy: Cloned.
+    //
+    // We catch invalid policies in the blanket impl for FromPyObject, which only
+    // complains when .extract() is actually used.
+    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, ob.py())
     }
 }
 

src/lib.rs

@@ -119,8 +119,8 @@
 
 pub use crate::class::*;
 pub use crate::conversion::{
-    AsPyPointer, FromPy, FromPyObject, FromPyPointer, IntoPy, IntoPyPointer, PyTryFrom, PyTryInto,
-    ToBorrowedObject, ToPyObject,
+    extract_impl, AsPyPointer, FromPy, FromPyObject, FromPyObjectImpl, FromPyPointer, IntoPy,
+    IntoPyPointer, PyTryFrom, PyTryInto, ToBorrowedObject, ToPyObject,
 };
 pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyErrValue, PyResult};
 pub use crate::gil::{init_once, GILGuard, GILPool};

src/types/mod.rs

@@ -164,6 +164,10 @@ macro_rules! pyobject_native_type_convert(
             }
         }
 
+        impl<$($type_param,)*> $crate::conversion::FromPyObjectImpl for &'_ $name {
+            type Impl = $crate::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`