From c1f524f5c7570e6af70aff82c28e368a326ec0c0 Mon Sep 17 00:00:00 2001 From: Michael Gilbert Date: Wed, 17 Jul 2024 14:02:33 -0700 Subject: [PATCH] fix: adding tests for pyclass hygiene cases (#4359) * 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 * fix: fully qualified clone and from calls for enums. * update: added changelog entry --------- Co-authored-by: MG --- newsfragments/4359.fixed.md | 1 + pyo3-macros-backend/src/pyclass.rs | 23 +++++++++++------------ src/tests/hygiene/pyclass.rs | 25 +++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 12 deletions(-) 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 diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 078a7b7f4af..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,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(::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 @@ -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(::std::clone::Clone::clone(&val)), + _ => ::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")), } } }; @@ -1287,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 @@ -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), +}