From ace2f1e0a8f5a9247b2051cfb2095700f59fccb6 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 8 Nov 2024 11:31:28 +0000 Subject: [PATCH 1/4] silence deprecation warning for enum with custom `__eq__` --- newsfragments/4692.fixed.md | 1 + pyo3-macros-backend/src/pyclass.rs | 7 +---- src/impl_/pyclass.rs | 45 +++++++++++++++++++++++++++++ tests/test_enum.rs | 46 ++++++++++++++++++++++++++++++ tests/ui/deprecations.stderr | 2 +- 5 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 newsfragments/4692.fixed.md diff --git a/newsfragments/4692.fixed.md b/newsfragments/4692.fixed.md new file mode 100644 index 00000000000..a5dc6d098cf --- /dev/null +++ b/newsfragments/4692.fixed.md @@ -0,0 +1 @@ +Fix incorrect deprecation warning for `#[pyclass] enum`s with custom `__eq__` implementation. diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index ae3082e3785..9bb94e00d6f 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1908,12 +1908,7 @@ fn pyclass_richcmp_simple_enum( let deprecation = (options.eq_int.is_none() && options.eq.is_none()) .then(|| { quote! { - #[deprecated( - since = "0.22.0", - note = "Implicit equality for simple enums is deprecated. Use `#[pyclass(eq, eq_int)]` to keep the current behavior." - )] - const DEPRECATION: () = (); - const _: () = DEPRECATION; + let _ = #pyo3_path::impl_::pyclass::DeprecationTest::<#cls>::new().autogenerated_equality(); } }) .unwrap_or_default(); diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index d5f292f9ebd..98880871099 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -878,6 +878,8 @@ macro_rules! generate_pyclass_richcompare_slot { other: *mut $crate::ffi::PyObject, op: ::std::os::raw::c_int, ) -> *mut $crate::ffi::PyObject { + impl $crate::impl_::pyclass::HasCustomRichCmp for $cls {} + $crate::impl_::trampoline::richcmpfunc(slf, other, op, |py, slf, other, op| { use $crate::class::basic::CompareOp; use $crate::impl_::pyclass::*; @@ -1519,6 +1521,49 @@ fn pyo3_get_value< Ok((unsafe { &*value }).clone().into_py(py).into_ptr()) } +/// Marker trait whether a class implemented a custom comparison. Used to +/// silence deprecation of autogenerated `__richcmp__` for enums. +pub trait HasCustomRichCmp {} + +/// Autoref specialization setup to emit deprecation warnings for autogenerated +/// pyclass equality. +pub struct DeprecationTest(Deprecation, ::std::marker::PhantomData); +pub struct Deprecation; + +impl DeprecationTest { + #[inline] + pub const fn new() -> Self { + DeprecationTest(Deprecation, ::std::marker::PhantomData) + } +} + +impl std::ops::Deref for DeprecationTest { + type Target = Deprecation; + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DeprecationTest +where + T: HasCustomRichCmp, +{ + /// For `HasCustomRichCmp` types; no deprecation warning. + #[inline] + pub fn autogenerated_equality(&self) {} +} + +impl Deprecation { + #[deprecated( + since = "0.22.0", + note = "Implicit equality for simple enums is deprecated. Use `#[pyclass(eq, eq_int)]` to keep the current behavior." + )] + /// For types which don't implement `HasCustomRichCmp`; emits deprecation warning. + #[inline] + pub fn autogenerated_equality(&self) {} +} + #[cfg(test)] #[cfg(feature = "macros")] mod tests { diff --git a/tests/test_enum.rs b/tests/test_enum.rs index 9cb9d5fae65..c0a8f8b1e35 100644 --- a/tests/test_enum.rs +++ b/tests/test_enum.rs @@ -2,6 +2,7 @@ use pyo3::prelude::*; use pyo3::py_run; +use pyo3::types::PyString; #[path = "../src/tests/common.rs"] mod common; @@ -357,3 +358,48 @@ mod deprecated { }) } } + +#[test] +fn custom_eq() { + #[pyclass(frozen)] + #[derive(PartialEq)] + pub enum CustomPyEq { + A, + B, + } + + #[pymethods] + impl CustomPyEq { + fn __eq__(&self, other: &Bound<'_, PyAny>) -> bool { + if let Ok(rhs) = other.downcast::() { + rhs.to_cow().map_or(false, |rhs| self.__str__() == rhs) + } else if let Ok(rhs) = other.downcast::() { + self == rhs.get() + } else { + false + } + } + + fn __str__(&self) -> String { + match self { + CustomPyEq::A => "A".to_string(), + CustomPyEq::B => "B".to_string(), + } + } + } + + Python::with_gil(|py| { + let a = Bound::new(py, CustomPyEq::A).unwrap(); + let b = Bound::new(py, CustomPyEq::B).unwrap(); + + assert!(a.as_any().eq(&a).unwrap()); + assert!(a.as_any().eq("A").unwrap()); + assert!(a.as_any().ne(&b).unwrap()); + assert!(a.as_any().ne("B").unwrap()); + + assert!(b.as_any().eq(&b).unwrap()); + assert!(b.as_any().eq("B").unwrap()); + assert!(b.as_any().ne(&a).unwrap()); + assert!(b.as_any().ne("A").unwrap()); + }) +} diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index 567f5697c7f..6236dc55631 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -28,7 +28,7 @@ error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`: 21 | fn pyfunction_option_4( | ^^^^^^^^^^^^^^^^^^^ -error: use of deprecated constant `SimpleEnumWithoutEq::__pyo3__generated____richcmp__::DEPRECATION`: Implicit equality for simple enums is deprecated. Use `#[pyclass(eq, eq_int)]` to keep the current behavior. +error: use of deprecated method `pyo3::impl_::pyclass::Deprecation::autogenerated_equality`: Implicit equality for simple enums is deprecated. Use `#[pyclass(eq, eq_int)]` to keep the current behavior. --> tests/ui/deprecations.rs:28:1 | 28 | #[pyclass] From 808676302d5886511fa00a684de54c19fe9d7830 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 8 Nov 2024 13:57:53 +0000 Subject: [PATCH 2/4] clippy --- src/impl_/pyclass.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 98880871099..c947df6e432 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -1532,6 +1532,7 @@ pub struct Deprecation; impl DeprecationTest { #[inline] + #[allow(clippy::new_without_default)] pub const fn new() -> Self { DeprecationTest(Deprecation, ::std::marker::PhantomData) } From 6f15625de1e0655579e66f73bf7688cd8590eb13 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 8 Nov 2024 21:12:58 +0000 Subject: [PATCH 3/4] also support `__richcmp__` --- pyo3-macros-backend/src/pymethod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 1254a8d510b..afdfd13a1f5 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -1348,6 +1348,11 @@ impl SlotDef { )?; let name = spec.name; let holders = holders.init_holders(ctx); + let dep = if method_name == "__richcmp__" { + quote! { impl #pyo3_path::impl_::pyclass::HasCustomRichCmp for #cls {} } + } else { + TokenStream::default() + }; let associated_method = quote! { #[allow(non_snake_case)] unsafe fn #wrapper_ident( @@ -1355,6 +1360,7 @@ impl SlotDef { _raw_slf: *mut #pyo3_path::ffi::PyObject, #(#arg_idents: #arg_types),* ) -> #pyo3_path::PyResult<#ret_ty> { + #dep let function = #cls::#name; // Shadow the method name to avoid #3017 let _slf = _raw_slf; #holders From c808598f91076a69c8a13d7efbfd0740a54a276c Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sat, 9 Nov 2024 15:41:24 +0100 Subject: [PATCH 4/4] fix ci --- pyo3-macros-backend/src/pymethod.rs | 5 ++++- tests/ui/invalid_proto_pymethods.stderr | 11 +++++++++++ tests/ui/invalid_pyclass_args.stderr | 11 +++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index afdfd13a1f5..560c3c9dcc1 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -1349,7 +1349,10 @@ impl SlotDef { let name = spec.name; let holders = holders.init_holders(ctx); let dep = if method_name == "__richcmp__" { - quote! { impl #pyo3_path::impl_::pyclass::HasCustomRichCmp for #cls {} } + quote! { + #[allow(unknown_lints, non_local_definitions)] + impl #pyo3_path::impl_::pyclass::HasCustomRichCmp for #cls {} + } } else { TokenStream::default() }; diff --git a/tests/ui/invalid_proto_pymethods.stderr b/tests/ui/invalid_proto_pymethods.stderr index 82c99c2ddc3..18c96113299 100644 --- a/tests/ui/invalid_proto_pymethods.stderr +++ b/tests/ui/invalid_proto_pymethods.stderr @@ -40,6 +40,17 @@ note: candidate #2 is defined in an impl for the type `EqAndRichcmp` | ^^^^^^^^^^^^ = note: this error originates in the macro `::pyo3::impl_::pyclass::generate_pyclass_richcompare_slot` which comes from the expansion of the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) +error[E0119]: conflicting implementations of trait `HasCustomRichCmp` for type `EqAndRichcmp` + --> tests/ui/invalid_proto_pymethods.rs:55:1 + | +55 | #[pymethods] + | ^^^^^^^^^^^^ + | | + | first implementation here + | conflicting implementation for `EqAndRichcmp` + | + = note: this error originates in the macro `::pyo3::impl_::pyclass::generate_pyclass_richcompare_slot` which comes from the expansion of the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0592]: duplicate definitions with name `__pymethod___richcmp____` --> tests/ui/invalid_proto_pymethods.rs:55:1 | diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 15aa0387cc6..d1335e0f1a1 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -162,6 +162,17 @@ error: The format string syntax cannot be used with enums 171 | #[pyclass(eq, str = "Stuff...")] | ^^^^^^^^^^ +error[E0119]: conflicting implementations of trait `HasCustomRichCmp` for type `EqOptAndManualRichCmp` + --> tests/ui/invalid_pyclass_args.rs:41:1 + | +37 | #[pyclass(eq)] + | -------------- first implementation here +... +41 | #[pymethods] + | ^^^^^^^^^^^^ conflicting implementation for `EqOptAndManualRichCmp` + | + = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0592]: duplicate definitions with name `__pymethod___richcmp____` --> tests/ui/invalid_pyclass_args.rs:37:1 |