emit deprecation on pyclasses that are affected by the FromPyObject blanket impl

guide/src/class.md

@@ -893,7 +893,7 @@ Classes can also be passed by value if they can be cloned, i.e. they automatical
 ```rust,no_run
 # #![allow(dead_code)]
 # use pyo3::prelude::*;
-#[pyclass]
+#[pyclass(from_py_object)]
 #[derive(Clone)]
 struct MyClass {
     my_field: Box<i32>,

guide/src/faq.md

@@ -99,7 +99,7 @@ You may have a nested struct similar to this:
 
 ```rust,no_run
 # use pyo3::prelude::*;
-#[pyclass]
+#[pyclass(from_py_object)]
 #[derive(Clone)]
 struct Inner {/* fields omitted */}
 

guide/src/migration.md

@@ -3,6 +3,18 @@
 This guide can help you upgrade code through breaking changes from one PyO3 version to the next.
 For a detailed list of all changes, see the [CHANGELOG](changelog.md).
 
+## from 0.27.* to 0.28
+
+### Deprecation of automatic `FromPyObject` for `#[pyclass]` types which implement `Clone`
+
+`#[pyclass]` types which implement `Clone` used to also implement `FromPyObject` automatically.
+This behaviour is phased out and replaced by an explicit opt-in.
+Affected types will by marked by a deprecation message.
+To migrate use either
+
+- `from_py_object` to keep the automatic derive, or
+- `skip_from_py_object` to accept the new behaviour
+
 ## from 0.26.* to 0.27
 
 ### `FromPyObject` reworked for flexibility and efficiency

newsfragments/5550.changed.md

@@ -0,0 +1 @@
+deprecate implicit by value extraction of pyclasses

pyo3-macros-backend/src/pyclass.rs

@@ -2511,6 +2511,20 @@ impl<'a> PyClassImplsBuilder<'a> {
             }
         };
 
