diff --git a/CHANGELOG.md b/CHANGELOG.md index 1262cf455b3..bbc0cc14a37 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs index 5c845bb8f5e..d72630cf19b 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::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 diff --git a/src/conversion.rs b/src/conversion.rs index d24e9f48ab5..56823da0fc0 100644 --- a/src/conversion.rs +++ b/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; + } + + 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 { + 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, + ::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, ob.py()) } } diff --git a/src/lib.rs b/src/lib.rs index 0367af54cc4..594533fc631 100755 --- a/src/lib.rs +++ b/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}; diff --git a/src/types/mod.rs b/src/types/mod.rs index 53a7f1e1ee8..b6bd6fa1e93 100644 --- a/src/types/mod.rs +++ b/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> 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`