From 2f83eec0c0656ebcf828775784d61271dbe836f2 Mon Sep 17 00:00:00 2001 From: MG Date: Wed, 17 Jul 2024 08:04:12 -0700 Subject: [PATCH 1/3] fix: updated pyclass heighten check to validate for eq and ord, fixing Ok issue in eq implementation. identified similar issues in Complex enum and tuple enum, resolved serveral cases, but working on current error coming from traits not being in scope --- pyo3-macros-backend/src/pyclass.rs | 17 ++++++++--------- src/tests/hygiene/pyclass.rs | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 078a7b7f4af..edd480bc74f 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1091,8 +1091,8 @@ fn impl_complex_enum_struct_variant_cls( let field_getter_impl = quote! { fn #field_name(slf: #pyo3_path::PyRef) -> #pyo3_path::PyResult<#field_type> { match &*slf.into_super() { - #enum_name::#variant_ident { #field_name, .. } => Ok(#field_name.clone()), - _ => unreachable!("Wrong complex enum variant found in variant wrapper PyClass"), + #enum_name::#variant_ident { #field_name, .. } => ::std::result::Result::Ok(#field_name.clone()), + _ => ::core::unreachable!("Wrong complex enum variant found in variant wrapper PyClass"), } } }; @@ -1157,12 +1157,11 @@ fn impl_complex_enum_tuple_variant_field_getters( } }) .collect(); - let field_getter_impl: syn::ImplItemFn = parse_quote! { fn #field_name(slf: #pyo3_path::PyRef) -> #pyo3_path::PyResult<#field_type> { match &*slf.into_super() { - #enum_name::#variant_ident ( #(#field_access_tokens), *) => Ok(val.clone()), - _ => unreachable!("Wrong complex enum variant found in variant wrapper PyClass"), + #enum_name::#variant_ident ( #(#field_access_tokens), *) => ::std::result::Result::Ok(val.clone()), + _ => ::core::unreachable!("Wrong complex enum variant found in variant wrapper PyClass"), } } }; @@ -1186,7 +1185,7 @@ fn impl_complex_enum_tuple_variant_len( let mut len_method_impl: syn::ImplItemFn = parse_quote! { fn __len__(slf: #pyo3_path::PyRef) -> #pyo3_path::PyResult { - Ok(#num_fields) + ::std::result::Result::Ok(#num_fields) } }; @@ -1208,7 +1207,7 @@ fn impl_complex_enum_tuple_variant_getitem( .map(|i| { let field_access = format_ident!("_{}", i); quote! { - #i => Ok( + #i => ::std::result::Result::Ok( #pyo3_path::IntoPy::into_py( #variant_cls::#field_access(slf)? , py) @@ -1223,7 +1222,7 @@ fn impl_complex_enum_tuple_variant_getitem( let py = slf.py(); match idx { #( #match_arms, )* - _ => Err(pyo3::exceptions::PyIndexError::new_err("tuple index out of range")), + _ => ::std::result::Result::Err(#pyo3_path::exceptions::PyIndexError::new_err("tuple index out of range")), } } }; @@ -1828,7 +1827,7 @@ fn pyclass_richcmp( op: #pyo3_path::pyclass::CompareOp ) -> #pyo3_path::PyResult<#pyo3_path::PyObject> { let self_val = self; - if let Ok(other) = #pyo3_path::types::PyAnyMethods::downcast::(other) { + if let ::std::result::Result::Ok(other) = #pyo3_path::types::PyAnyMethods::downcast::(other) { let other = &*other.borrow(); match op { #arms diff --git a/src/tests/hygiene/pyclass.rs b/src/tests/hygiene/pyclass.rs index 3c078f580d5..dcd347fb5f2 100644 --- a/src/tests/hygiene/pyclass.rs +++ b/src/tests/hygiene/pyclass.rs @@ -66,3 +66,28 @@ pub struct Foo4 { #[cfg(not(any()))] field: u32, } + +#[crate::pyclass(eq, ord)] +#[pyo3(crate = "crate")] +#[derive(PartialEq, PartialOrd)] +pub struct PointEqOrd { + x: u32, + y: u32, + z: u32, +} + +#[crate::pyclass(eq, ord)] +#[pyo3(crate = "crate")] +#[derive(PartialEq, PartialOrd)] +pub enum ComplexEnumEqOrd { + Variant1 { a: u32, b: u32 }, + Variant2 { c: u32 }, +} + +#[crate::pyclass(eq, ord)] +#[pyo3(crate = "crate")] +#[derive(PartialEq, PartialOrd)] +pub enum TupleEnumEqOrd { + Variant1(u32, u32), + Variant2(u32), +} From 40f65a76187a26f0f7c74320e454d3426b0abecf Mon Sep 17 00:00:00 2001 From: MG Date: Wed, 17 Jul 2024 12:53:44 -0700 Subject: [PATCH 2/3] fix: fully qualified clone and from calls for enums. --- pyo3-macros-backend/src/pyclass.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index edd480bc74f..125fdab4927 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -923,7 +923,7 @@ fn impl_complex_enum( let variant_cls = gen_complex_enum_variant_class_ident(cls, variant.get_ident()); quote! { #cls::#variant_ident { .. } => { - let pyclass_init = #pyo3_path::PyClassInitializer::from(self).add_subclass(#variant_cls); + let pyclass_init = <#pyo3_path::PyClassInitializer as ::std::convert::From>::from(self).add_subclass(#variant_cls); let variant_value = #pyo3_path::Py::new(py, pyclass_init).unwrap(); #pyo3_path::IntoPy::into_py(variant_value, py) } @@ -1091,7 +1091,7 @@ fn impl_complex_enum_struct_variant_cls( let field_getter_impl = quote! { fn #field_name(slf: #pyo3_path::PyRef) -> #pyo3_path::PyResult<#field_type> { match &*slf.into_super() { - #enum_name::#variant_ident { #field_name, .. } => ::std::result::Result::Ok(#field_name.clone()), + #enum_name::#variant_ident { #field_name, .. } => ::std::result::Result::Ok(::std::clone::Clone::clone(&#field_name)), _ => ::core::unreachable!("Wrong complex enum variant found in variant wrapper PyClass"), } } @@ -1114,7 +1114,7 @@ fn impl_complex_enum_struct_variant_cls( impl #variant_cls { fn __pymethod_constructor__(py: #pyo3_path::Python<'_>, #(#fields_with_types,)*) -> #pyo3_path::PyClassInitializer<#variant_cls> { let base_value = #enum_name::#variant_ident { #(#field_names,)* }; - #pyo3_path::PyClassInitializer::from(base_value).add_subclass(#variant_cls) + <#pyo3_path::PyClassInitializer<#enum_name> as ::std::convert::From<#enum_name>>::from(base_value).add_subclass(#variant_cls) } #match_args_const_impl @@ -1160,7 +1160,7 @@ fn impl_complex_enum_tuple_variant_field_getters( let field_getter_impl: syn::ImplItemFn = parse_quote! { fn #field_name(slf: #pyo3_path::PyRef) -> #pyo3_path::PyResult<#field_type> { match &*slf.into_super() { - #enum_name::#variant_ident ( #(#field_access_tokens), *) => ::std::result::Result::Ok(val.clone()), + #enum_name::#variant_ident ( #(#field_access_tokens), *) => ::std::result::Result::Ok(::std::clone::Clone::clone(&val)), _ => ::core::unreachable!("Wrong complex enum variant found in variant wrapper PyClass"), } } @@ -1286,7 +1286,7 @@ fn impl_complex_enum_tuple_variant_cls( impl #variant_cls { fn __pymethod_constructor__(py: #pyo3_path::Python<'_>, #(#field_names : #field_types,)*) -> #pyo3_path::PyClassInitializer<#variant_cls> { let base_value = #enum_name::#variant_ident ( #(#field_names,)* ); - #pyo3_path::PyClassInitializer::from(base_value).add_subclass(#variant_cls) + <#pyo3_path::PyClassInitializer<#enum_name> as ::std::convert::From<#enum_name>>::from(base_value).add_subclass(#variant_cls) } #len_method_impl From 46edf85f1db7784ceb299e46090b3b70fe7858ad Mon Sep 17 00:00:00 2001 From: MG Date: Wed, 17 Jul 2024 13:34:19 -0700 Subject: [PATCH 3/3] update: added changelog entry --- newsfragments/4359.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4359.fixed.md diff --git a/newsfragments/4359.fixed.md b/newsfragments/4359.fixed.md new file mode 100644 index 00000000000..7174cab0a9d --- /dev/null +++ b/newsfragments/4359.fixed.md @@ -0,0 +1 @@ +Fixed `pyclass` macro hygiene issues for structs and enums and added tests. \ No newline at end of file