+        let deprecation = if self.attr.options.skip_from_py_object.is_none()
+            && self.attr.options.from_py_object.is_none()
+        {
+            quote! {
+                const _: () = {
+                    #[allow(unused_import)]
+                    use #pyo3_path::impl_::pyclass::Probe as _;
+                    #pyo3_path::impl_::deprecated::HasAutomaticFromPyObject::<{ #pyo3_path::impl_::pyclass::IsClone::<#cls>::VALUE }>::MSG
+                };
+            }
+        } else {
+            TokenStream::new()
+        };
+
         let extract_pyclass_with_clone = if let Some(from_py_object) =
             self.attr.options.from_py_object
         {
@@ -2540,6 +2554,8 @@ impl<'a> PyClassImplsBuilder<'a> {
         };
 
         Ok(quote! {
+            #deprecation
+
             #extract_pyclass_with_clone
 
             #assertions

pytests/src/pyclasses.rs

@@ -4,7 +4,7 @@ use pyo3::exceptions::{PyStopIteration, PyValueError};
 use pyo3::prelude::*;
 use pyo3::types::PyType;
 
-#[pyclass]
+#[pyclass(from_py_object)]
 #[derive(Clone, Default)]
 struct EmptyClass {}
 
@@ -70,7 +70,7 @@ impl PyClassThreadIter {
 }
 
 /// Demonstrates a base class which can operate on the relevant subclass in its constructor.
-#[pyclass(subclass)]
+#[pyclass(subclass, skip_from_py_object)]
 #[derive(Clone, Debug)]
 struct AssertingBaseClass;
 
@@ -104,7 +104,7 @@ impl ClassWithDict {
     }
 }
 
-#[pyclass]
+#[pyclass(skip_from_py_object)]
 #[derive(Clone)]
 struct ClassWithDecorators {
     attr: usize,

src/impl_.rs

@@ -10,6 +10,7 @@ pub mod callback;
 pub mod concat;
 #[cfg(feature = "experimental-async")]
 pub mod coroutine;
+pub mod deprecated;
 pub mod exceptions;
 pub mod extract_argument;
 pub mod freelist;

src/impl_/deprecated.rs

@@ -0,0 +1,13 @@
+pub struct HasAutomaticFromPyObject<const IS_CLONE: bool> {}
+
+impl HasAutomaticFromPyObject<true> {
+    #[deprecated(
+        since = "0.28.0",
+        note = "The automatically derived `FromPyObject` implementation for `#[pyclass]` types which implement `Clone` is being phased out. Use `from_py_object` to keep the automatic derive or `skip_from_py_object` to accept the new behaviour."
+    )]
+    pub const MSG: () = ();
+}
+
+impl HasAutomaticFromPyObject<false> {
+    pub const MSG: () = ();
+}

src/impl_/pyclass/probes.rs

@@ -73,6 +73,12 @@ impl<T: super::doc::PyClassNewTextSignature> HasNewTextSignature<T> {
     pub const VALUE: bool = true;
 }
 
+probe!(IsClone);
+
+impl<T: Clone> IsClone<T> {
+    pub const VALUE: bool = true;
+}
+
 #[cfg(test)]
 macro_rules! value_of {
     ($probe:ident, $ty:ty) => {{

src/marker.rs

@@ -1007,7 +1007,7 @@ mod tests {
     fn test_py_run_inserts_globals_2() {
         use std::ffi::CString;
 
-        #[crate::pyclass(crate = "crate")]
+        #[crate::pyclass(crate = "crate", skip_from_py_object)]
         #[derive(Clone)]
         struct CodeRunner {
             code: CString,

src/pycell.rs

@@ -753,7 +753,7 @@ mod tests {
 
     use super::*;
 
-    #[crate::pyclass]
+    #[crate::pyclass(skip_from_py_object)]
     #[pyo3(crate = "crate")]
     #[derive(Copy, Clone, PartialEq, Eq, Debug)]
     struct SomeClass(i32);

src/tests/hygiene/pyclass.rs

@@ -1,4 +1,4 @@
-#[crate::pyclass]
+#[crate::pyclass(from_py_object)]
 #[pyo3(crate = "crate")]
 #[derive(::std::clone::Clone)]
 pub struct Foo;

tests/test_class_comparisons.rs

@@ -4,14 +4,14 @@ use pyo3::prelude::*;
 
 mod test_utils;
 
-#[pyclass(eq)]
+#[pyclass(eq, skip_from_py_object)]
 #[derive(Debug, Clone, PartialEq)]
 pub enum MyEnum {
     Variant,
     OtherVariant,
 }
 
-#[pyclass(eq, ord)]
+#[pyclass(eq, ord, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)]
 pub enum MyEnumOrd {
     Variant,
@@ -63,14 +63,14 @@ fn test_simple_enum_ord_comparable() {
     })
 }
 
-#[pyclass(eq, ord)]
+#[pyclass(eq, ord, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)]
 pub enum MyComplexEnumOrd {
     Variant(i32),
     OtherVariant(String),
 }
 
-#[pyclass(eq, ord)]
+#[pyclass(eq, ord, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)]
 pub enum MyComplexEnumOrd2 {
     Variant { msg: String, idx: u32 },
@@ -145,7 +145,7 @@ fn test_complex_enum_ord_comparable() {
     })
 }
 
-#[pyclass(eq, ord)]
+#[pyclass(eq, ord, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)]
 pub struct Point {
     x: i32,
@@ -170,7 +170,7 @@ fn test_struct_numeric_ord_comparable() {
     })
 }
 
-#[pyclass(eq, ord)]
+#[pyclass(eq, ord, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)]
 pub struct Person {
     surname: String,
@@ -222,7 +222,7 @@ fn test_struct_string_ord_comparable() {
     })
 }
 
-#[pyclass(eq, ord)]
+#[pyclass(eq, ord, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct Record {
     name: String,

tests/test_class_conversion.rs

@@ -5,7 +5,7 @@ use pyo3::prelude::*;
 #[macro_use]
 mod test_utils;
 
-#[pyclass]
+#[pyclass(from_py_object)]
 #[derive(Clone, Debug, PartialEq)]
 struct Cloneable {
     x: i32,

tests/test_class_formatting.rs

@@ -49,7 +49,7 @@ fn test_enum_class_fmt() {
     })
 }
 
-#[pyclass(str = "X: {x}, Y: {y}, Z: {z}")]
+#[pyclass(str = "X: {x}, Y: {y}, Z: {z}", skip_from_py_object)]
 #[derive(PartialEq, Eq, Clone, PartialOrd)]
 pub struct Point {
     x: i32,
@@ -65,7 +65,7 @@ fn test_custom_struct_custom_str() {
     })
 }
 
-#[pyclass(str)]
+#[pyclass(str, skip_from_py_object)]
 #[derive(PartialEq, Eq, Clone, PartialOrd)]
 pub struct Point2 {
     x: i32,

tests/test_enum.rs

@@ -6,7 +6,7 @@ use pyo3::types::PyString;
 
 mod test_utils;
 
-#[pyclass(eq, eq_int)]
+#[pyclass(eq, eq_int, from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum MyEnum {
     Variant,
@@ -73,7 +73,7 @@ fn test_enum_arg() {
     })
 }
 
-#[pyclass(eq, eq_int)]
+#[pyclass(eq, eq_int, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 enum CustomDiscriminant {
     One = 1,
@@ -126,7 +126,7 @@ fn test_enum_compare_int() {
     })
 }
 
