add #[pyclass(from_py_object)] to opt-in to a FromPyObject impl (#5506)

Author: Icxolu

guide/pyclass-parameters.md

@@ -9,6 +9,7 @@
 | `eq_int` | Implements `__eq__` using `__int__` for simple enums. |
 | <span style="white-space: pre">`extends = BaseType`</span>  | Use a custom baseclass. Defaults to [`PyAny`][params-1] |
 | <span style="white-space: pre">`freelist = N`</span> |  Implements a [free list][params-2] of size N. This can improve performance for types that are often created and deleted in quick succession. Profile your code to see whether `freelist` is right for you.  |
+| `from_py_object` | Implement `FromPyObject` for this pyclass. Requires the pyclass to be `Clone`. |
 | <span style="white-space: pre">`frozen`</span> | Declares that your pyclass is immutable. It removes the borrow checker overhead when retrieving a shared reference to the Rust struct, but disables the ability to get a mutable reference. |
 | `generic` | Implements runtime parametrization for the class following [PEP 560](https://peps.python.org/pep-0560/). |
 | `get_all` | Generates getters for all fields of the pyclass. |

guide/src/conversions/traits.md

@@ -558,6 +558,8 @@ impl<'py> FromPyObject<'_, 'py> for Number {
 }
 ```
 
+As a second step the `from_py_object` option was introduced. This option also opts-out of the blanket implementation and instead generates a custom `FromPyObject` implementation for the pyclass which is functionally equivalent to the blanket.
+
 ### `IntoPyObject`
 The [`IntoPyObject`] trait defines the to-python conversion for a Rust type. All types in PyO3 implement this trait,
 as does a `#[pyclass]` which doesn't use `extends`.

newsfragments/5506.added.md

@@ -0,0 +1 @@
+add `from_py_object` pyclass option, to opt-in to the extraction of pyclasses by value (requires `Clone`)

pyo3-macros-backend/src/attributes.rs

@@ -53,6 +53,7 @@ pub mod kw {
     syn::custom_keyword!(warn);
     syn::custom_keyword!(message);
     syn::custom_keyword!(category);
+    syn::custom_keyword!(from_py_object);
     syn::custom_keyword!(skip_from_py_object);
 }
 

pyo3-macros-backend/src/pyclass.rs

@@ -90,6 +90,7 @@ pub struct PyClassPyO3Options {
     pub unsendable: Option<kw::unsendable>,
     pub weakref: Option<kw::weakref>,
     pub generic: Option<kw::generic>,
+    pub from_py_object: Option<kw::from_py_object>,
     pub skip_from_py_object: Option<kw::skip_from_py_object>,
 }
 
@@ -116,6 +117,7 @@ pub enum PyClassPyO3Option {
     Unsendable(kw::unsendable),
     Weakref(kw::weakref),
     Generic(kw::generic),
+    FromPyObject(kw::from_py_object),
     SkipFromPyObject(kw::skip_from_py_object),
 }
 
@@ -166,6 +168,8 @@ impl Parse for PyClassPyO3Option {
             input.parse().map(PyClassPyO3Option::Weakref)
         } else if lookahead.peek(attributes::kw::generic) {
             input.parse().map(PyClassPyO3Option::Generic)
+        } else if lookahead.peek(attributes::kw::from_py_object) {
+            input.parse().map(PyClassPyO3Option::FromPyObject)
         } else if lookahead.peek(attributes::kw::skip_from_py_object) {
             input.parse().map(PyClassPyO3Option::SkipFromPyObject)
         } else {
@@ -248,8 +252,19 @@ impl PyClassPyO3Options {
             }
             PyClassPyO3Option::Generic(generic) => set_option!(generic),
             PyClassPyO3Option::SkipFromPyObject(skip_from_py_object) => {
+                ensure_spanned!(
+                    self.from_py_object.is_none(),
+                    skip_from_py_object.span() => "`skip_from_py_object` and `from_py_object` are mutually exclusive"
+                );
                 set_option!(skip_from_py_object)
             }
+            PyClassPyO3Option::FromPyObject(from_py_object) => {
+                ensure_spanned!(
+                    self.skip_from_py_object.is_none(),
+                    from_py_object.span() => "`skip_from_py_object` and `from_py_object` are mutually exclusive"
+                );
+                set_option!(from_py_object)
+            }
         }
         Ok(())
     }
@@ -2504,7 +2519,29 @@ impl<'a> PyClassImplsBuilder<'a> {
             quote! {}
         };
 
