Update PyClassGetterGenerator to allow IntoPyObject conversion for Deref::Target

newsfragments/4987.added.md

@@ -0,0 +1 @@
+Add `IntoPyObject` & `FromPyObject` for `Arc<T>`

pyo3-macros-backend/src/pymethod.rs

@@ -845,6 +845,7 @@ pub fn impl_py_getter_def(
                         { #pyo3_path::impl_::pyclass::IsIntoPy::<#ty>::VALUE },
                         { #pyo3_path::impl_::pyclass::IsIntoPyObjectRef::<#ty>::VALUE },
                         { #pyo3_path::impl_::pyclass::IsIntoPyObject::<#ty>::VALUE },
+                        { #pyo3_path::impl_::pyclass::IsDerefIntoPyObject::<#ty>::VALUE },
                     > = unsafe { #pyo3_path::impl_::pyclass::PyClassGetterGenerator::new() };
                     #generator
                 }

src/conversions/std/mod.rs

@@ -9,5 +9,6 @@ mod path;
 mod set;
 mod slice;
 mod string;
+mod sync;
 mod time;
 mod vec;

src/conversions/std/sync.rs

@@ -0,0 +1,19 @@
+#[cfg(feature = "experimental-inspect")]
+use crate::inspect::types::TypeInfo;
+use crate::types::PyAnyMethods;
+use crate::{Bound, FromPyObject, PyAny, PyResult};
+use std::sync::Arc;
+
+impl<'py, T> FromPyObject<'py> for Arc<T>
+where
+    T: FromPyObject<'py>,
+{
+    fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
+        ob.extract::<T>().map(Arc::new)
+    }
+
+    #[cfg(feature = "experimental-inspect")]
+    fn type_input() -> TypeInfo {
+        T::type_input()
+    }
+}

src/impl_/pyclass.rs

@@ -14,6 +14,7 @@ use crate::{
 };
 #[allow(deprecated)]
 use crate::{IntoPy, ToPyObject};
+use std::ops::Deref;
 use std::{
     borrow::Cow,
     ffi::{CStr, CString},
@@ -1229,6 +1230,7 @@ pub struct PyClassGetterGenerator<
     const IMPLEMENTS_INTOPY: bool,
     const IMPLEMENTS_INTOPYOBJECT_REF: bool,
     const IMPLEMENTS_INTOPYOBJECT: bool,
+    const IMPLEMENTS_DEREF: bool,
 >(PhantomData<(ClassT, FieldT, Offset)>);
 
 impl<
@@ -1240,6 +1242,7 @@ impl<
         const IMPLEMENTS_INTOPY: bool,
         const IMPLEMENTS_INTOPYOBJECT_REF: bool,
         const IMPLEMENTS_INTOPYOBJECT: bool,
+        const IMPLEMENTS_DEREF: bool,
     >
     PyClassGetterGenerator<
         ClassT,
@@ -1250,6 +1253,7 @@ impl<
         IMPLEMENTS_INTOPY,
         IMPLEMENTS_INTOPYOBJECT_REF,
         IMPLEMENTS_INTOPYOBJECT,
+        IMPLEMENTS_DEREF,
     >
 {
     /// Safety: constructing this type requires that there exists a value of type FieldT
@@ -1267,6 +1271,7 @@ impl<
         const IMPLEMENTS_INTOPY: bool,
         const IMPLEMENTS_INTOPYOBJECT_REF: bool,
         const IMPLEMENTS_INTOPYOBJECT: bool,
+        const IMPLEMENTS_DEREF: bool,
     >
     PyClassGetterGenerator<
         ClassT,
@@ -1277,6 +1282,7 @@ impl<
         IMPLEMENTS_INTOPY,
         IMPLEMENTS_INTOPYOBJECT_REF,
         IMPLEMENTS_INTOPYOBJECT,
+        IMPLEMENTS_DEREF,
     >
 {
     /// `Py<T>` fields have a potential optimization to use Python's "struct members" to read
@@ -1312,7 +1318,19 @@ impl<
         FieldT: ToPyObject,
         Offset: OffsetCalculator<ClassT, FieldT>,
         const IMPLEMENTS_INTOPY: bool,
-    > PyClassGetterGenerator<ClassT, FieldT, Offset, false, true, IMPLEMENTS_INTOPY, false, false>
+        const IMPLEMENTS_DEREF: bool,
+    >
+    PyClassGetterGenerator<
+        ClassT,
+        FieldT,
+        Offset,
+        false,
+        true,
+        IMPLEMENTS_INTOPY,
+        false,
+        false,
+        IMPLEMENTS_DEREF,
+    >
 {
     pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType {
         PyMethodDefType::Getter(PyGetterDef {
@@ -1332,6 +1350,7 @@ impl<
         const IMPLEMENTS_TOPYOBJECT: bool,
         const IMPLEMENTS_INTOPY: bool,
         const IMPLEMENTS_INTOPYOBJECT: bool,
+        const IMPLEMENTS_DEREF: bool,
     >
     PyClassGetterGenerator<
         ClassT,
@@ -1342,6 +1361,7 @@ impl<
         IMPLEMENTS_INTOPY,
         true,
         IMPLEMENTS_INTOPYOBJECT,
+        IMPLEMENTS_DEREF,
     >
 where
     ClassT: PyClass,
@@ -1357,9 +1377,34 @@ where
     }
 }
 
+/// If Field does not implement `IntoPyObject` but Deref::Target does, use that instead
+impl<ClassT, FieldT, Offset>
+    PyClassGetterGenerator<ClassT, FieldT, Offset, false, false, false, false, false, true>
+where
+    ClassT: PyClass,
+    FieldT: Deref,
+    for<'a, 'py> &'a FieldT::Target: IntoPyObject<'py>,
+    Offset: OffsetCalculator<ClassT, FieldT>,
+{
+    pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType {
+        PyMethodDefType::Getter(PyGetterDef {
+            name,
+            meth: pyo3_get_value_into_pyobject_deref::<ClassT, FieldT, Offset>,
+            doc,
+        })
+    }
+}
+
 /// Temporary case to prefer `IntoPyObject + Clone` over `IntoPy + Clone`, while still showing the
 /// `IntoPyObject` suggestion if neither is implemented;
-impl<ClassT, FieldT, Offset, const IMPLEMENTS_TOPYOBJECT: bool, const IMPLEMENTS_INTOPY: bool>
+impl<
+        ClassT,
+        FieldT,
+        Offset,
+        const IMPLEMENTS_TOPYOBJECT: bool,
+        const IMPLEMENTS_INTOPY: bool,
+        const IMPLEMENTS_DEREF: bool,
+    >
     PyClassGetterGenerator<
         ClassT,
         FieldT,
@@ -1369,6 +1414,7 @@ impl<ClassT, FieldT, Offset, const IMPLEMENTS_TOPYOBJECT: bool, const IMPLEMENTS
         IMPLEMENTS_INTOPY,
         false,
         true,
+        IMPLEMENTS_DEREF,
     >
 where
     ClassT: PyClass,
@@ -1386,8 +1432,18 @@ where
 
 /// IntoPy + Clone fallback case, which was the only behaviour before PyO3 0.22.
 #[allow(deprecated)]
-impl<ClassT, FieldT, Offset>
-    PyClassGetterGenerator<ClassT, FieldT, Offset, false, false, true, false, false>
+impl<ClassT, FieldT, Offset, const IMPLEMENTS_DEREF: bool>
+    PyClassGetterGenerator<
+        ClassT,
+        FieldT,
+        Offset,
+        false,
+        false,
+        true,
+        false,
+        false,
+        IMPLEMENTS_DEREF,
+    >
 where
     ClassT: PyClass,
     Offset: OffsetCalculator<ClassT, FieldT>,
@@ -1415,7 +1471,7 @@ impl<'py, T> PyO3GetField<'py> for T where T: IntoPyObject<'py> + Clone {}
 
 /// Base case attempts to use IntoPyObject + Clone
 impl<ClassT: PyClass, FieldT, Offset: OffsetCalculator<ClassT, FieldT>>
-    PyClassGetterGenerator<ClassT, FieldT, Offset, false, false, false, false, false>
+    PyClassGetterGenerator<ClassT, FieldT, Offset, false, false, false, false, false, false>
 {
     pub const fn generate(&self, _name: &'static CStr, _doc: &'static CStr) -> PyMethodDefType
     // The bound goes here rather than on the block so that this impl is always available
@@ -1489,6 +1545,28 @@ where
         .into_ptr())
 }
 
+fn pyo3_get_value_into_pyobject_deref<ClassT, FieldT, Offset>(
+    py: Python<'_>,
+    obj: *mut ffi::PyObject,
+) -> PyResult<*mut ffi::PyObject>
+where
+    ClassT: PyClass,
+    FieldT: Deref,
+    for<'a, 'py> &'a FieldT::Target: IntoPyObject<'py>,
+    Offset: OffsetCalculator<ClassT, FieldT>,
+{
+    let _holder = unsafe { ensure_no_mutable_alias::<ClassT>(py, &obj)? };
+    let value = field_from_object::<ClassT, FieldT, Offset>(obj);
+
+    // SAFETY: Offset is known to describe the location of the value, and
+    // _holder is preventing mutable aliasing
+    Ok((unsafe { &*value })
+        .deref()
+        .into_pyobject(py)
+        .map_err(Into::into)?
+        .into_ptr())
+}
+
 fn pyo3_get_value_into_pyobject<ClassT, FieldT, Offset>(
     py: Python<'_>,
     obj: *mut ffi::PyObject,

src/impl_/pyclass/probes.rs

@@ -1,8 +1,8 @@
-use std::marker::PhantomData;
-
 use crate::{conversion::IntoPyObject, Py};
 #[allow(deprecated)]
 use crate::{IntoPy, ToPyObject};
+use std::marker::PhantomData;
+use std::sync::Arc;
 
 /// Trait used to combine with zero-sized types to calculate at compile time
 /// some property of a type.
@@ -70,3 +70,19 @@ probe!(IsSync);
 impl<T: Sync> IsSync<T> {
     pub const VALUE: bool = true;
 }
+
+probe!(IsDerefIntoPyObject);
+
+impl<T> IsDerefIntoPyObject<Arc<T>>
+where
+    for<'a, 'py> &'a T: IntoPyObject<'py>,
+{
+    pub const VALUE: bool = true;
+}
+
+impl<T> IsDerefIntoPyObject<Box<T>>
+where
+    for<'a, 'py> &'a T: IntoPyObject<'py>,
+{
+    pub const VALUE: bool = true;
+}

tests/test_getter_setter.rs

@@ -318,3 +318,24 @@ fn test_optional_setter() {
         );
     })
 }
+
+#[pyclass(get_all)]
+struct ArcGetterSetter {
+    #[pyo3(set)]
+    foo: std::sync::Arc<i32>,
+}
+
+#[test]
+fn test_arc_getter_setter() {
+    Python::with_gil(|py| {
+        let instance = Py::new(
+            py,
+            ArcGetterSetter {
+                foo: std::sync::Arc::new(42),
+            },
+        )
+        .unwrap();
+        py_run!(py, instance, "assert instance.foo == 42");
+        py_run!(py, instance, "instance.foo = 43; assert instance.foo == 43");
+    })
+}

tests/ui/invalid_property_args.stderr

@@ -65,11 +65,11 @@ error[E0277]: `PhantomData<i32>` cannot be converted to a Python object
              &'a (T0, T1, T2, T3, T4)
            and $N others
    = note: required for `PhantomData<i32>` to implement `for<'py> PyO3GetField<'py>`
-note: required by a bound in `PyClassGetterGenerator::<ClassT, FieldT, Offset, false, false, false, false, false>::generate`
+note: required by a bound in `PyClassGetterGenerator::<ClassT, FieldT, Offset, false, false, false, false, false, false>::generate`
   --> src/impl_/pyclass.rs
    |
    |     pub const fn generate(&self, _name: &'static CStr, _doc: &'static CStr) -> PyMethodDefType
    |                  -------- required by a bound in this associated function
 ...
    |         for<'py> FieldT: PyO3GetField<'py>,
-   |                          ^^^^^^^^^^^^^^^^^ required by this bound in `PyClassGetterGenerator::<ClassT, FieldT, Offset, false, false, false, false, false>::generate`
+   |                          ^^^^^^^^^^^^^^^^^ required by this bound in `PyClassGetterGenerator::<ClassT, FieldT, Offset, false, false, false, false, false, false>::generate`