diff options
author | David Tolnay <dtolnay@gmail.com> | 2022-11-28 06:40:20 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-28 09:40:20 -0500 |
commit | 0a78cde484fd5b6a5bbfd4301b3c4cf043a60a38 (patch) | |
tree | ec24cc604a33a0443999b77335fc29e70a722f2b | |
parent | 95fd17b874910184cc0fcd33b287fa4e205d9d7a (diff) |
Fix name collision between C enum and typedef (#2326)
Fixes #2008.
Example:
```c
enum Enum { Variant };
typedef int16_t Enum;
```
This is valid and idiomatic C (though not valid C++). `cbindgen` uses this idiom as the default C translation of Rust enums, the equivalent of what would be `enum Enum : int16_t { Variant };` in C++.
`bindgen header.h` before:
```rust
pub const Enum_Variant: Enum = 0;
pub type Enum = ::std::os::raw::c_uint;
pub type Enum = i16;
```
```console
error[E0428]: the name `Enum` is defined multiple times
--> generated.rs:3:1
|
2 | pub type Enum = ::std::os::raw::c_uint;
| --------------------------------------- previous definition of the type `Enum` here
3 | pub type Enum = i16;
| ^^^^^^^^^^^^^^^^^^^^ `Enum` redefined here
|
= note: `Enum` must be defined only once in the type namespace of this module
```
After:
```rust
pub const Enum_Variant: Enum = 0;
pub type Enum = i16;
```
-rw-r--r-- | bindgen-tests/tests/expectations/tests/enum-typedef.rs | 11 | ||||
-rw-r--r-- | bindgen-tests/tests/headers/enum-typedef.h | 18 | ||||
-rw-r--r-- | bindgen/codegen/mod.rs | 15 | ||||
-rw-r--r-- | bindgen/ir/context.rs | 82 | ||||
-rw-r--r-- | bindgen/ir/ty.rs | 5 |
5 files changed, 126 insertions, 5 deletions
diff --git a/bindgen-tests/tests/expectations/tests/enum-typedef.rs b/bindgen-tests/tests/expectations/tests/enum-typedef.rs new file mode 100644 index 00000000..dc78eb11 --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/enum-typedef.rs @@ -0,0 +1,11 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +pub const Enum_Variant: Enum = 0; +pub type Enum = i16; +pub type TypedefFirst = i16; +pub const TypedefFirst_Variant2: TypedefFirst = 0; diff --git a/bindgen-tests/tests/headers/enum-typedef.h b/bindgen-tests/tests/headers/enum-typedef.h new file mode 100644 index 00000000..f345f4de --- /dev/null +++ b/bindgen-tests/tests/headers/enum-typedef.h @@ -0,0 +1,18 @@ +typedef short int16_t; + +// `cbindgen` emits this C idiom as the translation of: +// +// #[repr(i16)] +// pub enum Enum { +// Variant, +// } +enum Enum { + Variant, +}; +typedef int16_t Enum; + +// C is also fine with the typedef coming before the enum. +typedef int16_t TypedefFirst; +enum TypedefFirst { + Variant2, +}; diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index b4050637..154d7fd1 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -2709,6 +2709,7 @@ impl<'a> EnumBuilder<'a> { mut attrs: Vec<proc_macro2::TokenStream>, repr: proc_macro2::TokenStream, enum_variation: EnumVariation, + has_typedef: bool, ) -> Self { let ident = Ident::new(name, Span::call_site()); @@ -2741,10 +2742,12 @@ impl<'a> EnumBuilder<'a> { EnumVariation::Consts => { let mut variants = Vec::new(); - variants.push(quote! { - #( #attrs )* - pub type #ident = #repr; - }); + if !has_typedef { + variants.push(quote! { + #( #attrs )* + pub type #ident = #repr; + }); + } EnumBuilder::Consts { variants } } @@ -3157,8 +3160,10 @@ impl CodeGenerator for Enum { } let repr = repr.to_rust_ty_or_opaque(ctx, item); + let has_typedef = ctx.is_enum_typedef_combo(item.id()); - let mut builder = EnumBuilder::new(&name, attrs, repr, variation); + let mut builder = + EnumBuilder::new(&name, attrs, repr, variation, has_typedef); // A map where we keep a value -> variant relation. let mut seen_values = HashMap::<_, Ident>::default(); diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index 3cc30f1c..c5df37d7 100644 --- a/bindgen/ir/context.rs +++ b/bindgen/ir/context.rs @@ -398,6 +398,22 @@ pub struct BindgenContext { /// bitfield allocation units computed. Drained in `compute_bitfield_units`. need_bitfield_allocation: Vec<ItemId>, + /// The set of enums that are defined by a pair of `enum` and `typedef`, + /// which is legal in C (but not C++). + /// + /// ```c++ + /// // in either order + /// enum Enum { Variants... }; + /// typedef int16_t Enum; + /// ``` + /// + /// The stored `ItemId` is that of the `TypeKind::Enum`, not of the + /// `TypeKind::Alias`. + /// + /// This is populated when we enter codegen by `compute_enum_typedef_combos` + /// and is always `None` before that and `Some` after. + enum_typedef_combos: Option<HashSet<ItemId>>, + /// The set of (`ItemId`s of) types that can't derive debug. /// /// This is populated when we enter codegen by `compute_cannot_derive_debug` @@ -565,6 +581,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" codegen_items: None, used_template_parameters: None, need_bitfield_allocation: Default::default(), + enum_typedef_combos: None, cannot_derive_debug: None, cannot_derive_default: None, cannot_derive_copy: None, @@ -1157,6 +1174,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" self.compute_sizedness(); self.compute_has_destructor(); self.find_used_template_parameters(); + self.compute_enum_typedef_combos(); self.compute_cannot_derive_debug(); self.compute_cannot_derive_default(); self.compute_cannot_derive_copy(); @@ -2476,6 +2494,70 @@ If you encounter an error missing from this list, please file an issue or a PR!" self.generated_bindgen_complex.get() } + /// Compute which `enum`s have an associated `typedef` definition. + fn compute_enum_typedef_combos(&mut self) { + let _t = self.timer("compute_enum_typedef_combos"); + assert!(self.enum_typedef_combos.is_none()); + + let mut enum_typedef_combos = HashSet::default(); + for item in &self.items { + if let Some(ItemKind::Module(module)) = + item.as_ref().map(Item::kind) + { + // Find typedefs in this module, and build set of their names. + let mut names_of_typedefs = HashSet::default(); + for child_id in module.children() { + if let Some(ItemKind::Type(ty)) = + self.items[child_id.0].as_ref().map(Item::kind) + { + if let (Some(name), TypeKind::Alias(type_id)) = + (ty.name(), ty.kind()) + { + // We disregard aliases that refer to the enum + // itself, such as in `typedef enum { ... } Enum;`. + if type_id + .into_resolver() + .through_type_refs() + .through_type_aliases() + .resolve(self) + .expect_type() + .is_int() + { + names_of_typedefs.insert(name); + } + } + } + } + + // Find enums in this module, and record the id of each one that + // has a typedef. + for child_id in module.children() { + if let Some(ItemKind::Type(ty)) = + self.items[child_id.0].as_ref().map(Item::kind) + { + if let (Some(name), true) = (ty.name(), ty.is_enum()) { + if names_of_typedefs.contains(name) { + enum_typedef_combos.insert(*child_id); + } + } + } + } + } + } + + self.enum_typedef_combos = Some(enum_typedef_combos); + } + + /// Look up whether `id` refers to an `enum` whose underlying type is + /// defined by a `typedef`. + pub fn is_enum_typedef_combo(&self, id: ItemId) -> bool { + assert!( + self.in_codegen_phase(), + "We only compute enum_typedef_combos when we enter codegen", + ); + self.enum_typedef_combos.as_ref().unwrap().contains(&id) + } + /// Compute whether we can derive debug. fn compute_cannot_derive_debug(&mut self) { let _t = self.timer("compute_cannot_derive_debug"); diff --git a/bindgen/ir/ty.rs b/bindgen/ir/ty.rs index ed3331ad..fef340de 100644 --- a/bindgen/ir/ty.rs +++ b/bindgen/ir/ty.rs @@ -95,6 +95,11 @@ impl Type { matches!(self.kind, TypeKind::BlockPointer(..)) } + /// Is this an integer type, including `bool` or `char`? + pub fn is_int(&self) -> bool { + matches!(self.kind, TypeKind::Int(_)) + } + /// Is this a compound type? pub fn is_comp(&self) -> bool { matches!(self.kind, TypeKind::Comp(..)) |