Skip to content

Commit

Permalink
silence deprecation warning for enum with custom __eq__ (#4692)
Browse files Browse the repository at this point in the history
* silence deprecation warning for enum with custom `__eq__`

* clippy

* also support `__richcmp__`

* fix ci

---------

Co-authored-by: Icxolu <[email protected]>
  • Loading branch information
davidhewitt and Icxolu authored Nov 9, 2024
1 parent de709e5 commit 3a6296e
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 7 deletions.
1 change: 1 addition & 0 deletions newsfragments/4692.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix incorrect deprecation warning for `#[pyclass] enum`s with custom `__eq__` implementation.
7 changes: 1 addition & 6 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
9 changes: 9 additions & 0 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,13 +1348,22 @@ impl SlotDef {
)?;
let name = spec.name;
let holders = holders.init_holders(ctx);
let dep = if method_name == "__richcmp__" {
quote! {
#[allow(unknown_lints, non_local_definitions)]
impl #pyo3_path::impl_::pyclass::HasCustomRichCmp for #cls {}
}
} else {
TokenStream::default()
};
let associated_method = quote! {
#[allow(non_snake_case)]
unsafe fn #wrapper_ident(
py: #pyo3_path::Python<'_>,
_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
Expand Down
46 changes: 46 additions & 0 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -1519,6 +1521,50 @@ 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<T>(Deprecation, ::std::marker::PhantomData<T>);
pub struct Deprecation;

impl<T> DeprecationTest<T> {
#[inline]
#[allow(clippy::new_without_default)]
pub const fn new() -> Self {
DeprecationTest(Deprecation, ::std::marker::PhantomData)
}
}

impl<T> std::ops::Deref for DeprecationTest<T> {
type Target = Deprecation;
#[inline]
fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T> DeprecationTest<T>
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 {
Expand Down
46 changes: 46 additions & 0 deletions tests/test_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use pyo3::prelude::*;
use pyo3::py_run;
use pyo3::types::PyString;

#[path = "../src/tests/common.rs"]
mod common;
Expand Down Expand Up @@ -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::<PyString>() {
rhs.to_cow().map_or(false, |rhs| self.__str__() == rhs)
} else if let Ok(rhs) = other.downcast::<Self>() {
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());
})
}
2 changes: 1 addition & 1 deletion tests/ui/deprecations.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/invalid_proto_pymethods.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/invalid_pyclass_args.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down

0 comments on commit 3a6296e

Please sign in to comment.