-#[pyclass(eq, eq_int)]
+#[pyclass(eq, eq_int, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[repr(u8)]
 enum SmallEnum {
@@ -141,7 +141,7 @@ fn test_enum_compare_int_no_throw_when_overflow() {
     })
 }
 
-#[pyclass(eq, eq_int)]
+#[pyclass(eq, eq_int, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[repr(usize)]
 #[allow(clippy::enum_clike_unportable_variant)]
@@ -160,7 +160,7 @@ fn test_big_enum_no_overflow() {
     })
 }
 
-#[pyclass(eq, eq_int)]
+#[pyclass(eq, eq_int, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[repr(u16, align(8))]
 enum TestReprParse {
@@ -172,7 +172,7 @@ fn test_repr_parse() {
     assert_eq!(std::mem::align_of::<TestReprParse>(), 8);
 }
 
-#[pyclass(eq, eq_int, name = "MyEnum")]
+#[pyclass(eq, eq_int, name = "MyEnum", skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum RenameEnum {
     Variant,
@@ -186,7 +186,7 @@ fn test_rename_enum_repr_correct() {
     })
 }
 
-#[pyclass(eq, eq_int)]
+#[pyclass(eq, eq_int, skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum RenameVariantEnum {
     #[pyo3(name = "VARIANT")]
@@ -201,7 +201,7 @@ fn test_rename_variant_repr_correct() {
     })
 }
 
-#[pyclass(eq, eq_int, rename_all = "SCREAMING_SNAKE_CASE")]
+#[pyclass(eq, eq_int, rename_all = "SCREAMING_SNAKE_CASE", skip_from_py_object)]
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[allow(clippy::enum_variant_names)]
 enum RenameAllVariantsEnum {
@@ -244,7 +244,7 @@ fn test_custom_module() {
     });
 }
 
-#[pyclass(eq)]
+#[pyclass(eq, skip_from_py_object)]
 #[derive(Debug, Clone, PartialEq)]
 pub enum EqOnly {
     VariantA,
@@ -369,7 +369,7 @@ fn custom_eq() {
     })
 }
 
-#[pyclass]
+#[pyclass(skip_from_py_object)]
 #[derive(Clone, Copy)]
 pub enum ComplexEnumWithRaw {
     Raw { r#type: i32 },

tests/test_frompyobject.rs

@@ -129,7 +129,7 @@ pub struct E<T, T2> {
     test2: T2,
 }
 
-#[pyclass]
+#[pyclass(skip_from_py_object)]
 #[derive(Clone)]
 pub struct PyE {
     #[pyo3(get)]

tests/test_pyself.rs

@@ -9,7 +9,7 @@ mod test_utils;
 
 /// Assumes it's a file reader or so.
 /// Inspired by https://github.com/jothan/cordoba, thanks.
-#[pyclass]
+#[pyclass(skip_from_py_object)]
 #[derive(Clone, Debug)]
 struct Reader {
     inner: HashMap<u8, String>,

tests/ui/forbid_unsafe.rs

@@ -11,20 +11,20 @@ mod gh_4394 {
     use pyo3::prelude::*;
 
     #[derive(Eq, Ord, PartialEq, PartialOrd, Clone)]
-    #[pyclass(get_all)]
+    #[pyclass(get_all, skip_from_py_object)]
     pub struct VersionSpecifier {
         pub(crate) operator: Operator,
         pub(crate) version: Version,
     }
 
     #[derive(Eq, Ord, PartialEq, PartialOrd, Debug, Hash, Clone, Copy)]
-    #[pyo3::pyclass(eq, eq_int)]
+    #[pyo3::pyclass(eq, eq_int, skip_from_py_object)]
     pub enum Operator {
         Equal,
     }
 
     #[derive(Clone, Eq, PartialEq, PartialOrd, Ord)]
-    #[pyclass]
+    #[pyclass(skip_from_py_object)]
     pub struct Version;
 }
 

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, IMPLEMENTS_INTOPYOBJECT>::generate`
+note: required by a bound in `PyClassGetterGenerator::<ClassT, FieldT, OFFSET, false, false, IMPLEMENTS_INTOPYOBJECT>::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, IMPLEMENTS_INTOPYOBJECT>::generate`
+   |                          ^^^^^^^^^^^^^^^^^ required by this bound in `PyClassGetterGenerator::<ClassT, FieldT, OFFSET, false, false, IMPLEMENTS_INTOPYOBJECT>::generate`