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

cfg features for enum variants #4509

Merged
merged 8 commits into from
Sep 16, 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/4509.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix compile failure when using `#[cfg]` attributes for simple enum variants.
87 changes: 78 additions & 9 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::attributes::{
use crate::konst::{ConstAttributes, ConstSpec};
use crate::method::{FnArg, FnSpec, PyArg, RegularArg};
use crate::pyfunction::ConstructorAttribute;
use crate::pyimpl::{gen_py_const, PyClassMethodsType};
use crate::pyimpl::{gen_py_const, get_cfg_attributes, PyClassMethodsType};
use crate::pymethod::{
impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef, MethodAndSlotDef, PropertyType,
SlotDef, __GETITEM__, __HASH__, __INT__, __LEN__, __REPR__, __RICHCMP__, __STR__,
Expand Down Expand Up @@ -533,7 +533,12 @@ impl<'a> PyClassSimpleEnum<'a> {
_ => bail_spanned!(variant.span() => "Must be a unit variant."),
};
let options = EnumVariantPyO3Options::take_pyo3_options(&mut variant.attrs)?;
Ok(PyClassEnumUnitVariant { ident, options })
let cfg_attrs = get_cfg_attributes(&variant.attrs);
Ok(PyClassEnumUnitVariant {
ident,
options,
cfg_attrs,
})
}

let ident = &enum_.ident;
Expand Down Expand Up @@ -693,6 +698,7 @@ impl<'a> EnumVariant for PyClassEnumVariant<'a> {
struct PyClassEnumUnitVariant<'a> {
ident: &'a syn::Ident,
options: EnumVariantPyO3Options,
cfg_attrs: Vec<&'a syn::Attribute>,
}

impl<'a> EnumVariant for PyClassEnumUnitVariant<'a> {
Expand Down Expand Up @@ -877,20 +883,23 @@ fn impl_simple_enum(
ensure_spanned!(variant.options.constructor.is_none(), variant.options.constructor.span() => "`constructor` can't be used on a simple enum variant");
}

let variant_cfg_check = generate_cfg_check(&variants, cls);

let (default_repr, default_repr_slot) = {
let variants_repr = variants.iter().map(|variant| {
let variant_name = variant.ident;
let cfg_attrs = &variant.cfg_attrs;
// Assuming all variants are unit variants because they are the only type we support.
let repr = format!(
"{}.{}",
get_class_python_name(cls, args),
variant.get_python_name(args),
);
quote! { #cls::#variant_name => #repr, }
quote! { #(#cfg_attrs)* #cls::#variant_name => #repr, }
});
let mut repr_impl: syn::ImplItemFn = syn::parse_quote! {
fn __pyo3__repr__(&self) -> &'static str {
match self {
match *self {
#(#variants_repr)*
}
}
Expand All @@ -908,11 +917,12 @@ fn impl_simple_enum(
// This implementation allows us to convert &T to #repr_type without implementing `Copy`
let variants_to_int = variants.iter().map(|variant| {
let variant_name = variant.ident;
quote! { #cls::#variant_name => #cls::#variant_name as #repr_type, }
let cfg_attrs = &variant.cfg_attrs;
quote! { #(#cfg_attrs)* #cls::#variant_name => #cls::#variant_name as #repr_type, }
});
let mut int_impl: syn::ImplItemFn = syn::parse_quote! {
fn __pyo3__int__(&self) -> #repr_type {
match self {
match *self {
#(#variants_to_int)*
}
}
Expand All @@ -936,7 +946,9 @@ fn impl_simple_enum(
methods_type,
simple_enum_default_methods(
cls,
variants.iter().map(|v| (v.ident, v.get_python_name(args))),
variants
.iter()
.map(|v| (v.ident, v.get_python_name(args), &v.cfg_attrs)),
ctx,
),
default_slots,
Expand All @@ -945,6 +957,8 @@ fn impl_simple_enum(
.impl_all(ctx)?;

Ok(quote! {
#variant_cfg_check

#pytypeinfo

#pyclass_impls
Expand Down Expand Up @@ -1474,7 +1488,13 @@ fn generate_default_protocol_slot(

fn simple_enum_default_methods<'a>(
cls: &'a syn::Ident,
unit_variant_names: impl IntoIterator<Item = (&'a syn::Ident, Cow<'a, syn::Ident>)>,
unit_variant_names: impl IntoIterator<
Item = (
&'a syn::Ident,
Cow<'a, syn::Ident>,
&'a Vec<&'a syn::Attribute>,
),
>,
ctx: &Ctx,
) -> Vec<MethodAndMethodDef> {
let cls_type = syn::parse_quote!(#cls);
Expand All @@ -1490,7 +1510,25 @@ fn simple_enum_default_methods<'a>(
};
unit_variant_names
.into_iter()
.map(|(var, py_name)| gen_py_const(&cls_type, &variant_to_attribute(var, &py_name), ctx))
.map(|(var, py_name, attrs)| {
let method = gen_py_const(&cls_type, &variant_to_attribute(var, &py_name), ctx);
let associated_method_tokens = method.associated_method;
let method_def_tokens = method.method_def;

let associated_method = quote! {
#(#attrs)*
#associated_method_tokens
};
let method_def = quote! {
#(#attrs)*
#method_def_tokens
};

MethodAndMethodDef {
associated_method,
method_def,
}
})
.collect()
}

Expand Down Expand Up @@ -2395,6 +2433,37 @@ fn define_inventory_class(inventory_class_name: &syn::Ident, ctx: &Ctx) -> Token
}
}

fn generate_cfg_check(variants: &[PyClassEnumUnitVariant<'_>], cls: &syn::Ident) -> TokenStream {
if variants.is_empty() {
return quote! {};
}

let mut conditions = Vec::new();

for variant in variants {
let cfg_attrs = &variant.cfg_attrs;

if cfg_attrs.is_empty() {
// There's at least one variant of the enum without cfg attributes,
// so the check is not necessary
return quote! {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return quote! {};
// There's at least one variant of the enum without cfg attributes,
// so the check is not necessary
return quote! {};

}

for attr in cfg_attrs {
if let syn::Meta::List(meta) = &attr.meta {
let cfg_tokens = &meta.tokens;
conditions.push(quote! { not(#cfg_tokens) });
}
}
}

quote_spanned! {
cls.span() =>
#[cfg(all(#(#conditions),*))]
::core::compile_error!(concat!("#[pyclass] can't be used on enums without any variants - all variants of enum `", stringify!(#cls), "` have been configured out by cfg attributes"));
}
}

const UNIQUE_GET: &str = "`get` may only be specified once";
const UNIQUE_SET: &str = "`set` may only be specified once";
const UNIQUE_NAME: &str = "`name` may only be specified once";
Expand Down
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/pyimpl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ fn submit_methods_inventory(
}
}

fn get_cfg_attributes(attrs: &[syn::Attribute]) -> Vec<&syn::Attribute> {
pub(crate) fn get_cfg_attributes(attrs: &[syn::Attribute]) -> Vec<&syn::Attribute> {
attrs
.iter()
.filter(|attr| attr.path().is_ident("cfg"))
Expand Down
24 changes: 24 additions & 0 deletions tests/test_field_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ struct CfgClass {
pub b: u32,
}

#[pyclass(eq, eq_int)]
#[derive(PartialEq)]
enum CfgSimpleEnum {
#[cfg(any())]
DisabledVariant,
#[cfg(not(any()))]
EnabledVariant,
}

#[test]
fn test_cfg() {
Python::with_gil(|py| {
Expand All @@ -27,3 +36,18 @@ fn test_cfg() {
assert_eq!(b, 3);
});
}

#[test]
fn test_cfg_simple_enum() {
Python::with_gil(|py| {
let simple = py.get_type::<CfgSimpleEnum>();
pyo3::py_run!(
py,
simple,
r#"
assert hasattr(simple, "EnabledVariant")
assert not hasattr(simple, "DisabledVariant")
"#
);
})
}
9 changes: 9 additions & 0 deletions tests/ui/invalid_pyclass_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,13 @@ enum InvalidOrderedComplexEnum2 {
VariantB { msg: String }
}

#[pyclass(eq)]
#[derive(PartialEq)]
enum AllEnumVariantsDisabled {
#[cfg(any())]
DisabledA,
#[cfg(not(all()))]
DisabledB,
}

fn main() {}
6 changes: 6 additions & 0 deletions tests/ui/invalid_pyclass_enum.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ error: The `ord` option requires the `eq` option.
83 | #[pyclass(ord)]
| ^^^

error: #[pyclass] can't be used on enums without any variants - all variants of enum `AllEnumVariantsDisabled` have been configured out by cfg attributes
--> tests/ui/invalid_pyclass_enum.rs:98:6
|
98 | enum AllEnumVariantsDisabled {
| ^^^^^^^^^^^^^^^^^^^^^^^

error[E0369]: binary operation `==` cannot be applied to type `&SimpleEqOptRequiresPartialEq`
--> tests/ui/invalid_pyclass_enum.rs:31:11
|
Expand Down
Loading