diff --git a/bindgen-tests/tests/expectations/tests/bitfield-enum-basic.rs b/bindgen-tests/tests/expectations/tests/bitfield-enum-basic.rs index adc4690c86..aecb9dc639 100644 --- a/bindgen-tests/tests/expectations/tests/bitfield-enum-basic.rs +++ b/bindgen-tests/tests/expectations/tests/bitfield-enum-basic.rs @@ -1,14 +1,8 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] impl Foo { pub const Bar: Foo = Foo(2); -} -impl Foo { pub const Baz: Foo = Foo(4); -} -impl Foo { pub const Duplicated: Foo = Foo(4); -} -impl Foo { pub const Negative: Foo = Foo(-3); } impl ::std::ops::BitOr for Foo { @@ -42,14 +36,8 @@ impl ::std::ops::BitAndAssign for Foo { pub struct Foo(pub ::std::os::raw::c_int); impl Buz { pub const Bar: Buz = Buz(2); -} -impl Buz { pub const Baz: Buz = Buz(4); -} -impl Buz { pub const Duplicated: Buz = Buz(4); -} -impl Buz { pub const Negative: Buz = Buz(-3); } impl ::std::ops::BitOr for Buz { diff --git a/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-c.rs b/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-c.rs index 0b5202dfe3..df5a69eddd 100644 --- a/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-c.rs +++ b/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-c.rs @@ -1,14 +1,8 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] impl Foo { pub const Bar: Foo = Foo(2); -} -impl Foo { pub const Baz: Foo = Foo(4); -} -impl Foo { pub const Duplicated: Foo = Foo(4); -} -impl Foo { pub const Negative: Foo = Foo(-3); } impl ::std::ops::BitOr for Foo { diff --git a/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-transparent.rs b/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-transparent.rs index 0b5202dfe3..df5a69eddd 100644 --- a/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-transparent.rs +++ b/bindgen-tests/tests/expectations/tests/bitfield-enum-repr-transparent.rs @@ -1,14 +1,8 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] impl Foo { pub const Bar: Foo = Foo(2); -} -impl Foo { pub const Baz: Foo = Foo(4); -} -impl Foo { pub const Duplicated: Foo = Foo(4); -} -impl Foo { pub const Negative: Foo = Foo(-3); } impl ::std::ops::BitOr for Foo { diff --git a/bindgen-tests/tests/expectations/tests/enum-default-bitfield.rs b/bindgen-tests/tests/expectations/tests/enum-default-bitfield.rs index 58b8bf092f..b7b14fbc1c 100644 --- a/bindgen-tests/tests/expectations/tests/enum-default-bitfield.rs +++ b/bindgen-tests/tests/expectations/tests/enum-default-bitfield.rs @@ -52,8 +52,6 @@ impl Default for foo { } impl Foo { pub const Bar: Foo = Foo(0); -} -impl Foo { pub const Qux: Foo = Foo(1); } impl ::std::ops::BitOr for Foo { @@ -92,8 +90,6 @@ pub mod Neg { } impl NoDebug { pub const NoDebug1: NoDebug = NoDebug(0); -} -impl NoDebug { pub const NoDebug2: NoDebug = NoDebug(1); } impl ::std::ops::BitOr for NoDebug { @@ -128,8 +124,6 @@ impl ::std::ops::BitAndAssign for NoDebug { pub struct NoDebug(pub ::std::os::raw::c_uint); impl Debug { pub const Debug1: Debug = Debug(0); -} -impl Debug { pub const Debug2: Debug = Debug(1); } impl ::std::ops::BitOr for Debug { diff --git a/bindgen-tests/tests/expectations/tests/enum-doc-bitfield.rs b/bindgen-tests/tests/expectations/tests/enum-doc-bitfield.rs index ba73a8ea3e..33eec3e44f 100644 --- a/bindgen-tests/tests/expectations/tests/enum-doc-bitfield.rs +++ b/bindgen-tests/tests/expectations/tests/enum-doc-bitfield.rs @@ -2,24 +2,14 @@ impl B { /// Document field with three slashes pub const VAR_A: B = B(0); -} -impl B { /// Document field with preceding star pub const VAR_B: B = B(1); -} -impl B { /// Document field with preceding exclamation pub const VAR_C: B = B(2); -} -impl B { ///< Document field with following star pub const VAR_D: B = B(3); -} -impl B { ///< Document field with following exclamation pub const VAR_E: B = B(4); -} -impl B { /** Document field with preceding star, with a loong long multiline comment. diff --git a/bindgen-tests/tests/expectations/tests/issue-1198-alias-rust-bitfield-enum.rs b/bindgen-tests/tests/expectations/tests/issue-1198-alias-rust-bitfield-enum.rs index 7bbcb0d50b..5aaff691b4 100644 --- a/bindgen-tests/tests/expectations/tests/issue-1198-alias-rust-bitfield-enum.rs +++ b/bindgen-tests/tests/expectations/tests/issue-1198-alias-rust-bitfield-enum.rs @@ -1,11 +1,7 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] impl MyDupeEnum { pub const A: MyDupeEnum = MyDupeEnum(0); -} -impl MyDupeEnum { pub const A_alias: MyDupeEnum = MyDupeEnum(0); -} -impl MyDupeEnum { pub const B: MyDupeEnum = MyDupeEnum(1); } impl ::std::ops::BitOr for MyDupeEnum { @@ -39,11 +35,7 @@ impl ::std::ops::BitAndAssign for MyDupeEnum { pub struct MyDupeEnum(pub ::std::os::raw::c_uint); impl MyOtherDupeEnum { pub const C: MyOtherDupeEnum = MyOtherDupeEnum(0); -} -impl MyOtherDupeEnum { pub const C_alias: MyOtherDupeEnum = MyOtherDupeEnum(0); -} -impl MyOtherDupeEnum { pub const D: MyOtherDupeEnum = MyOtherDupeEnum(1); } impl ::std::ops::BitOr for MyOtherDupeEnum { diff --git a/bindgen-tests/tests/expectations/tests/issue-1435.rs b/bindgen-tests/tests/expectations/tests/issue-1435.rs index 7b9bfe8422..b50e1f88c0 100644 --- a/bindgen-tests/tests/expectations/tests/issue-1435.rs +++ b/bindgen-tests/tests/expectations/tests/issue-1435.rs @@ -6,8 +6,8 @@ pub mod root { pub mod ns { #[allow(unused_imports)] use self::super::super::root; - pub const AB_A: root::ns::AB = 0; - pub const AB_B: root::ns::AB = 1; + pub const AB_A: AB = 0; + pub const AB_B: AB = 1; pub type AB = ::std::os::raw::c_int; } pub use self::super::root::ns::AB as AB; diff --git a/bindgen-tests/tests/expectations/tests/newtype-enum.rs b/bindgen-tests/tests/expectations/tests/newtype-enum.rs index ea25950de0..0045c52a61 100644 --- a/bindgen-tests/tests/expectations/tests/newtype-enum.rs +++ b/bindgen-tests/tests/expectations/tests/newtype-enum.rs @@ -1,14 +1,8 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] impl Foo { pub const Bar: Foo = Foo(2); -} -impl Foo { pub const Baz: Foo = Foo(4); -} -impl Foo { pub const Duplicated: Foo = Foo(4); -} -impl Foo { pub const Negative: Foo = Foo(-3); } #[repr(transparent)] diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index f5518e432d..48f3947b3b 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -3266,111 +3266,106 @@ impl FromStr for EnumVariation { } } +struct EnumBuilder { + /// Type identifier of the enum. + /// + /// This is the base name, i.e. for ModuleConst enums, this does not include the module name. + enum_type: Ident, + /// Attributes applying to the enum type + attrs: Vec, + /// The representation of the enum, e.g. `u32`. + repr: syn::Type, + /// The enum kind we are generating + kind: EnumBuilderKind, + /// A list of all variants this enum has. + enum_variants: Vec, +} + /// A helper type to construct different enum variations. -enum EnumBuilder<'a> { +enum EnumBuilderKind { Rust { - attrs: Vec, - ident: Ident, - tokens: proc_macro2::TokenStream, - emitted_any_variants: bool, + non_exhaustive: bool, }, NewType { - canonical_name: &'a str, - tokens: proc_macro2::TokenStream, is_bitfield: bool, is_global: bool, + /// if the enum is named or not. + is_anonymous: bool, }, Consts { - variants: Vec, + needs_typedef: bool, }, ModuleConsts { - module_name: &'a str, - module_items: Vec, + module_name: Ident, }, } -impl<'a> EnumBuilder<'a> { +impl EnumBuilder { /// Returns true if the builder is for a rustified enum. fn is_rust_enum(&self) -> bool { - matches!(*self, EnumBuilder::Rust { .. }) + matches!(self.kind, EnumBuilderKind::Rust { .. }) } /// Create a new enum given an item builder, a canonical name, a name for /// the representation, and which variation it should be generated as. fn new( - name: &'a str, - mut attrs: Vec, + name: &str, + attrs: Vec, repr: &syn::Type, enum_variation: EnumVariation, has_typedef: bool, + enum_is_anonymous: bool, ) -> Self { let ident = Ident::new(name, Span::call_site()); + // For most variants this is the same + let mut enum_ty = ident.clone(); - match enum_variation { + let kind = match enum_variation { EnumVariation::NewType { is_bitfield, is_global, - } => EnumBuilder::NewType { - canonical_name: name, - tokens: quote! { - #( #attrs )* - pub struct #ident (pub #repr); - }, + } => EnumBuilderKind::NewType { is_bitfield, is_global, + is_anonymous: enum_is_anonymous, }, - EnumVariation::Rust { .. } => { - // `repr` is guaranteed to be Rustified in Enum::codegen - attrs.insert(0, quote! { #[repr( #repr )] }); - let tokens = quote!(); - EnumBuilder::Rust { - attrs, - ident, - tokens, - emitted_any_variants: false, - } + EnumVariation::Rust { non_exhaustive } => { + EnumBuilderKind::Rust { non_exhaustive } } - EnumVariation::Consts => { - let mut variants = Vec::new(); - - if !has_typedef { - variants.push(quote! { - #( #attrs )* - pub type #ident = #repr; - }); - } - - EnumBuilder::Consts { variants } - } + EnumVariation::Consts => EnumBuilderKind::Consts { + needs_typedef: !has_typedef, + }, EnumVariation::ModuleConsts => { - let ident = Ident::new( + enum_ty = Ident::new( CONSTIFIED_ENUM_MODULE_REPR_NAME, Span::call_site(), ); - let type_definition = quote! { - #( #attrs )* - pub type #ident = #repr; - }; - EnumBuilder::ModuleConsts { - module_name: name, - module_items: vec![type_definition], + EnumBuilderKind::ModuleConsts { + module_name: ident.clone(), } } + }; + EnumBuilder { + enum_type: enum_ty, + attrs, + repr: repr.clone(), + kind, + enum_variants: vec![], } } /// Add a variant to this enum. fn with_variant( - self, + mut self, ctx: &BindgenContext, variant: &EnumVariant, + variant_doc: proc_macro2::TokenStream, mangling_prefix: Option<&str>, rust_ty: &syn::Type, - result: &mut CodegenResult<'_>, is_ty_named: bool, ) -> Self { let variant_name = ctx.rust_mangle(variant.name()); @@ -3384,66 +3379,38 @@ impl<'a> EnumBuilder<'a> { EnumVariantValue::Unsigned(v) => helpers::ast_ty::uint_expr(v), }; - let mut doc = quote! {}; - if ctx.options().generate_comments { - if let Some(raw_comment) = variant.comment() { - let comment = ctx.options().process_comment(raw_comment); - doc = attributes::doc(&comment); - } - } - - match self { - EnumBuilder::Rust { - attrs, - ident, - tokens, - emitted_any_variants: _, - } => { + match self.kind { + EnumBuilderKind::Rust { .. } => { let name = ctx.rust_ident(variant_name); - EnumBuilder::Rust { - attrs, - ident, - tokens: quote! { - #tokens - #doc - #name = #expr, - }, - emitted_any_variants: true, - } + self.enum_variants.push(EnumVariantInfo { + variant_name: name, + variant_doc, + value: expr, + }); + self } - EnumBuilder::NewType { - canonical_name, - is_global, - .. - } => { - if is_ty_named && !is_global { - let enum_ident = ctx.rust_ident(canonical_name); - let variant_ident = ctx.rust_ident(variant_name); - - result.push(quote! { - impl #enum_ident { - #doc - pub const #variant_ident : #rust_ty = #rust_ty ( #expr ); - } - }); + EnumBuilderKind::NewType { is_global, .. } => { + let variant_ident = if is_ty_named && !is_global { + ctx.rust_ident(variant_name) } else { - let ident = ctx.rust_ident(match mangling_prefix { + ctx.rust_ident(match mangling_prefix { Some(prefix) => { Cow::Owned(format!("{prefix}_{variant_name}")) } None => variant_name, - }); - result.push(quote! { - #doc - pub const #ident : #rust_ty = #rust_ty ( #expr ); - }); - } + }) + }; + self.enum_variants.push(EnumVariantInfo { + variant_name: variant_ident, + variant_doc, + value: quote! { #rust_ty ( #expr )}, + }); self } - EnumBuilder::Consts { .. } => { + EnumBuilderKind::Consts { .. } => { let constant_name = match mangling_prefix { Some(prefix) => { Cow::Owned(format!("{prefix}_{variant_name}")) @@ -3452,27 +3419,58 @@ impl<'a> EnumBuilder<'a> { }; let ident = ctx.rust_ident(constant_name); - result.push(quote! { - #doc - pub const #ident : #rust_ty = #expr ; + self.enum_variants.push(EnumVariantInfo { + variant_name: ident, + variant_doc, + value: quote! { #expr }, }); self } - EnumBuilder::ModuleConsts { - module_name, - mut module_items, - } => { + EnumBuilderKind::ModuleConsts { .. } => { let name = ctx.rust_ident(variant_name); - let ty = ctx.rust_ident(CONSTIFIED_ENUM_MODULE_REPR_NAME); - module_items.push(quote! { - #doc - pub const #name : #ty = #expr ; + self.enum_variants.push(EnumVariantInfo { + variant_name: name, + variant_doc, + value: quote! { #expr }, }); + self + } + } + } + + fn newtype_bitfield_impl( + prefix: Ident, + rust_ty: &syn::Type, + ) -> proc_macro2::TokenStream { + let rust_ty_name = &rust_ty; + quote! { + impl ::#prefix::ops::BitOr<#rust_ty> for #rust_ty { + type Output = Self; + + #[inline] + fn bitor(self, other: Self) -> Self { + #rust_ty_name(self.0 | other.0) + } + } + impl ::#prefix::ops::BitOrAssign for #rust_ty { + #[inline] + fn bitor_assign(&mut self, rhs: #rust_ty) { + self.0 |= rhs.0; + } + } + impl ::#prefix::ops::BitAnd<#rust_ty> for #rust_ty { + type Output = Self; - EnumBuilder::ModuleConsts { - module_name, - module_items, + #[inline] + fn bitand(self, other: Self) -> Self { + #rust_ty_name(self.0 & other.0) + } + } + impl ::#prefix::ops::BitAndAssign for #rust_ty { + #[inline] + fn bitand_assign(&mut self, rhs: #rust_ty) { + self.0 &= rhs.0; } } } @@ -3482,94 +3480,141 @@ impl<'a> EnumBuilder<'a> { self, ctx: &BindgenContext, rust_ty: &syn::Type, - result: &mut CodegenResult<'_>, ) -> proc_macro2::TokenStream { - match self { - EnumBuilder::Rust { - attrs, - ident, - tokens, - emitted_any_variants, - .. - } => { - let variants = if emitted_any_variants { - tokens - } else { - quote!(__bindgen_cannot_repr_c_on_empty_enum = 0) - }; + let enum_ident = self.enum_type; + + // 1. Construct a list of the enum variants + let variants = match self.kind { + EnumBuilderKind::Rust { .. } => { + let mut variants = vec![]; + + for v in self.enum_variants { + let variant_doc = &v.variant_doc; + let variant_ident = &v.variant_name; + let variant_value = &v.value; + + variants.push(quote! { + #variant_doc + #variant_ident = #variant_value, + }); + } + + if variants.is_empty() { + variants.push( + quote! {__bindgen_cannot_repr_c_on_empty_enum = 0,}, + ); + } + variants + } + EnumBuilderKind::NewType { .. } => { + let mut variants = vec![]; + + for v in self.enum_variants { + let variant_doc = &v.variant_doc; + let variant_ident = &v.variant_name; + let variant_value = &v.value; + + variants.push(quote! { + #variant_doc + pub const #variant_ident: #enum_ident = #variant_value; + }); + } + variants + } + EnumBuilderKind::Consts { .. } | + EnumBuilderKind::ModuleConsts { .. } => { + let mut variants = vec![]; + + for v in self.enum_variants { + let variant_doc = &v.variant_doc; + let variant_ident = &v.variant_name; + let variant_value = &v.value; + + variants.push(quote! { + #variant_doc + pub const #variant_ident: #enum_ident = #variant_value; + }); + } + variants + } + }; + let attrs = self.attrs; + let enum_repr = &self.repr; + + // 2. Generate the enum representation + match self.kind { + EnumBuilderKind::Rust { non_exhaustive } => { + let non_exhaustive_opt = + non_exhaustive.then(attributes::non_exhaustive); quote! { + // Note: repr is on top of attrs to keep the test expectations diff small. + // a future commit could move it further down. + #[repr(#enum_repr)] + #non_exhaustive_opt #( #attrs )* - pub enum #ident { - #variants + pub enum #enum_ident { + #( #variants )* } } } - EnumBuilder::NewType { - canonical_name, - tokens, + EnumBuilderKind::NewType { is_bitfield, - .. + is_global, + is_anonymous, } => { - if !is_bitfield { - return tokens; - } - - let rust_ty_name = ctx.rust_ident_raw(canonical_name); - let prefix = ctx.trait_prefix(); - - result.push(quote! { - impl ::#prefix::ops::BitOr<#rust_ty> for #rust_ty { - type Output = Self; - - #[inline] - fn bitor(self, other: Self) -> Self { - #rust_ty_name(self.0 | other.0) - } + // There doesn't seem to be a technical reason why we generate + // anon enum variants as global constants. + // We keep this behavior to avoid breaking changes in the bindings. + let impl_variants = if is_anonymous || is_global { + quote! { + #( #variants )* } - }); - - result.push(quote! { - impl ::#prefix::ops::BitOrAssign for #rust_ty { - #[inline] - fn bitor_assign(&mut self, rhs: #rust_ty) { - self.0 |= rhs.0; + } else { + quote! { + impl #enum_ident { + #( #variants )* } } - }); + }; - result.push(quote! { - impl ::#prefix::ops::BitAnd<#rust_ty> for #rust_ty { - type Output = Self; + let prefix = ctx.trait_prefix(); + let bitfield_impl_opt = is_bitfield + .then(|| Self::newtype_bitfield_impl(prefix, rust_ty)); - #[inline] - fn bitand(self, other: Self) -> Self { - #rust_ty_name(self.0 & other.0) - } - } - }); + quote! { + // Previously variant impls where before the enum definition. + // lets keep this as is for now, to reduce the diff in generated bindings. + #impl_variants - result.push(quote! { - impl ::#prefix::ops::BitAndAssign for #rust_ty { - #[inline] - fn bitand_assign(&mut self, rhs: #rust_ty) { - self.0 &= rhs.0; - } + #bitfield_impl_opt + + #[repr(transparent)] + #( #attrs )* + pub struct #enum_ident (pub #enum_repr); + } + } + EnumBuilderKind::Consts { needs_typedef } => { + let typedef_opt = needs_typedef.then(|| { + quote! { + #( #attrs )* + pub type #enum_ident = #enum_repr; } }); + quote! { + #( #variants )* - tokens + #typedef_opt + } } - EnumBuilder::Consts { variants, .. } => quote! { #( #variants )* }, - EnumBuilder::ModuleConsts { - module_items, - module_name, - .. - } => { - let ident = ctx.rust_ident(module_name); + EnumBuilderKind::ModuleConsts { module_name, .. } => { quote! { - pub mod #ident { - #( #module_items )* + // todo: Probably some attributes, e.g. `cfg` should apply to the `mod`. + pub mod #module_name { + #( #attrs )* + pub type #enum_ident = #enum_repr; + + #( #variants )* } } } @@ -3656,29 +3701,6 @@ impl CodeGenerator for Enum { let mut attrs = vec![]; - // TODO(emilio): Delegate this to the builders? - match variation { - EnumVariation::Rust { non_exhaustive } => { - if non_exhaustive && - ctx.options().rust_features().non_exhaustive - { - attrs.push(attributes::non_exhaustive()); - } else if non_exhaustive && - !ctx.options().rust_features().non_exhaustive - { - panic!("The rust target you're using doesn't seem to support non_exhaustive enums"); - } - } - EnumVariation::NewType { .. } => { - if true { - attrs.push(attributes::repr("transparent")); - } else { - attrs.push(attributes::repr("C")); - } - } - _ => {} - }; - if let Some(comment) = item.comment(ctx) { attrs.push(attributes::doc(&comment)); } @@ -3779,8 +3801,14 @@ impl CodeGenerator for Enum { ); }); - let mut builder = - EnumBuilder::new(&name, attrs, &repr, variation, has_typedef); + let mut builder = EnumBuilder::new( + &name, + attrs, + &repr, + variation, + has_typedef, + enum_ty.name().is_none(), + ); // A map where we keep a value -> variant relation. let mut seen_values = HashMap::<_, Ident>::default(); @@ -3822,6 +3850,15 @@ impl CodeGenerator for Enum { continue; } + let mut variant_doc = quote! {}; + if ctx.options().generate_comments { + if let Some(raw_comment) = variant.comment() { + let processed_comment = + ctx.options().process_comment(raw_comment); + variant_doc = attributes::doc(&processed_comment); + } + } + match seen_values.entry(variant.val()) { Entry::Occupied(ref entry) => { if variation.is_rust() { @@ -3864,9 +3901,9 @@ impl CodeGenerator for Enum { builder = builder.with_variant( ctx, variant, + variant_doc, constant_mangling_prefix, &enum_rust_ty, - result, enum_ty.name().is_some(), ); } @@ -3875,9 +3912,9 @@ impl CodeGenerator for Enum { builder = builder.with_variant( ctx, variant, + variant_doc, constant_mangling_prefix, &enum_rust_ty, - result, enum_ty.name().is_some(), ); @@ -3917,11 +3954,17 @@ impl CodeGenerator for Enum { } } - let item = builder.build(ctx, &enum_rust_ty, result); + let item = builder.build(ctx, &enum_rust_ty); result.push(item); } } +struct EnumVariantInfo { + variant_name: Ident, + variant_doc: proc_macro2::TokenStream, + value: proc_macro2::TokenStream, +} + /// Enum for the default type of macro constants. #[derive(Copy, Clone, PartialEq, Eq, Debug, Default)] pub enum MacroTypeVariation {