From 528b36392ede4940cd14da75e760d70b5ba1c7ba Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 26 May 2024 17:30:58 +0200 Subject: [PATCH 1/6] add pyclass `eq` option --- guide/pyclass-parameters.md | 1 + guide/src/class.md | 23 ++++--- guide/src/class/object.md | 10 +++ pyo3-macros-backend/src/attributes.rs | 1 + pyo3-macros-backend/src/pyclass.rs | 94 +++++++++++++++++++++++++-- pytests/src/enums.rs | 3 +- src/tests/hygiene/pyclass.rs | 3 +- tests/test_declarative_module.rs | 3 +- tests/test_default_impls.rs | 6 +- tests/test_enum.rs | 27 +++++--- tests/ui/deprecations.rs | 6 ++ tests/ui/deprecations.stderr | 8 +++ tests/ui/invalid_pyclass_args.rs | 3 + tests/ui/invalid_pyclass_args.stderr | 38 ++++++++++- tests/ui/invalid_pyclass_enum.rs | 12 ++++ tests/ui/invalid_pyclass_enum.stderr | 68 +++++++++++++++++++ 16 files changed, 276 insertions(+), 30 deletions(-) diff --git a/guide/pyclass-parameters.md b/guide/pyclass-parameters.md index 9bd0534ea5d..dccde5579fb 100644 --- a/guide/pyclass-parameters.md +++ b/guide/pyclass-parameters.md @@ -5,6 +5,7 @@ | `constructor` | This is currently only allowed on [variants of complex enums][params-constructor]. It allows customization of the generated class constructor for each variant. It uses the same syntax and supports the same options as the `signature` attribute of functions and methods. | | `crate = "some::path"` | Path to import the `pyo3` crate, if it's not accessible at `::pyo3`. | | `dict` | Gives instances of this class an empty `__dict__` to store custom attributes. | +| `eq` | Implements `__eq__` using the `PartialEq` implementation of the underlying Rust datatype. | | `extends = BaseType` | Use a custom baseclass. Defaults to [`PyAny`][params-1] | | `freelist = N` | Implements a [free list][params-2] of size N. This can improve performance for types that are often created and deleted in quick succession. Profile your code to see whether `freelist` is right for you. | | `frozen` | Declares that your pyclass is immutable. It removes the borrow checker overhead when retrieving a shared reference to the Rust struct, but disables the ability to get a mutable reference. | diff --git a/guide/src/class.md b/guide/src/class.md index 57a5cf6d467..c1164f19a24 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -37,14 +37,16 @@ struct Number(i32); // PyO3 supports unit-only enums (which contain only unit variants) // These simple enums behave similarly to Python's enumerations (enum.Enum) -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum MyEnum { Variant, OtherVariant = 30, // PyO3 supports custom discriminants. } // PyO3 supports custom discriminants in unit-only enums -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum HttpResponse { Ok = 200, NotFound = 404, @@ -1053,7 +1055,8 @@ PyO3 adds a class attribute for each variant, so you can access them in Python w ```rust # use pyo3::prelude::*; -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum MyEnum { Variant, OtherVariant, @@ -1075,7 +1078,8 @@ You can also convert your simple enums into `int`: ```rust # use pyo3::prelude::*; -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum MyEnum { Variant, OtherVariant = 10, @@ -1087,8 +1091,6 @@ Python::with_gil(|py| { pyo3::py_run!(py, cls x, r#" assert int(cls.Variant) == x assert int(cls.OtherVariant) == 10 - assert cls.OtherVariant == 10 # You can also compare against int. - assert 10 == cls.OtherVariant "#) }) ``` @@ -1097,7 +1099,8 @@ PyO3 also provides `__repr__` for enums: ```rust # use pyo3::prelude::*; -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum MyEnum{ Variant, OtherVariant, @@ -1117,7 +1120,8 @@ All methods defined by PyO3 can be overridden. For example here's how you overri ```rust # use pyo3::prelude::*; -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum MyEnum { Answer = 42, } @@ -1139,7 +1143,8 @@ Enums and their variants can also be renamed using `#[pyo3(name)]`. ```rust # use pyo3::prelude::*; -#[pyclass(name = "RenamedEnum")] +#[pyclass(eq, name = "RenamedEnum")] +#[derive(PartialEq)] enum MyEnum { #[pyo3(name = "UPPERCASE")] Variant, diff --git a/guide/src/class/object.md b/guide/src/class/object.md index db6cc7d3234..59e368d13df 100644 --- a/guide/src/class/object.md +++ b/guide/src/class/object.md @@ -226,6 +226,16 @@ impl Number { # } ``` +To implement `__eq__` in terms of the `PartialEq` implementation of the underlying datatype, the `eq` option can be used. + +```rust +# use pyo3::prelude::*; +# +#[pyclass(eq)] +#[derive(PartialEq)] +struct Number(i32); +``` + ### Truthyness We'll consider `Number` to be `True` if it is nonzero: diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs index d9c805aa3fa..f79072dc280 100644 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -14,6 +14,7 @@ pub mod kw { syn::custom_keyword!(cancel_handle); syn::custom_keyword!(constructor); syn::custom_keyword!(dict); + syn::custom_keyword!(eq); syn::custom_keyword!(extends); syn::custom_keyword!(freelist); syn::custom_keyword!(from_py_with); diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 47c52c84518..110be695ac2 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -18,7 +18,7 @@ use crate::utils::Ctx; use crate::utils::{self, apply_renaming_rule, PythonDoc}; use crate::PyFunctionOptions; use proc_macro2::{Ident, Span, TokenStream}; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, quote_spanned}; use syn::ext::IdentExt; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; @@ -59,6 +59,7 @@ impl PyClassArgs { pub struct PyClassPyO3Options { pub krate: Option, pub dict: Option, + pub eq: Option, pub extends: Option, pub get_all: Option, pub freelist: Option, @@ -77,6 +78,7 @@ pub struct PyClassPyO3Options { enum PyClassPyO3Option { Crate(CrateAttribute), Dict(kw::dict), + Eq(kw::eq), Extends(ExtendsAttribute), Freelist(FreelistAttribute), Frozen(kw::frozen), @@ -99,6 +101,8 @@ impl Parse for PyClassPyO3Option { input.parse().map(PyClassPyO3Option::Crate) } else if lookahead.peek(kw::dict) { input.parse().map(PyClassPyO3Option::Dict) + } else if lookahead.peek(kw::eq) { + input.parse().map(PyClassPyO3Option::Eq) } else if lookahead.peek(kw::extends) { input.parse().map(PyClassPyO3Option::Extends) } else if lookahead.peek(attributes::kw::freelist) { @@ -166,6 +170,7 @@ impl PyClassPyO3Options { match option { PyClassPyO3Option::Crate(krate) => set_option!(krate), PyClassPyO3Option::Dict(dict) => set_option!(dict), + PyClassPyO3Option::Eq(eq) => set_option!(eq), PyClassPyO3Option::Extends(extends) => set_option!(extends), PyClassPyO3Option::Freelist(freelist) => set_option!(freelist), PyClassPyO3Option::Frozen(frozen) => set_option!(frozen), @@ -350,6 +355,12 @@ fn impl_class( let Ctx { pyo3_path } = ctx; let pytypeinfo_impl = impl_pytypeinfo(cls, args, None, ctx); + let (default_richcmp, default_richcmp_slot) = + pyclass_richcmp(&args.options, &syn::parse_quote!(#cls), ctx); + + let mut slots = Vec::new(); + slots.extend(default_richcmp_slot); + let py_class_impl = PyClassImplsBuilder::new( cls, args, @@ -361,7 +372,7 @@ fn impl_class( field_options, ctx, )?, - vec![], + slots, ) .doc(doc) .impl_all(ctx)?; @@ -372,6 +383,12 @@ fn impl_class( #pytypeinfo_impl #py_class_impl + + #[doc(hidden)] + #[allow(non_snake_case)] + impl #cls { + #default_richcmp + } }) } @@ -775,7 +792,7 @@ fn impl_simple_enum( (int_impl, int_slot) }; - let (default_richcmp, default_richcmp_slot) = { + let (old_default_richcmp, old_default_richcmp_slot) = { let mut richcmp_impl: syn::ImplItemFn = syn::parse_quote! { fn __pyo3__richcmp__( &self, @@ -786,6 +803,12 @@ fn impl_simple_enum( use #pyo3_path::conversion::ToPyObject; use #pyo3_path::types::PyAnyMethods; use ::core::result::Result::*; + #[deprecated( + since = "0.22.0", + note = "Implicit equality for simple enums in deprecated. Use `#[pyclass(eq)` to opt-in to the new behavior." + )] + const DEPRECATION: () = (); + const _: () = DEPRECATION; match op { #pyo3_path::basic::CompareOp::Eq => { let self_val = self.__pyo3__int__(); @@ -818,6 +841,15 @@ fn impl_simple_enum( (richcmp_impl, richcmp_slot) }; + let (default_richcmp, default_richcmp_slot) = { + let (item, slot) = pyclass_richcmp(&args.options, &ty, ctx); + + ( + item.unwrap_or(old_default_richcmp), + slot.unwrap_or(old_default_richcmp_slot), + ) + }; + let default_slots = vec![default_repr_slot, default_int_slot, default_richcmp_slot]; let pyclass_impls = PyClassImplsBuilder::new( @@ -857,6 +889,8 @@ fn impl_complex_enum( ctx: &Ctx, ) -> Result { let Ctx { pyo3_path } = ctx; + let cls = complex_enum.ident; + let ty: syn::Type = syn::parse_quote!(#cls); // Need to rig the enum PyClass options let args = { @@ -873,7 +907,10 @@ fn impl_complex_enum( let variants = complex_enum.variants; let pytypeinfo = impl_pytypeinfo(cls, &args, None, ctx); - let default_slots = vec![]; + let (default_richcmp, default_richcmp_slot) = pyclass_richcmp(&args.options, &ty, ctx); + + let mut default_slots = vec![]; + default_slots.extend(default_richcmp_slot); let impl_builder = PyClassImplsBuilder::new( cls, @@ -978,7 +1015,9 @@ fn impl_complex_enum( #[doc(hidden)] #[allow(non_snake_case)] - impl #cls {} + impl #cls { + #default_richcmp + } #(#variant_cls_zsts)* @@ -1637,6 +1676,51 @@ fn impl_pytypeinfo( } } +fn pyclass_richcmp( + options: &PyClassPyO3Options, + cls: &syn::Type, + ctx: &Ctx, +) -> (Option, Option) { + let Ctx { pyo3_path } = ctx; + + let eq_arms = options + .eq + .map(|eq| { + quote_spanned! { eq.span() => + #pyo3_path::pyclass::CompareOp::Eq => { + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self == other, py)) + }, + #pyo3_path::pyclass::CompareOp::Ne => { + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self != other, py)) + }, + } + }) + .unwrap_or_default(); + + // TODO: `ord` can be integrated here (#4202) + if options.eq.is_some() { + let mut richcmp_impl = parse_quote! { + fn __pyo3__richcmp__( + &self, + py: #pyo3_path::Python, + other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, + op: #pyo3_path::pyclass::CompareOp + ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { + let other = &*#pyo3_path::types::PyAnyMethods::downcast::(other)?.borrow(); + match op { + #eq_arms + _ => ::std::result::Result::Ok(py.NotImplemented()) + } + } + }; + let richcmp_slot = + generate_default_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); + (Some(richcmp_impl), Some(richcmp_slot)) + } else { + (None, None) + } +} + /// Implements most traits used by `#[pyclass]`. /// /// Specifically, it implements traits that only depend on class name, diff --git a/pytests/src/enums.rs b/pytests/src/enums.rs index 964f0d431c3..4e32f73f4d4 100644 --- a/pytests/src/enums.rs +++ b/pytests/src/enums.rs @@ -18,7 +18,8 @@ pub fn enums(m: &Bound<'_, PyModule>) -> PyResult<()> { Ok(()) } -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] pub enum SimpleEnum { Sunday, Monday, diff --git a/src/tests/hygiene/pyclass.rs b/src/tests/hygiene/pyclass.rs index 34b30a8c6f4..6cead2541d5 100644 --- a/src/tests/hygiene/pyclass.rs +++ b/src/tests/hygiene/pyclass.rs @@ -29,8 +29,9 @@ pub struct Bar { c: ::std::option::Option>, } -#[crate::pyclass] +#[crate::pyclass(eq)] #[pyo3(crate = "crate")] +#[derive(PartialEq)] pub enum Enum { Var0, } diff --git a/tests/test_declarative_module.rs b/tests/test_declarative_module.rs index 2e46f4a64d1..0e5f578596b 100644 --- a/tests/test_declarative_module.rs +++ b/tests/test_declarative_module.rs @@ -89,7 +89,8 @@ mod declarative_module { } } - #[pyclass] + #[pyclass(eq)] + #[derive(PartialEq)] enum Enum { A, B, diff --git a/tests/test_default_impls.rs b/tests/test_default_impls.rs index 526f88e8f82..75152b0e339 100644 --- a/tests/test_default_impls.rs +++ b/tests/test_default_impls.rs @@ -6,7 +6,8 @@ use pyo3::prelude::*; mod common; // Test default generated __repr__. -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum TestDefaultRepr { Var, } @@ -23,7 +24,8 @@ fn test_default_slot_exists() { }) } -#[pyclass] +#[pyclass(eq)] +#[derive(PartialEq)] enum OverrideSlot { Var, } diff --git a/tests/test_enum.rs b/tests/test_enum.rs index 63492b8d3cd..d6b7a90fda9 100644 --- a/tests/test_enum.rs +++ b/tests/test_enum.rs @@ -6,7 +6,7 @@ use pyo3::py_run; #[path = "../src/tests/common.rs"] mod common; -#[pyclass] +#[pyclass(eq)] #[derive(Debug, PartialEq, Eq, Clone)] pub enum MyEnum { Variant, @@ -73,7 +73,8 @@ fn test_enum_eq_incomparable() { }) } -#[pyclass] +#[pyclass(eq)] +#[derive(Debug, PartialEq, Eq, Clone)] enum CustomDiscriminant { One = 1, Two = 2, @@ -121,7 +122,8 @@ fn test_enum_compare_int() { }) } -#[pyclass] +#[pyclass(eq)] +#[derive(Debug, PartialEq, Eq, Clone)] #[repr(u8)] enum SmallEnum { V = 1, @@ -135,7 +137,8 @@ fn test_enum_compare_int_no_throw_when_overflow() { }) } -#[pyclass] +#[pyclass(eq)] +#[derive(Debug, PartialEq, Eq, Clone)] #[repr(usize)] #[allow(clippy::enum_clike_unportable_variant)] enum BigEnum { @@ -145,14 +148,19 @@ enum BigEnum { #[test] fn test_big_enum_no_overflow() { Python::with_gil(|py| { + // use pyo3::types::IntoPyDict; let usize_max = usize::MAX; let v = Py::new(py, BigEnum::V).unwrap(); - py_assert!(py, usize_max v, "v == usize_max"); + + // let env = [("v", v.as_any()), ("usize_max", &usize_max.into_py(py))].into_py_dict_bound(py); + // py_run!(py, *env, "print(v)\nprint(usize_max)"); + // py_assert!(py, usize_max v, "v == usize_max"); py_assert!(py, usize_max v, "int(v) == usize_max"); }) } -#[pyclass] +#[pyclass(eq)] +#[derive(Debug, PartialEq, Eq, Clone)] #[repr(u16, align(8))] enum TestReprParse { V, @@ -163,7 +171,7 @@ fn test_repr_parse() { assert_eq!(std::mem::align_of::(), 8); } -#[pyclass(name = "MyEnum")] +#[pyclass(eq, name = "MyEnum")] #[derive(Debug, PartialEq, Eq, Clone)] pub enum RenameEnum { Variant, @@ -177,7 +185,7 @@ fn test_rename_enum_repr_correct() { }) } -#[pyclass] +#[pyclass(eq)] #[derive(Debug, PartialEq, Eq, Clone)] pub enum RenameVariantEnum { #[pyo3(name = "VARIANT")] @@ -192,7 +200,8 @@ fn test_rename_variant_repr_correct() { }) } -#[pyclass(rename_all = "SCREAMING_SNAKE_CASE")] +#[pyclass(eq, rename_all = "SCREAMING_SNAKE_CASE")] +#[derive(Debug, PartialEq, Eq, Clone)] #[allow(clippy::enum_variant_names)] enum RenameAllVariantsEnum { VariantOne, diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs index fc9e8687cae..dbd0f8aa462 100644 --- a/tests/ui/deprecations.rs +++ b/tests/ui/deprecations.rs @@ -196,3 +196,9 @@ fn test_wrap_pyfunction(py: Python<'_>, m: &Bound<'_, PyModule>) { let _ = wrap_pyfunction_bound!(double, py); let _ = wrap_pyfunction_bound!(double)(py); } + +#[pyclass] +pub enum SimpleEnumWithoutEq { + VariamtA, + VariantB, +} diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index d014a06bbcc..fbea2996350 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -34,6 +34,14 @@ error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`: 138 | fn pyfunction_option_4( | ^^^^^^^^^^^^^^^^^^^ +error: use of deprecated constant `SimpleEnumWithoutEq::__pyo3__richcmp__::DEPRECATION`: Implicit equality for simple enums in deprecated. Use `#[pyclass(eq)` to opt-in to the new behavior. + --> tests/ui/deprecations.rs:200:1 + | +200 | #[pyclass] + | ^^^^^^^^^^ + | + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) + error: use of deprecated struct `pyo3::PyCell`: `PyCell` was merged into `Bound`, use that instead; see the migration guide for more info --> tests/ui/deprecations.rs:23:30 | diff --git a/tests/ui/invalid_pyclass_args.rs b/tests/ui/invalid_pyclass_args.rs index fac21db078c..d8d0ffe7ad0 100644 --- a/tests/ui/invalid_pyclass_args.rs +++ b/tests/ui/invalid_pyclass_args.rs @@ -30,4 +30,7 @@ struct InvalidArg {} #[pyclass(mapping, sequence)] struct CannotBeMappingAndSequence {} +#[pyclass(eq)] +struct EqOptRequiresEq {} + fn main() {} diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 5b2bd24dd3a..50835afd5f3 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -1,4 +1,4 @@ -error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `eq`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:3:11 | 3 | #[pyclass(extend=pyo3::types::PyDict)] @@ -46,7 +46,7 @@ error: expected string literal 24 | #[pyclass(module = my_module)] | ^^^^^^^^^ -error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `eq`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:27:11 | 27 | #[pyclass(weakrev)] @@ -57,3 +57,37 @@ error: a `#[pyclass]` cannot be both a `mapping` and a `sequence` | 31 | struct CannotBeMappingAndSequence {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0369]: binary operation `==` cannot be applied to type `&EqOptRequiresEq` + --> tests/ui/invalid_pyclass_args.rs:33:11 + | +33 | #[pyclass(eq)] + | ^^ + | +note: an implementation of `PartialEq` might be missing for `EqOptRequiresEq` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct EqOptRequiresEq {} + | ^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` +help: consider annotating `EqOptRequiresEq` with `#[derive(PartialEq)]` + | +34 + #[derive(PartialEq)] +35 | struct EqOptRequiresEq {} + | + +error[E0369]: binary operation `!=` cannot be applied to type `&EqOptRequiresEq` + --> tests/ui/invalid_pyclass_args.rs:33:11 + | +33 | #[pyclass(eq)] + | ^^ + | +note: an implementation of `PartialEq` might be missing for `EqOptRequiresEq` + --> tests/ui/invalid_pyclass_args.rs:34:1 + | +34 | struct EqOptRequiresEq {} + | ^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` +help: consider annotating `EqOptRequiresEq` with `#[derive(PartialEq)]` + | +34 + #[derive(PartialEq)] +35 | struct EqOptRequiresEq {} + | diff --git a/tests/ui/invalid_pyclass_enum.rs b/tests/ui/invalid_pyclass_enum.rs index e98010fea32..2c6d1f2a011 100644 --- a/tests/ui/invalid_pyclass_enum.rs +++ b/tests/ui/invalid_pyclass_enum.rs @@ -28,4 +28,16 @@ enum SimpleNoSignature { B, } +#[pyclass(eq)] +enum SimpleEqOptRequiresPartialEq { + A, + B, +} + +#[pyclass(eq)] +enum ComplexEqOptRequiresPartialEq { + A(i32), + B { msg: String }, +} + fn main() {} diff --git a/tests/ui/invalid_pyclass_enum.stderr b/tests/ui/invalid_pyclass_enum.stderr index 7e3b6ffa425..948e34a1fb8 100644 --- a/tests/ui/invalid_pyclass_enum.stderr +++ b/tests/ui/invalid_pyclass_enum.stderr @@ -29,3 +29,71 @@ error: `constructor` can't be used on a simple enum variant | 26 | #[pyo3(constructor = (a, b))] | ^^^^^^^^^^^ + +error[E0369]: binary operation `==` cannot be applied to type `&SimpleEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:31:11 + | +31 | #[pyclass(eq)] + | ^^ + | +note: an implementation of `PartialEq` might be missing for `SimpleEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum SimpleEqOptRequiresPartialEq { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` +help: consider annotating `SimpleEqOptRequiresPartialEq` with `#[derive(PartialEq)]` + | +32 + #[derive(PartialEq)] +33 | enum SimpleEqOptRequiresPartialEq { + | + +error[E0369]: binary operation `!=` cannot be applied to type `&SimpleEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:31:11 + | +31 | #[pyclass(eq)] + | ^^ + | +note: an implementation of `PartialEq` might be missing for `SimpleEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:32:1 + | +32 | enum SimpleEqOptRequiresPartialEq { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` +help: consider annotating `SimpleEqOptRequiresPartialEq` with `#[derive(PartialEq)]` + | +32 + #[derive(PartialEq)] +33 | enum SimpleEqOptRequiresPartialEq { + | + +error[E0369]: binary operation `==` cannot be applied to type `&ComplexEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:37:11 + | +37 | #[pyclass(eq)] + | ^^ + | +note: an implementation of `PartialEq` might be missing for `ComplexEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:38:1 + | +38 | enum ComplexEqOptRequiresPartialEq { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` +help: consider annotating `ComplexEqOptRequiresPartialEq` with `#[derive(PartialEq)]` + | +38 + #[derive(PartialEq)] +39 | enum ComplexEqOptRequiresPartialEq { + | + +error[E0369]: binary operation `!=` cannot be applied to type `&ComplexEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:37:11 + | +37 | #[pyclass(eq)] + | ^^ + | +note: an implementation of `PartialEq` might be missing for `ComplexEqOptRequiresPartialEq` + --> tests/ui/invalid_pyclass_enum.rs:38:1 + | +38 | enum ComplexEqOptRequiresPartialEq { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ must implement `PartialEq` +help: consider annotating `ComplexEqOptRequiresPartialEq` with `#[derive(PartialEq)]` + | +38 + #[derive(PartialEq)] +39 | enum ComplexEqOptRequiresPartialEq { + | From 700e19b8952b3698c6d89541affe1af8c784dfe0 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 29 May 2024 19:20:29 +0200 Subject: [PATCH 2/6] prevent manual impl of `__richcmp__` or `__eq__` with `#[pyclass(eq)]` --- pyo3-macros-backend/src/pyclass.rs | 2 +- tests/ui/invalid_pyclass_args.rs | 16 ++++++++ tests/ui/invalid_pyclass_args.stderr | 56 ++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 110be695ac2..59dc8b74cf5 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1700,7 +1700,7 @@ fn pyclass_richcmp( // TODO: `ord` can be integrated here (#4202) if options.eq.is_some() { let mut richcmp_impl = parse_quote! { - fn __pyo3__richcmp__( + fn __pymethod___richcmp____( &self, py: #pyo3_path::Python, other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, diff --git a/tests/ui/invalid_pyclass_args.rs b/tests/ui/invalid_pyclass_args.rs index d8d0ffe7ad0..6fa91dad847 100644 --- a/tests/ui/invalid_pyclass_args.rs +++ b/tests/ui/invalid_pyclass_args.rs @@ -33,4 +33,20 @@ struct CannotBeMappingAndSequence {} #[pyclass(eq)] struct EqOptRequiresEq {} +#[pyclass(eq)] +#[derive(PartialEq)] +struct EqOptAndManualRichCmp {} + +#[pymethods] +impl EqOptAndManualRichCmp { + fn __richcmp__( + &self, + _py: Python, + _other: Bound<'_, PyAny>, + _op: pyo3::pyclass::CompareOp, + ) -> PyResult { + todo!() + } +} + fn main() {} diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 50835afd5f3..85f4cc1c19d 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -58,6 +58,17 @@ error: a `#[pyclass]` cannot be both a `mapping` and a `sequence` 31 | struct CannotBeMappingAndSequence {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +error[E0592]: duplicate definitions with name `__pymethod___richcmp____` + --> tests/ui/invalid_pyclass_args.rs:36:1 + | +36 | #[pyclass(eq)] + | ^^^^^^^^^^^^^^ duplicate definitions for `__pymethod___richcmp____` +... +40 | #[pymethods] + | ------------ other definition for `__pymethod___richcmp____` + | + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0369]: binary operation `==` cannot be applied to type `&EqOptRequiresEq` --> tests/ui/invalid_pyclass_args.rs:33:11 | @@ -91,3 +102,48 @@ help: consider annotating `EqOptRequiresEq` with `#[derive(PartialEq)]` 34 + #[derive(PartialEq)] 35 | struct EqOptRequiresEq {} | + +error[E0034]: multiple applicable items in scope + --> tests/ui/invalid_pyclass_args.rs:36:1 + | +36 | #[pyclass(eq)] + | ^^^^^^^^^^^^^^ multiple `__pymethod___richcmp____` found + | +note: candidate #1 is defined in an impl for the type `EqOptAndManualRichCmp` + --> tests/ui/invalid_pyclass_args.rs:36:1 + | +36 | #[pyclass(eq)] + | ^^^^^^^^^^^^^^ +note: candidate #2 is defined in an impl for the type `EqOptAndManualRichCmp` + --> tests/ui/invalid_pyclass_args.rs:40:1 + | +40 | #[pymethods] + | ^^^^^^^^^^^^ + = note: this error originates in the attribute macro `pyclass` which comes from the expansion of the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument + --> tests/ui/invalid_pyclass_args.rs:36:1 + | +36 | #[pyclass(eq)] + | ^^^^^^^^^^^^^^ + | + = 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) + +error[E0034]: multiple applicable items in scope + --> tests/ui/invalid_pyclass_args.rs:40:1 + | +40 | #[pymethods] + | ^^^^^^^^^^^^ multiple `__pymethod___richcmp____` found + | +note: candidate #1 is defined in an impl for the type `EqOptAndManualRichCmp` + --> tests/ui/invalid_pyclass_args.rs:36:1 + | +36 | #[pyclass(eq)] + | ^^^^^^^^^^^^^^ +note: candidate #2 is defined in an impl for the type `EqOptAndManualRichCmp` + --> tests/ui/invalid_pyclass_args.rs:40:1 + | +40 | #[pymethods] + | ^^^^^^^^^^^^ + = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) From cd006c80bcf693d0f1dc0001fd7592f756f5e56d Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 29 May 2024 20:49:50 +0200 Subject: [PATCH 3/6] add simple enum `eq_int` option --- guide/pyclass-parameters.md | 1 + guide/src/class.md | 14 +- pyo3-macros-backend/src/attributes.rs | 1 + pyo3-macros-backend/src/pyclass.rs | 183 ++++++++++++++++---------- pytests/src/enums.rs | 2 +- src/tests/hygiene/pyclass.rs | 2 +- tests/test_declarative_module.rs | 2 +- tests/test_default_impls.rs | 4 +- tests/test_enum.rs | 21 ++- tests/ui/deprecations.stderr | 2 +- tests/ui/invalid_pyclass_args.rs | 3 + tests/ui/invalid_pyclass_args.stderr | 10 +- tests/ui/invalid_pyclass_enum.rs | 8 +- tests/ui/invalid_pyclass_enum.stderr | 10 +- 14 files changed, 162 insertions(+), 101 deletions(-) diff --git a/guide/pyclass-parameters.md b/guide/pyclass-parameters.md index dccde5579fb..77750e36cdf 100644 --- a/guide/pyclass-parameters.md +++ b/guide/pyclass-parameters.md @@ -6,6 +6,7 @@ | `crate = "some::path"` | Path to import the `pyo3` crate, if it's not accessible at `::pyo3`. | | `dict` | Gives instances of this class an empty `__dict__` to store custom attributes. | | `eq` | Implements `__eq__` using the `PartialEq` implementation of the underlying Rust datatype. | +| `eq_int` | Implements `__eq__` using `__int__` for simple enums. | | `extends = BaseType` | Use a custom baseclass. Defaults to [`PyAny`][params-1] | | `freelist = N` | Implements a [free list][params-2] of size N. This can improve performance for types that are often created and deleted in quick succession. Profile your code to see whether `freelist` is right for you. | | `frozen` | Declares that your pyclass is immutable. It removes the borrow checker overhead when retrieving a shared reference to the Rust struct, but disables the ability to get a mutable reference. | diff --git a/guide/src/class.md b/guide/src/class.md index c1164f19a24..b72cae34e25 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -37,7 +37,7 @@ struct Number(i32); // PyO3 supports unit-only enums (which contain only unit variants) // These simple enums behave similarly to Python's enumerations (enum.Enum) -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum MyEnum { Variant, @@ -45,7 +45,7 @@ enum MyEnum { } // PyO3 supports custom discriminants in unit-only enums -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum HttpResponse { Ok = 200, @@ -1055,7 +1055,7 @@ PyO3 adds a class attribute for each variant, so you can access them in Python w ```rust # use pyo3::prelude::*; -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum MyEnum { Variant, @@ -1078,7 +1078,7 @@ You can also convert your simple enums into `int`: ```rust # use pyo3::prelude::*; -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum MyEnum { Variant, @@ -1099,7 +1099,7 @@ PyO3 also provides `__repr__` for enums: ```rust # use pyo3::prelude::*; -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum MyEnum{ Variant, @@ -1120,7 +1120,7 @@ All methods defined by PyO3 can be overridden. For example here's how you overri ```rust # use pyo3::prelude::*; -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum MyEnum { Answer = 42, @@ -1143,7 +1143,7 @@ Enums and their variants can also be renamed using `#[pyo3(name)]`. ```rust # use pyo3::prelude::*; -#[pyclass(eq, name = "RenamedEnum")] +#[pyclass(eq, eq_int, name = "RenamedEnum")] #[derive(PartialEq)] enum MyEnum { #[pyo3(name = "UPPERCASE")] diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs index f79072dc280..3bccf0ae3ee 100644 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -15,6 +15,7 @@ pub mod kw { syn::custom_keyword!(constructor); syn::custom_keyword!(dict); syn::custom_keyword!(eq); + syn::custom_keyword!(eq_int); syn::custom_keyword!(extends); syn::custom_keyword!(freelist); syn::custom_keyword!(from_py_with); diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 59dc8b74cf5..74f1835bfca 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -60,6 +60,7 @@ pub struct PyClassPyO3Options { pub krate: Option, pub dict: Option, pub eq: Option, + pub eq_int: Option, pub extends: Option, pub get_all: Option, pub freelist: Option, @@ -79,6 +80,7 @@ enum PyClassPyO3Option { Crate(CrateAttribute), Dict(kw::dict), Eq(kw::eq), + EqInt(kw::eq_int), Extends(ExtendsAttribute), Freelist(FreelistAttribute), Frozen(kw::frozen), @@ -103,6 +105,8 @@ impl Parse for PyClassPyO3Option { input.parse().map(PyClassPyO3Option::Dict) } else if lookahead.peek(kw::eq) { input.parse().map(PyClassPyO3Option::Eq) + } else if lookahead.peek(kw::eq_int) { + input.parse().map(PyClassPyO3Option::EqInt) } else if lookahead.peek(kw::extends) { input.parse().map(PyClassPyO3Option::Extends) } else if lookahead.peek(attributes::kw::freelist) { @@ -171,6 +175,7 @@ impl PyClassPyO3Options { PyClassPyO3Option::Crate(krate) => set_option!(krate), PyClassPyO3Option::Dict(dict) => set_option!(dict), PyClassPyO3Option::Eq(eq) => set_option!(eq), + PyClassPyO3Option::EqInt(eq_int) => set_option!(eq_int), PyClassPyO3Option::Extends(extends) => set_option!(extends), PyClassPyO3Option::Freelist(freelist) => set_option!(freelist), PyClassPyO3Option::Frozen(frozen) => set_option!(frozen), @@ -356,7 +361,7 @@ fn impl_class( let pytypeinfo_impl = impl_pytypeinfo(cls, args, None, ctx); let (default_richcmp, default_richcmp_slot) = - pyclass_richcmp(&args.options, &syn::parse_quote!(#cls), ctx); + pyclass_richcmp(&args.options, &syn::parse_quote!(#cls), ctx)?; let mut slots = Vec::new(); slots.extend(default_richcmp_slot); @@ -740,7 +745,6 @@ fn impl_simple_enum( methods_type: PyClassMethodsType, ctx: &Ctx, ) -> Result { - let Ctx { pyo3_path } = ctx; let cls = simple_enum.ident; let ty: syn::Type = syn::parse_quote!(#cls); let variants = simple_enum.variants; @@ -792,65 +796,11 @@ fn impl_simple_enum( (int_impl, int_slot) }; - let (old_default_richcmp, old_default_richcmp_slot) = { - let mut richcmp_impl: syn::ImplItemFn = syn::parse_quote! { - fn __pyo3__richcmp__( - &self, - py: #pyo3_path::Python, - other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, - op: #pyo3_path::basic::CompareOp - ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { - use #pyo3_path::conversion::ToPyObject; - use #pyo3_path::types::PyAnyMethods; - use ::core::result::Result::*; - #[deprecated( - since = "0.22.0", - note = "Implicit equality for simple enums in deprecated. Use `#[pyclass(eq)` to opt-in to the new behavior." - )] - const DEPRECATION: () = (); - const _: () = DEPRECATION; - match op { - #pyo3_path::basic::CompareOp::Eq => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val == i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val == other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); - } - #pyo3_path::basic::CompareOp::Ne => { - let self_val = self.__pyo3__int__(); - if let Ok(i) = other.extract::<#repr_type>() { - return Ok((self_val != i).to_object(py)); - } - if let Ok(other) = other.extract::<#pyo3_path::PyRef>() { - return Ok((self_val != other.__pyo3__int__()).to_object(py)); - } - - return Ok(py.NotImplemented()); - } - _ => Ok(py.NotImplemented()), - } - } - }; - let richcmp_slot = - generate_default_protocol_slot(&ty, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); - (richcmp_impl, richcmp_slot) - }; - - let (default_richcmp, default_richcmp_slot) = { - let (item, slot) = pyclass_richcmp(&args.options, &ty, ctx); - - ( - item.unwrap_or(old_default_richcmp), - slot.unwrap_or(old_default_richcmp_slot), - ) - }; + let (default_richcmp, default_richcmp_slot) = + pyclass_richcmp_simple_enum(&args.options, &ty, repr_type, ctx); - let default_slots = vec![default_repr_slot, default_int_slot, default_richcmp_slot]; + let mut default_slots = vec![default_repr_slot, default_int_slot]; + default_slots.extend(default_richcmp_slot); let pyclass_impls = PyClassImplsBuilder::new( cls, @@ -907,7 +857,7 @@ fn impl_complex_enum( let variants = complex_enum.variants; let pytypeinfo = impl_pytypeinfo(cls, &args, None, ctx); - let (default_richcmp, default_richcmp_slot) = pyclass_richcmp(&args.options, &ty, ctx); + let (default_richcmp, default_richcmp_slot) = pyclass_richcmp(&args.options, &ty, ctx)?; let mut default_slots = vec![]; default_slots.extend(default_richcmp_slot); @@ -1676,11 +1626,7 @@ fn impl_pytypeinfo( } } -fn pyclass_richcmp( - options: &PyClassPyO3Options, - cls: &syn::Type, - ctx: &Ctx, -) -> (Option, Option) { +fn pyclass_richcmp_arms(options: &PyClassPyO3Options, ctx: &Ctx) -> TokenStream { let Ctx { pyo3_path } = ctx; let eq_arms = options @@ -1688,16 +1634,110 @@ fn pyclass_richcmp( .map(|eq| { quote_spanned! { eq.span() => #pyo3_path::pyclass::CompareOp::Eq => { - ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self == other, py)) + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val == other, py)) }, #pyo3_path::pyclass::CompareOp::Ne => { - ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self != other, py)) + ::std::result::Result::Ok(#pyo3_path::conversion::IntoPy::into_py(self_val != other, py)) }, } }) .unwrap_or_default(); // TODO: `ord` can be integrated here (#4202) + #[allow(clippy::let_and_return)] + eq_arms +} + +fn pyclass_richcmp_simple_enum( + options: &PyClassPyO3Options, + cls: &syn::Type, + repr_type: &syn::Ident, + ctx: &Ctx, +) -> (Option, Option) { + let Ctx { pyo3_path } = ctx; + + let arms = pyclass_richcmp_arms(options, ctx); + + let deprecation = options + .eq_int + .map(|_| TokenStream::new()) + .unwrap_or_else(|| { + quote! { + #[deprecated( + since = "0.22.0", + note = "Implicit equality for simple enums is deprecated. Use `#[pyclass(eq_int)` to keep the current behavior." + )] + const DEPRECATION: () = (); + const _: () = DEPRECATION; + } + }); + + let mut options = options.clone(); + options.eq_int = Some(parse_quote!(eq_int)); + + if options.eq.is_none() && options.eq_int.is_none() { + return (None, None); + } + + let eq = options.eq.map(|eq| { + quote_spanned! { eq.span() => + let self_val = self; + if let ::std::result::Result::Ok(other) = #pyo3_path::types::PyAnyMethods::downcast::(other) { + let other = &*other.borrow(); + return match op { + #arms + _ => ::std::result::Result::Ok(py.NotImplemented()) + } + } + } + }); + + let eq_int = options.eq_int.map(|eq_int| { + quote_spanned! { eq_int.span() => + let self_val = self.__pyo3__int__(); + if let ::std::result::Result::Ok(other) = #pyo3_path::types::PyAnyMethods::extract::<#repr_type>(other).or_else(|_| { + #pyo3_path::types::PyAnyMethods::downcast::(other).map(|o| o.borrow().__pyo3__int__()) + }) { + return match op { + #arms + _ => ::std::result::Result::Ok(py.NotImplemented()) + } + } + } + }); + + let mut richcmp_impl = parse_quote! { + fn __pymethod___richcmp____( + &self, + py: #pyo3_path::Python, + other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, + op: #pyo3_path::pyclass::CompareOp + ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { + #deprecation + + #eq + + #eq_int + + ::std::result::Result::Ok(py.NotImplemented()) + } + }; + let richcmp_slot = + generate_default_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); + (Some(richcmp_impl), Some(richcmp_slot)) +} + +fn pyclass_richcmp( + options: &PyClassPyO3Options, + cls: &syn::Type, + ctx: &Ctx, +) -> Result<(Option, Option)> { + let Ctx { pyo3_path } = ctx; + if let Some(eq_int) = options.eq_int { + bail_spanned!(eq_int.span() => "`eq_int` can only be used on simple enums.") + } + + let arms = pyclass_richcmp_arms(options, ctx); if options.eq.is_some() { let mut richcmp_impl = parse_quote! { fn __pymethod___richcmp____( @@ -1706,18 +1746,19 @@ fn pyclass_richcmp( other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, op: #pyo3_path::pyclass::CompareOp ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { + let self_val = self; let other = &*#pyo3_path::types::PyAnyMethods::downcast::(other)?.borrow(); match op { - #eq_arms + #arms _ => ::std::result::Result::Ok(py.NotImplemented()) } } }; let richcmp_slot = generate_default_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); - (Some(richcmp_impl), Some(richcmp_slot)) + Ok((Some(richcmp_impl), Some(richcmp_slot))) } else { - (None, None) + Ok((None, None)) } } diff --git a/pytests/src/enums.rs b/pytests/src/enums.rs index 4e32f73f4d4..80d7550e1ec 100644 --- a/pytests/src/enums.rs +++ b/pytests/src/enums.rs @@ -18,7 +18,7 @@ pub fn enums(m: &Bound<'_, PyModule>) -> PyResult<()> { Ok(()) } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] pub enum SimpleEnum { Sunday, diff --git a/src/tests/hygiene/pyclass.rs b/src/tests/hygiene/pyclass.rs index 6cead2541d5..27a6b388769 100644 --- a/src/tests/hygiene/pyclass.rs +++ b/src/tests/hygiene/pyclass.rs @@ -29,7 +29,7 @@ pub struct Bar { c: ::std::option::Option>, } -#[crate::pyclass(eq)] +#[crate::pyclass(eq, eq_int)] #[pyo3(crate = "crate")] #[derive(PartialEq)] pub enum Enum { diff --git a/tests/test_declarative_module.rs b/tests/test_declarative_module.rs index 0e5f578596b..820cf63806d 100644 --- a/tests/test_declarative_module.rs +++ b/tests/test_declarative_module.rs @@ -89,7 +89,7 @@ mod declarative_module { } } - #[pyclass(eq)] + #[pyclass(eq, eq_int)] #[derive(PartialEq)] enum Enum { A, diff --git a/tests/test_default_impls.rs b/tests/test_default_impls.rs index 75152b0e339..670772369f8 100644 --- a/tests/test_default_impls.rs +++ b/tests/test_default_impls.rs @@ -6,7 +6,7 @@ use pyo3::prelude::*; mod common; // Test default generated __repr__. -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum TestDefaultRepr { Var, @@ -24,7 +24,7 @@ fn test_default_slot_exists() { }) } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(PartialEq)] enum OverrideSlot { Var, diff --git a/tests/test_enum.rs b/tests/test_enum.rs index d6b7a90fda9..148520dd771 100644 --- a/tests/test_enum.rs +++ b/tests/test_enum.rs @@ -6,7 +6,7 @@ use pyo3::py_run; #[path = "../src/tests/common.rs"] mod common; -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(Debug, PartialEq, Eq, Clone)] pub enum MyEnum { Variant, @@ -73,7 +73,7 @@ fn test_enum_eq_incomparable() { }) } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(Debug, PartialEq, Eq, Clone)] enum CustomDiscriminant { One = 1, @@ -122,7 +122,7 @@ fn test_enum_compare_int() { }) } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(Debug, PartialEq, Eq, Clone)] #[repr(u8)] enum SmallEnum { @@ -137,7 +137,7 @@ fn test_enum_compare_int_no_throw_when_overflow() { }) } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(Debug, PartialEq, Eq, Clone)] #[repr(usize)] #[allow(clippy::enum_clike_unportable_variant)] @@ -148,18 +148,15 @@ enum BigEnum { #[test] fn test_big_enum_no_overflow() { Python::with_gil(|py| { - // use pyo3::types::IntoPyDict; let usize_max = usize::MAX; let v = Py::new(py, BigEnum::V).unwrap(); - // let env = [("v", v.as_any()), ("usize_max", &usize_max.into_py(py))].into_py_dict_bound(py); - // py_run!(py, *env, "print(v)\nprint(usize_max)"); - // py_assert!(py, usize_max v, "v == usize_max"); + py_assert!(py, usize_max v, "v == usize_max"); py_assert!(py, usize_max v, "int(v) == usize_max"); }) } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(Debug, PartialEq, Eq, Clone)] #[repr(u16, align(8))] enum TestReprParse { @@ -171,7 +168,7 @@ fn test_repr_parse() { assert_eq!(std::mem::align_of::(), 8); } -#[pyclass(eq, name = "MyEnum")] +#[pyclass(eq, eq_int, name = "MyEnum")] #[derive(Debug, PartialEq, Eq, Clone)] pub enum RenameEnum { Variant, @@ -185,7 +182,7 @@ fn test_rename_enum_repr_correct() { }) } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] #[derive(Debug, PartialEq, Eq, Clone)] pub enum RenameVariantEnum { #[pyo3(name = "VARIANT")] @@ -200,7 +197,7 @@ fn test_rename_variant_repr_correct() { }) } -#[pyclass(eq, rename_all = "SCREAMING_SNAKE_CASE")] +#[pyclass(eq, eq_int, rename_all = "SCREAMING_SNAKE_CASE")] #[derive(Debug, PartialEq, Eq, Clone)] #[allow(clippy::enum_variant_names)] enum RenameAllVariantsEnum { diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index fbea2996350..eab47597f2f 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -34,7 +34,7 @@ error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`: 138 | fn pyfunction_option_4( | ^^^^^^^^^^^^^^^^^^^ -error: use of deprecated constant `SimpleEnumWithoutEq::__pyo3__richcmp__::DEPRECATION`: Implicit equality for simple enums in deprecated. Use `#[pyclass(eq)` to opt-in to the new behavior. +error: use of deprecated constant `SimpleEnumWithoutEq::__pymethod___richcmp____::DEPRECATION`: Implicit equality for simple enums is deprecated. Use `#[pyclass(eq_int)` to keep the current behavior. --> tests/ui/deprecations.rs:200:1 | 200 | #[pyclass] diff --git a/tests/ui/invalid_pyclass_args.rs b/tests/ui/invalid_pyclass_args.rs index 6fa91dad847..6e359f6130e 100644 --- a/tests/ui/invalid_pyclass_args.rs +++ b/tests/ui/invalid_pyclass_args.rs @@ -49,4 +49,7 @@ impl EqOptAndManualRichCmp { } } +#[pyclass(eq_int)] +struct NoEqInt {} + fn main() {} diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 85f4cc1c19d..5944ef47798 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -1,4 +1,4 @@ -error: expected one of: `crate`, `dict`, `eq`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:3:11 | 3 | #[pyclass(extend=pyo3::types::PyDict)] @@ -46,7 +46,7 @@ error: expected string literal 24 | #[pyclass(module = my_module)] | ^^^^^^^^^ -error: expected one of: `crate`, `dict`, `eq`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` +error: expected one of: `crate`, `dict`, `eq`, `eq_int`, `extends`, `freelist`, `frozen`, `get_all`, `mapping`, `module`, `name`, `rename_all`, `sequence`, `set_all`, `subclass`, `unsendable`, `weakref` --> tests/ui/invalid_pyclass_args.rs:27:11 | 27 | #[pyclass(weakrev)] @@ -58,6 +58,12 @@ error: a `#[pyclass]` cannot be both a `mapping` and a `sequence` 31 | struct CannotBeMappingAndSequence {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: `eq_int` can only be used on simple enums. + --> tests/ui/invalid_pyclass_args.rs:52:11 + | +52 | #[pyclass(eq_int)] + | ^^^^^^ + error[E0592]: duplicate definitions with name `__pymethod___richcmp____` --> tests/ui/invalid_pyclass_args.rs:36:1 | diff --git a/tests/ui/invalid_pyclass_enum.rs b/tests/ui/invalid_pyclass_enum.rs index 2c6d1f2a011..3c6f08da653 100644 --- a/tests/ui/invalid_pyclass_enum.rs +++ b/tests/ui/invalid_pyclass_enum.rs @@ -28,7 +28,7 @@ enum SimpleNoSignature { B, } -#[pyclass(eq)] +#[pyclass(eq, eq_int)] enum SimpleEqOptRequiresPartialEq { A, B, @@ -40,4 +40,10 @@ enum ComplexEqOptRequiresPartialEq { B { msg: String }, } +#[pyclass(eq_int)] +enum NoEqInt { + A(i32), + B { msg: String }, +} + fn main() {} diff --git a/tests/ui/invalid_pyclass_enum.stderr b/tests/ui/invalid_pyclass_enum.stderr index 948e34a1fb8..551d920eae3 100644 --- a/tests/ui/invalid_pyclass_enum.stderr +++ b/tests/ui/invalid_pyclass_enum.stderr @@ -30,10 +30,16 @@ error: `constructor` can't be used on a simple enum variant 26 | #[pyo3(constructor = (a, b))] | ^^^^^^^^^^^ +error: `eq_int` can only be used on simple enums. + --> tests/ui/invalid_pyclass_enum.rs:43:11 + | +43 | #[pyclass(eq_int)] + | ^^^^^^ + error[E0369]: binary operation `==` cannot be applied to type `&SimpleEqOptRequiresPartialEq` --> tests/ui/invalid_pyclass_enum.rs:31:11 | -31 | #[pyclass(eq)] +31 | #[pyclass(eq, eq_int)] | ^^ | note: an implementation of `PartialEq` might be missing for `SimpleEqOptRequiresPartialEq` @@ -50,7 +56,7 @@ help: consider annotating `SimpleEqOptRequiresPartialEq` with `#[derive(PartialE error[E0369]: binary operation `!=` cannot be applied to type `&SimpleEqOptRequiresPartialEq` --> tests/ui/invalid_pyclass_enum.rs:31:11 | -31 | #[pyclass(eq)] +31 | #[pyclass(eq, eq_int)] | ^^ | note: an implementation of `PartialEq` might be missing for `SimpleEqOptRequiresPartialEq` From c3f0e1981df8a9d3c8f5f062642747bbb8928341 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Thu, 30 May 2024 09:17:06 +0100 Subject: [PATCH 4/6] rearrange names to fix deprecation warning --- pyo3-macros-backend/src/pyclass.rs | 31 +++++++++++++++++++++++----- tests/ui/deprecations.stderr | 2 +- tests/ui/invalid_pyclass_args.stderr | 9 -------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 74f1835bfca..b13344cfdf7 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1265,6 +1265,23 @@ fn gen_complex_enum_variant_class_ident(enum_: &syn::Ident, variant: &syn::Ident format_ident!("{}_{}", enum_, variant) } +fn generate_protocol_slot( + cls: &syn::Type, + method: &mut syn::ImplItemFn, + slot: &SlotDef, + name: &str, + ctx: &Ctx, +) -> syn::Result { + let spec = FnSpec::parse( + &mut method.sig, + &mut Vec::new(), + PyFunctionOptions::default(), + ctx, + ) + .unwrap(); + slot.generate_type_slot(&syn::parse_quote!(#cls), &spec, name, ctx) +} + fn generate_default_protocol_slot( cls: &syn::Type, method: &mut syn::ImplItemFn, @@ -1707,7 +1724,7 @@ fn pyclass_richcmp_simple_enum( }); let mut richcmp_impl = parse_quote! { - fn __pymethod___richcmp____( + fn __pyo3__generated____richcmp__( &self, py: #pyo3_path::Python, other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, @@ -1722,8 +1739,11 @@ fn pyclass_richcmp_simple_enum( ::std::result::Result::Ok(py.NotImplemented()) } }; - let richcmp_slot = - generate_default_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); + let richcmp_slot = if options.eq.is_some() { + generate_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, "__richcmp__", ctx).unwrap() + } else { + generate_default_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap() + }; (Some(richcmp_impl), Some(richcmp_slot)) } @@ -1740,7 +1760,7 @@ fn pyclass_richcmp( let arms = pyclass_richcmp_arms(options, ctx); if options.eq.is_some() { let mut richcmp_impl = parse_quote! { - fn __pymethod___richcmp____( + fn __pyo3__generated____richcmp__( &self, py: #pyo3_path::Python, other: &#pyo3_path::Bound<'_, #pyo3_path::PyAny>, @@ -1755,7 +1775,8 @@ fn pyclass_richcmp( } }; let richcmp_slot = - generate_default_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, ctx).unwrap(); + generate_protocol_slot(cls, &mut richcmp_impl, &__RICHCMP__, "__richcmp__", ctx) + .unwrap(); Ok((Some(richcmp_impl), Some(richcmp_slot))) } else { Ok((None, None)) diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index eab47597f2f..42b0e908f73 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -34,7 +34,7 @@ error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`: 138 | fn pyfunction_option_4( | ^^^^^^^^^^^^^^^^^^^ -error: use of deprecated constant `SimpleEnumWithoutEq::__pymethod___richcmp____::DEPRECATION`: Implicit equality for simple enums is deprecated. Use `#[pyclass(eq_int)` to keep the current behavior. +error: use of deprecated constant `SimpleEnumWithoutEq::__pyo3__generated____richcmp__::DEPRECATION`: Implicit equality for simple enums is deprecated. Use `#[pyclass(eq_int)` to keep the current behavior. --> tests/ui/deprecations.rs:200:1 | 200 | #[pyclass] diff --git a/tests/ui/invalid_pyclass_args.stderr b/tests/ui/invalid_pyclass_args.stderr index 5944ef47798..72da82385e7 100644 --- a/tests/ui/invalid_pyclass_args.stderr +++ b/tests/ui/invalid_pyclass_args.stderr @@ -127,15 +127,6 @@ note: candidate #2 is defined in an impl for the type `EqOptAndManualRichCmp` | ^^^^^^^^^^^^ = note: this error originates in the attribute macro `pyclass` which comes from the expansion of the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info) -warning: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/invalid_pyclass_args.rs:36:1 - | -36 | #[pyclass(eq)] - | ^^^^^^^^^^^^^^ - | - = 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) - error[E0034]: multiple applicable items in scope --> tests/ui/invalid_pyclass_args.rs:40:1 | From 5dada38d048b09e338e14096dc85c3ca5aa90add Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Thu, 30 May 2024 11:37:30 +0200 Subject: [PATCH 5/6] add newsfragment and migration --- guide/src/migration.md | 31 +++++++++++++++++++++++++++++++ newsfragments/4210.added.md | 1 + newsfragments/4210.changed.md | 1 + 3 files changed, 33 insertions(+) create mode 100644 newsfragments/4210.added.md create mode 100644 newsfragments/4210.changed.md diff --git a/guide/src/migration.md b/guide/src/migration.md index f56db2a5fc7..49aa6a270df 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -47,6 +47,37 @@ However, take care to note that the behaviour is different from previous version Related to this, we also added a `pyo3_disable_reference_pool` conditional compilation flag which removes the infrastructure necessary to apply delayed reference count decrements implied by `impl Drop for Py`. They do not appear to be a soundness hazard as they should lead to memory leaks in the worst case. However, the global synchronization adds significant overhead to cross the Python-Rust boundary. Enabling this feature will remove these costs and make the `Drop` implementation abort the process if called without the GIL being held instead. +### Deprecation of implicit integer comparison for simple enums +
+Click to expand + +With `pyo3` 0.22 the implicit implementation of integer comparison for simple enums using their discriminants is deprecated. To migrate, place a `#[pyo3(eq_int)]` attribute on affected classes. In addition the `#[pyo3(eq)]` option can be used to implement comparison based on the `PartialEq` implementation. If both options are specified, comparing by `PartialEq` and on failure the integer comparision is used as a fallback. + +Before: + +```rust +# #![allow(deprecated, dead_code)] +# use pyo3::prelude::*; +#[pyclass] +enum SimpleEnum { + VariantA, + VariantB = 42, +} +``` + +After: + +```rust +# #![allow(dead_code)] +# use pyo3::prelude::*; +#[pyclass(eq_int)] +enum SimpleEnum { + VariantA, + VariantB = 42, +} +``` +
+ ## from 0.20.* to 0.21
Click to expand diff --git a/newsfragments/4210.added.md b/newsfragments/4210.added.md new file mode 100644 index 00000000000..33acd63025f --- /dev/null +++ b/newsfragments/4210.added.md @@ -0,0 +1 @@ +Added `#[pyclass(eq)]` option to generate `__eq__` based on `PartialEq`. \ No newline at end of file diff --git a/newsfragments/4210.changed.md b/newsfragments/4210.changed.md new file mode 100644 index 00000000000..a69e3c3a37e --- /dev/null +++ b/newsfragments/4210.changed.md @@ -0,0 +1 @@ +Deprecate implicit integer comparision for simple enums in favor of `#[pyclass(eq_int)]`. \ No newline at end of file From 9bc53d2a168ed07424d0ee0822e5527ca37307b3 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 31 May 2024 15:31:56 +0200 Subject: [PATCH 6/6] update docs --- guide/src/class/object.md | 3 ++- guide/src/migration.md | 9 ++++++--- newsfragments/4210.added.md | 3 ++- pyo3-macros-backend/src/pyclass.rs | 2 +- tests/ui/deprecations.stderr | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/guide/src/class/object.md b/guide/src/class/object.md index 59e368d13df..e7a366870b7 100644 --- a/guide/src/class/object.md +++ b/guide/src/class/object.md @@ -226,7 +226,7 @@ impl Number { # } ``` -To implement `__eq__` in terms of the `PartialEq` implementation of the underlying datatype, the `eq` option can be used. +To implement `__eq__` using the Rust [`PartialEq`] trait implementation, the `eq` option can be used. ```rust # use pyo3::prelude::*; @@ -315,3 +315,4 @@ fn my_module(m: &Bound<'_, PyModule>) -> PyResult<()> { [`Hasher`]: https://doc.rust-lang.org/std/hash/trait.Hasher.html [`DefaultHasher`]: https://doc.rust-lang.org/std/collections/hash_map/struct.DefaultHasher.html [SipHash]: https://en.wikipedia.org/wiki/SipHash +[`PartialEq`]: https://doc.rust-lang.org/stable/std/cmp/trait.PartialEq.html diff --git a/guide/src/migration.md b/guide/src/migration.md index 49aa6a270df..31e912a6856 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -47,11 +47,13 @@ However, take care to note that the behaviour is different from previous version Related to this, we also added a `pyo3_disable_reference_pool` conditional compilation flag which removes the infrastructure necessary to apply delayed reference count decrements implied by `impl Drop for Py`. They do not appear to be a soundness hazard as they should lead to memory leaks in the worst case. However, the global synchronization adds significant overhead to cross the Python-Rust boundary. Enabling this feature will remove these costs and make the `Drop` implementation abort the process if called without the GIL being held instead.
-### Deprecation of implicit integer comparison for simple enums +### Require explicit opt-in for comparison for simple enums
Click to expand -With `pyo3` 0.22 the implicit implementation of integer comparison for simple enums using their discriminants is deprecated. To migrate, place a `#[pyo3(eq_int)]` attribute on affected classes. In addition the `#[pyo3(eq)]` option can be used to implement comparison based on the `PartialEq` implementation. If both options are specified, comparing by `PartialEq` and on failure the integer comparision is used as a fallback. +With `pyo3` 0.22 the new `#[pyo3(eq)]` options allows automatic implementation of Python equality using Rust's `PartialEq`. Previously simple enums automatically implemented equality in terms of their discriminants. To make PyO3 more consistent, this automatic equality implementation is deprecated in favour of having opt-ins for all `#[pyclass]` types. Similarly, simple enums supported comparison with integers, which is not covered by Rust's `PartialEq` derive, so has been split out into the `#[pyo3(eq_int)]` attribute. + +To migrate, place a `#[pyo3(eq, eq_int)]` attribute on simple enum classes. Before: @@ -70,7 +72,8 @@ After: ```rust # #![allow(dead_code)] # use pyo3::prelude::*; -#[pyclass(eq_int)] +#[pyclass(eq, eq_int)] +#[derive(PartialEq)] enum SimpleEnum { VariantA, VariantB = 42, diff --git a/newsfragments/4210.added.md b/newsfragments/4210.added.md index 33acd63025f..dae8cd8dabb 100644 --- a/newsfragments/4210.added.md +++ b/newsfragments/4210.added.md @@ -1 +1,2 @@ -Added `#[pyclass(eq)]` option to generate `__eq__` based on `PartialEq`. \ No newline at end of file +Added `#[pyclass(eq)]` option to generate `__eq__` based on `PartialEq`. +Added `#[pyclass(eq_int)]` for simple enums to implement equality based on their discriminants. \ No newline at end of file diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index b13344cfdf7..5838f7c5f5c 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1682,7 +1682,7 @@ fn pyclass_richcmp_simple_enum( quote! { #[deprecated( since = "0.22.0", - note = "Implicit equality for simple enums is deprecated. Use `#[pyclass(eq_int)` to keep the current behavior." + note = "Implicit equality for simple enums is deprecated. Use `#[pyclass(eq, eq_int)` to keep the current behavior." )] const DEPRECATION: () = (); const _: () = DEPRECATION; diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index 42b0e908f73..e08139863d1 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -34,7 +34,7 @@ error: use of deprecated constant `__pyfunction_pyfunction_option_4::SIGNATURE`: 138 | fn pyfunction_option_4( | ^^^^^^^^^^^^^^^^^^^ -error: use of deprecated constant `SimpleEnumWithoutEq::__pyo3__generated____richcmp__::DEPRECATION`: Implicit equality for simple enums is deprecated. Use `#[pyclass(eq_int)` to keep the current behavior. +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. --> tests/ui/deprecations.rs:200:1 | 200 | #[pyclass]