Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch deprecated implicit eq for simple enums #4730

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4730.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Removed the deprecated implicit eq fallback for simple enums.
17 changes: 1 addition & 16 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1903,24 +1903,11 @@ fn pyclass_richcmp_simple_enum(
ensure_spanned!(options.eq.is_some(), eq_int.span() => "The `eq_int` option requires the `eq` option.");
}

let deprecation = (options.eq_int.is_none() && options.eq.is_none())
.then(|| {
quote! {
let _ = #pyo3_path::impl_::pyclass::DeprecationTest::<#cls>::new().autogenerated_equality();
}
})
.unwrap_or_default();

let mut options = options.clone();
if options.eq.is_none() {
options.eq_int = Some(parse_quote!(eq_int));
}

if options.eq.is_none() && options.eq_int.is_none() {
return Ok((None, None));
}

let arms = pyclass_richcmp_arms(&options, ctx)?;
let arms = pyclass_richcmp_arms(options, ctx)?;

let eq = options.eq.map(|eq| {
quote_spanned! { eq.span() =>
Expand Down Expand Up @@ -1954,8 +1941,6 @@ fn pyclass_richcmp_simple_enum(
other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>,
op: #pyo3_path::pyclass::CompareOp
) -> #pyo3_path::PyResult<#pyo3_path::PyObject> {
#deprecation

#eq

#eq_int
Expand Down
9 changes: 0 additions & 9 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,22 +1348,13 @@ 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: 0 additions & 46 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,6 @@ 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 @@ -1546,50 +1544,6 @@ impl<const IMPLEMENTS_INTOPYOBJECT: bool> ConvertField<false, IMPLEMENTS_INTOPYO
}
}

/// 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
85 changes: 25 additions & 60 deletions tests/test_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,27 @@ fn test_enum_class_attr() {
})
}

#[test]
fn test_enum_eq_enum() {
Python::with_gil(|py| {
let var1 = Py::new(py, MyEnum::Variant).unwrap();
let var2 = Py::new(py, MyEnum::Variant).unwrap();
let other_var = Py::new(py, MyEnum::OtherVariant).unwrap();
py_assert!(py, var1 var2, "var1 == var2");
py_assert!(py, var1 other_var, "var1 != other_var");
py_assert!(py, var1 var2, "(var1 != var2) == False");
})
}

#[test]
fn test_enum_eq_incomparable() {
Python::with_gil(|py| {
let var1 = Py::new(py, MyEnum::Variant).unwrap();
py_assert!(py, var1, "(var1 == 'foo') == False");
py_assert!(py, var1, "(var1 != 'foo') == True");
})
}

#[pyfunction]
fn return_enum() -> MyEnum {
MyEnum::Variant
Expand Down Expand Up @@ -70,7 +91,11 @@ fn test_custom_discriminant() {
py_run!(py, CustomDiscriminant one two, r#"
assert CustomDiscriminant.One == one
assert CustomDiscriminant.Two == two
assert CustomDiscriminant.One == 1
assert CustomDiscriminant.Two == 2
assert one != two
assert CustomDiscriminant.One != 2
assert CustomDiscriminant.Two != 1
"#);
})
}
Expand Down Expand Up @@ -300,66 +325,6 @@ fn test_complex_enum_with_hash() {
});
}

#[allow(deprecated)]
mod deprecated {
use crate::py_assert;
use pyo3::prelude::*;
use pyo3::py_run;

#[pyclass]
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum MyEnum {
Variant,
OtherVariant,
}

#[test]
fn test_enum_eq_enum() {
Python::with_gil(|py| {
let var1 = Py::new(py, MyEnum::Variant).unwrap();
let var2 = Py::new(py, MyEnum::Variant).unwrap();
let other_var = Py::new(py, MyEnum::OtherVariant).unwrap();
py_assert!(py, var1 var2, "var1 == var2");
py_assert!(py, var1 other_var, "var1 != other_var");
py_assert!(py, var1 var2, "(var1 != var2) == False");
})
}

#[test]
fn test_enum_eq_incomparable() {
Python::with_gil(|py| {
let var1 = Py::new(py, MyEnum::Variant).unwrap();
py_assert!(py, var1, "(var1 == 'foo') == False");
py_assert!(py, var1, "(var1 != 'foo') == True");
})
}

#[pyclass]
enum CustomDiscriminant {
One = 1,
Two = 2,
}

#[test]
fn test_custom_discriminant() {
Python::with_gil(|py| {
#[allow(non_snake_case)]
let CustomDiscriminant = py.get_type::<CustomDiscriminant>();
let one = Py::new(py, CustomDiscriminant::One).unwrap();
let two = Py::new(py, CustomDiscriminant::Two).unwrap();
py_run!(py, CustomDiscriminant one two, r#"
assert CustomDiscriminant.One == one
assert CustomDiscriminant.Two == two
assert CustomDiscriminant.One == 1
assert CustomDiscriminant.Two == 2
assert one != two
assert CustomDiscriminant.One != 2
assert CustomDiscriminant.Two != 1
"#);
})
}
}

#[test]
fn custom_eq() {
#[pyclass(frozen)]
Expand Down
6 changes: 0 additions & 6 deletions tests/ui/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,4 @@ fn pyfunction_option_4(
) {
}

#[pyclass]
pub enum SimpleEnumWithoutEq {
VariamtA,
VariantB,
}

fn main() {}
8 changes: 0 additions & 8 deletions tests/ui/deprecations.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,3 @@ error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`:
|
21 | fn pyfunction_option_4(
| ^^^^^^^^^^^^^^^^^^^

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]
| ^^^^^^^^^^
|
= note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)
11 changes: 0 additions & 11 deletions tests/ui/invalid_proto_pymethods.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@ 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: 0 additions & 11 deletions tests/ui/invalid_pyclass_args.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,6 @@ 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
Loading