diff --git a/guide/src/class.md b/guide/src/class.md index ecc68f3a8a6..0e6529732c3 100644 --- a/guide/src/class.md +++ b/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, diff --git a/guide/src/faq.md b/guide/src/faq.md index 4fdb7f2b78c..9c81f793285 100644 --- a/guide/src/faq.md +++ b/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 */} diff --git a/guide/src/migration.md b/guide/src/migration.md index c92fed5126d..b4b7963fe0b 100644 --- a/guide/src/migration.md +++ b/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 diff --git a/newsfragments/5550.changed.md b/newsfragments/5550.changed.md new file mode 100644 index 00000000000..362b9c576a2 --- /dev/null +++ b/newsfragments/5550.changed.md @@ -0,0 +1 @@ +deprecate implicit by value extraction of pyclasses diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 07fccf3d655..5167ce3571a 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/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 diff --git a/pytests/src/pyclasses.rs b/pytests/src/pyclasses.rs index 631ec4fa0a0..ca943833f24 100644 --- a/pytests/src/pyclasses.rs +++ b/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, diff --git a/src/impl_.rs b/src/impl_.rs index d72a084e48f..17c1f08c36b 100644 --- a/src/impl_.rs +++ b/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; diff --git a/src/impl_/deprecated.rs b/src/impl_/deprecated.rs new file mode 100644 index 00000000000..68249f71820 --- /dev/null +++ b/src/impl_/deprecated.rs @@ -0,0 +1,13 @@ +pub struct HasAutomaticFromPyObject {} + +impl HasAutomaticFromPyObject { + #[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 { + pub const MSG: () = (); +} diff --git a/src/impl_/pyclass/probes.rs b/src/impl_/pyclass/probes.rs index ac254085dd1..b59f1e1a2f5 100644 --- a/src/impl_/pyclass/probes.rs +++ b/src/impl_/pyclass/probes.rs @@ -73,6 +73,12 @@ impl HasNewTextSignature { pub const VALUE: bool = true; } +probe!(IsClone); + +impl IsClone { + pub const VALUE: bool = true; +} + #[cfg(test)] macro_rules! value_of { ($probe:ident, $ty:ty) => {{ diff --git a/src/marker.rs b/src/marker.rs index 4c4cc7d7a82..ea09004fc8e 100644 --- a/src/marker.rs +++ b/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, diff --git a/src/pycell.rs b/src/pycell.rs index a9a282b24f0..92f207fd826 100644 --- a/src/pycell.rs +++ b/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); diff --git a/src/tests/hygiene/pyclass.rs b/src/tests/hygiene/pyclass.rs index 306712e4d78..0e2f707cdd4 100644 --- a/src/tests/hygiene/pyclass.rs +++ b/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; diff --git a/tests/test_class_comparisons.rs b/tests/test_class_comparisons.rs index e8509d1e266..c7950990b53 100644 --- a/tests/test_class_comparisons.rs +++ b/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, diff --git a/tests/test_class_conversion.rs b/tests/test_class_conversion.rs index 2ff0f56a9b3..be0a6ffe140 100644 --- a/tests/test_class_conversion.rs +++ b/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, diff --git a/tests/test_class_formatting.rs b/tests/test_class_formatting.rs index 1b4add063d4..b01122aa1c0 100644 --- a/tests/test_class_formatting.rs +++ b/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, diff --git a/tests/test_enum.rs b/tests/test_enum.rs index cdc9935a6df..80219306444 100644 --- a/tests/test_enum.rs +++ b/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::(), 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 }, diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index ecec5bcac06..c1136b3e040 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -129,7 +129,7 @@ pub struct E { test2: T2, } -#[pyclass] +#[pyclass(skip_from_py_object)] #[derive(Clone)] pub struct PyE { #[pyo3(get)] diff --git a/tests/test_pyself.rs b/tests/test_pyself.rs index b9b9b0ea4d2..a4d361773ec 100644 --- a/tests/test_pyself.rs +++ b/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, diff --git a/tests/ui/forbid_unsafe.rs b/tests/ui/forbid_unsafe.rs index 1966ab99a8f..5acc5c77d67 100644 --- a/tests/ui/forbid_unsafe.rs +++ b/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; } diff --git a/tests/ui/invalid_property_args.stderr b/tests/ui/invalid_property_args.stderr index 047e6709c21..dc52fc31159 100644 --- a/tests/ui/invalid_property_args.stderr +++ b/tests/ui/invalid_property_args.stderr @@ -65,11 +65,11 @@ error[E0277]: `PhantomData` cannot be converted to a Python object &'a (T0, T1, T2, T3, T4) and $N others = note: required for `PhantomData` to implement `for<'py> PyO3GetField<'py>` -note: required by a bound in `PyClassGetterGenerator::::generate` +note: required by a bound in `PyClassGetterGenerator::::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::::generate` + | ^^^^^^^^^^^^^^^^^ required by this bound in `PyClassGetterGenerator::::generate` diff --git a/tests/ui/invalid_pyclass_args.rs b/tests/ui/invalid_pyclass_args.rs index 8817a6f710f..917a88deb65 100644 --- a/tests/ui/invalid_pyclass_args.rs +++ b/tests/ui/invalid_pyclass_args.rs @@ -110,7 +110,7 @@ struct Coord(u32, u32, u32); #[derive(PartialEq)] struct Coord2(u32, u32, u32); -#[pyclass(str = "X: {aaaa}, Y: {y}, Z: {z}")] +#[pyclass(str = "X: {aaaa}, Y: {y}, Z: {z}", skip_from_py_object)] #[derive(PartialEq, Eq, Clone, PartialOrd)] pub struct Point { x: i32, @@ -118,7 +118,7 @@ pub struct Point { z: i32, } -#[pyclass(str = "X: {x}, Y: {y}}}, Z: {zzz}")] +#[pyclass(str = "X: {x}, Y: {y}}}, Z: {zzz}", skip_from_py_object)] #[derive(PartialEq, Eq, Clone, PartialOrd)] pub struct Point2 { x: i32, @@ -193,4 +193,11 @@ struct StructFromPyObjectNoClone { b: String, } +#[pyclass] +#[derive(Clone)] +struct StructImplictFromPyObjectDeprecated { + a: String, + b: String, +} + fn main() {} diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 98aa1a09118..8402451ec44 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -371,7 +371,7 @@ note: candidate #2 is defined in an impl for the type `StrOptAndManualStr` error[E0609]: no field `aaaa` on type `&Point` --> tests/ui/invalid_pyclass_args.rs:113:17 | -113 | #[pyclass(str = "X: {aaaa}, Y: {y}, Z: {z}")] +113 | #[pyclass(str = "X: {aaaa}, Y: {y}, Z: {z}", skip_from_py_object)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown field | = note: available fields are: `x`, `y`, `z` @@ -379,7 +379,7 @@ error[E0609]: no field `aaaa` on type `&Point` error[E0609]: no field `zzz` on type `&Point2` --> tests/ui/invalid_pyclass_args.rs:121:17 | -121 | #[pyclass(str = "X: {x}, Y: {y}}}, Z: {zzz}")] +121 | #[pyclass(str = "X: {x}, Y: {y}}}, Z: {zzz}", skip_from_py_object)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown field | = note: available fields are: `x`, `y`, `z` @@ -391,3 +391,12 @@ error[E0609]: no field `162543` on type `&Coord3` | ^^^^^^^^^^^^^^^^^^^^ unknown field | = note: available fields are: `0`, `1`, `2` + +warning: use of deprecated associated constant `pyo3::impl_::deprecated::HasAutomaticFromPyObject::::MSG`: 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. + --> tests/ui/invalid_pyclass_args.rs:196:1 + | +196 | #[pyclass] + | ^^^^^^^^^^ + | + = note: `#[warn(deprecated)]` on by default + = note: this warning originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)