-        let extract_pyclass_with_clone = if self.attr.options.skip_from_py_object.is_none() {
+        let extract_pyclass_with_clone = if let Some(from_py_object) =
+            self.attr.options.from_py_object
+        {
+            let input_ty = if cfg!(feature = "experimental-inspect") {
+                quote!(const INPUT_TYPE: &'static str = <#cls as #pyo3_path::impl_::pyclass::PyClassImpl>::TYPE_NAME;)
+            } else {
+                TokenStream::new()
+            };
+            quote_spanned! { from_py_object.span() =>
+                impl<'a, 'py> #pyo3_path::FromPyObject<'a, 'py> for #cls
+                where
+                    Self: ::std::clone::Clone,
+                {
+                    type Error = #pyo3_path::pyclass::PyClassGuardError<'a, 'py>;
+
+                    #input_ty
+
+                    fn extract(obj: #pyo3_path::Borrowed<'a, 'py, #pyo3_path::PyAny>) -> ::std::result::Result<Self, Self::Error> {
+                        ::std::result::Result::Ok(::std::clone::Clone::clone(&*obj.extract::<#pyo3_path::PyClassGuard<'_, #cls>>()?))
+                    }
+                }
+            }
+        } else if self.attr.options.skip_from_py_object.is_none() {
             quote!( impl #pyo3_path::impl_::pyclass::ExtractPyClassWithClone for #cls {} )
         } else {
             TokenStream::new()

src/conversion.rs

@@ -636,4 +636,25 @@ mod tests {
         })
         .unwrap();
     }
+
+    #[test]
+    #[cfg(feature = "macros")]
+    fn test_pyclass_from_py_object() {
+        use crate::{types::PyAnyMethods, IntoPyObject, PyErr, Python};
+
+        #[crate::pyclass(crate = "crate", from_py_object)]
+        #[derive(Clone)]
+        struct Foo(i32);
+
+        Python::attach(|py| {
+            let foo1 = 42i32.into_pyobject(py)?;
+            assert!(foo1.extract::<Foo>().is_err());
+
+            let foo2 = Foo(0).into_pyobject(py)?;
+            assert_eq!(foo2.extract::<Foo>()?.0, 0);
+
+            Ok::<_, PyErr>(())
+        })
+        .unwrap();
+    }
 }

src/tests/hygiene/pyclass.rs

@@ -151,3 +151,9 @@ impl ::std::fmt::Display for Point {
         ::std::write!(f, "({}, {}, {})", self.x, self.y, self.z)
     }
 }
+
+#[crate::pyclass(crate = "crate", from_py_object)]
+#[derive(Clone)]
+pub struct Foo5 {
+    a: i32,
+}

tests/ui/invalid_pyclass_args.rs

@@ -181,4 +181,16 @@ impl Display for MyEnumInvalidStrFmt {
     }
 }
 
+#[pyclass(from_py_object, skip_from_py_object)]
+struct StructTooManyFromPyObject {
+    a: String,
+    b: String,
+}
+
+#[pyclass(from_py_object)]
+struct StructFromPyObjectNoClone {
+    a: String,
+    b: String,
+}
+
 fn main() {}

tests/ui/invalid_pyclass_args.stderr

@@ -1,4 +1,4 @@
-error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `immutable_type`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `str`, `subclass`, `unsendable`, `weakref`, `generic`, `skip_from_py_object`
+error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `immutable_type`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `str`, `subclass`, `unsendable`, `weakref`, `generic`, `from_py_object`, `skip_from_py_object`
  --> tests/ui/invalid_pyclass_args.rs:4:11
   |
 4 | #[pyclass(extend=pyo3::types::PyDict)]
@@ -46,7 +46,7 @@ error: expected string literal
 25 | #[pyclass(module = my_module)]
    |                    ^^^^^^^^^
 
-error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `immutable_type`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `str`, `subclass`, `unsendable`, `weakref`, `generic`, `skip_from_py_object`
+error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `hash`, `immutable_type`, `mapping`, `module`, `name`, `ord`, `rename_all`, `sequence`, `set_all`, `str`, `subclass`, `unsendable`, `weakref`, `generic`, `from_py_object`, `skip_from_py_object`
   --> tests/ui/invalid_pyclass_args.rs:28:11
    |
 28 | #[pyclass(weakrev)]
@@ -162,6 +162,25 @@ error: The format string syntax cannot be used with enums
 171 | #[pyclass(eq, str = "Stuff...")]
     |                     ^^^^^^^^^^
 
+error: `skip_from_py_object` and `from_py_object` are mutually exclusive
+   --> tests/ui/invalid_pyclass_args.rs:184:27
+    |
+184 | #[pyclass(from_py_object, skip_from_py_object)]
+    |                           ^^^^^^^^^^^^^^^^^^^
+
+error[E0277]: the trait bound `StructFromPyObjectNoClone: Clone` is not satisfied
+   --> tests/ui/invalid_pyclass_args.rs:190:11
+    |
+190 | #[pyclass(from_py_object)]
+    |           ^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `StructFromPyObjectNoClone`
+    |
+    = help: see issue #48214
+help: consider annotating `StructFromPyObjectNoClone` with `#[derive(Clone)]`
+    |
+191 + #[derive(Clone)]
+192 | struct StructFromPyObjectNoClone {
+    |
+
 error[E0592]: duplicate definitions with name `__pymethod___richcmp____`
   --> tests/ui/invalid_pyclass_args.rs:37:1
    |