Repository: mozilla/cbindgen (language: rust)

Issue:
Enums shadowed by names in module
I'm seeing misbehavior where an `enum` in a module causes mistranslation of a struct containing a `#[repr(u32)] enum` with the same name.

Here's a very simple test case:
`test.rs`:
```rust
mod uhoh {
    enum BindingType { Buffer, NotBuffer }
}

#[repr(u32)]
pub enum BindingType { Buffer = 0, NotBuffer = 1 }

#[repr(C)]
pub struct BindGroupLayoutEntry {
    pub ty: BindingType, // This is the repr(u32) enum
}
```

`cbindgen.toml`
```toml
language = "C"

[export]
include = [
    "BindGroupLayoutEntry",
]
```

Running `cbindgen 0.16.0`:
```
$ cbindgen -c cbindgen.toml test.rs
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>

enum BindingType {
  Buffer = 0,
  NotBuffer = 1,
};
typedef uint32_t BindingType;

typedef struct BindGroupLayoutEntry {
  struct BindingType ty;
} BindGroupLayoutEntry;
```

Having the member be `struct BindingType ty` is invalid, because there is no `struct BindingType` in the header; it should instead be `BindingType ty`.


Discussion / hints:
Did this use to work? I assume the version in the OP should be `0.16.0`, right?

Fix patch:
diff --git a/src/bindgen/declarationtyperesolver.rs b/src/bindgen/declarationtyperesolver.rs
index bbdca17d9..e72761e34 100644
--- a/src/bindgen/declarationtyperesolver.rs
+++ b/src/bindgen/declarationtyperesolver.rs
@@ -2,15 +2,18 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use std::collections::HashSet;
-
 use crate::bindgen::ir::Path;
+use std::collections::hash_map::Entry;
+use std::collections::HashMap;
 
-#[derive(Default)]
-pub struct DeclarationTypeResolver {
-    structs: HashSet<Path>,
-    enums: HashSet<Path>,
-    unions: HashSet<Path>,
+impl DeclarationType {
+    pub fn to_str(self) -> &'static str {
+        match self {
+            DeclarationType::Struct => "struct",
+            DeclarationType::Enum => "enum",
+            DeclarationType::Union => "union",
+        }
+    }
 }
 
 #[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)]
@@ -20,39 +23,35 @@ pub enum DeclarationType {
     Union,
 }
 
-impl DeclarationType {
-    pub fn to_str(self) -> &'static str {
-        match self {
-            DeclarationType::Struct => "struct",
-            DeclarationType::Enum => "enum",
-            DeclarationType::Union => "union",
-        }
-    }
+#[derive(Default)]
+pub struct DeclarationTypeResolver {
+    types: HashMap<Path, Option<DeclarationType>>,
 }
 
 impl DeclarationTypeResolver {
+    fn insert(&mut self, path: &Path, ty: Option<DeclarationType>) {
+        if let Entry::Vacant(vacant_entry) = self.types.entry(path.clone()) {
+            vacant_entry.insert(ty);
+        }
+    }
+
     pub fn add_enum(&mut self, path: &Path) {
-        self.enums.insert(path.clone());
+        self.insert(path, Some(DeclarationType::Enum));
     }
 
     pub fn add_struct(&mut self, path: &Path) {
-        self.structs.insert(path.clone());
+        self.insert(path, Some(DeclarationType::Struct));
     }
 
     pub fn add_union(&mut self, path: &Path) {
-        self.unions.insert(path.clone());
+        self.insert(path, Some(DeclarationType::Union));
+    }
+
+    pub fn add_none(&mut self, path: &Path) {
+        self.insert(path, None);
     }
 
     pub fn type_for(&self, path: &Path) -> Option<DeclarationType> {
-        // FIXME: don't look up by name, but by full path:
-        if self.structs.contains(path) {
-            Some(DeclarationType::Struct)
-        } else if self.enums.contains(path) {
-            Some(DeclarationType::Enum)
-        } else if self.unions.contains(path) {
-            Some(DeclarationType::Union)
-        } else {
-            None
-        }
+        *self.types.get(path)?
     }
 }
diff --git a/src/bindgen/ir/enumeration.rs b/src/bindgen/ir/enumeration.rs
index 7359e19b0..9d96b4c3b 100644
--- a/src/bindgen/ir/enumeration.rs
+++ b/src/bindgen/ir/enumeration.rs
@@ -476,12 +476,17 @@ impl Item for Enum {
     }
 
     fn collect_declaration_types(&self, resolver: &mut DeclarationTypeResolver) {
-        if self.tag.is_some() && self.repr.style == ReprStyle::C {
-            resolver.add_struct(&self.path);
-        } else if self.tag.is_some() && self.repr.style != ReprStyle::C {
-            resolver.add_union(&self.path);
+        if self.tag.is_some() {
+            if self.repr.style == ReprStyle::C {
+                resolver.add_struct(&self.path);
+            } else {
+                resolver.add_union(&self.path);
+            }
         } else if self.repr.style == ReprStyle::C {
             resolver.add_enum(&self.path);
+        } else {
+            // This is important to handle conflicting names with opaque items.
+            resolver.add_none(&self.path);
         }
     }
 
diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs
index 86867585e..05539eb1d 100644
--- a/src/bindgen/ir/structure.rs
+++ b/src/bindgen/ir/structure.rs
@@ -266,7 +266,9 @@ impl Item for Struct {
     }
 
     fn collect_declaration_types(&self, resolver: &mut DeclarationTypeResolver) {
-        if !self.is_transparent {
+        if self.is_transparent {
+            resolver.add_none(&self.path);
+        } else {
             resolver.add_struct(&self.path);
         }
     }
diff --git a/src/bindgen/ir/typedef.rs b/src/bindgen/ir/typedef.rs
index 9c779a408..61c071b54 100644
--- a/src/bindgen/ir/typedef.rs
+++ b/src/bindgen/ir/typedef.rs
@@ -138,6 +138,10 @@ impl Item for Typedef {
         self.aliased.rename_for_config(config, &self.generic_params);
     }
 
+    fn collect_declaration_types(&self, resolver: &mut DeclarationTypeResolver) {
+        resolver.add_none(&self.path);
+    }
+
     fn resolve_declaration_types(&mut self, resolver: &DeclarationTypeResolver) {
         self.aliased.resolve_declaration_types(resolver);
     }
diff --git a/src/bindgen/library.rs b/src/bindgen/library.rs
index 855cca2cc..b1f9eb457 100644
--- a/src/bindgen/library.rs
+++ b/src/bindgen/library.rs
@@ -318,15 +318,21 @@ impl Library {
             x.collect_declaration_types(&mut resolver);
         });
 
-        self.opaque_items.for_all_items(|x| {
+        self.enums.for_all_items(|x| {
             x.collect_declaration_types(&mut resolver);
         });
 
-        self.enums.for_all_items(|x| {
+        self.unions.for_all_items(|x| {
             x.collect_declaration_types(&mut resolver);
         });
 
-        self.unions.for_all_items(|x| {
+        self.typedefs.for_all_items(|x| {
+            x.collect_declaration_types(&mut resolver);
+        });
+
+        // NOTE: Intentionally last, so that in case there's an opaque type
+        // which is conflicting with a non-opaque one, the later wins.
+        self.opaque_items.for_all_items(|x| {
             x.collect_declaration_types(&mut resolver);
         });
 
diff --git a/src/bindgen/parser.rs b/src/bindgen/parser.rs
index a8e165ad4..ebb393511 100644
--- a/src/bindgen/parser.rs
+++ b/src/bindgen/parser.rs
@@ -653,7 +653,7 @@ impl Parse {
         }
 
         let loggable_item_name = || {
-            let mut items = vec![];
+            let mut items = Vec::with_capacity(3);
             items.push(crate_name.to_owned());
             if let Some(ref self_type) = self_type {
                 items.push(self_type.to_string());


Test patch:
diff --git a/tests/expectations/decl_name_conflicting.both.c b/tests/expectations/decl_name_conflicting.both.c
new file mode 100644
index 000000000..486b31c60
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.both.c
@@ -0,0 +1,16 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+enum BindingType {
+  Buffer = 0,
+  NotBuffer = 1,
+};
+typedef uint32_t BindingType;
+
+typedef struct BindGroupLayoutEntry {
+  BindingType ty;
+} BindGroupLayoutEntry;
+
+void root(struct BindGroupLayoutEntry entry);
diff --git a/tests/expectations/decl_name_conflicting.both.compat.c b/tests/expectations/decl_name_conflicting.both.compat.c
new file mode 100644
index 000000000..e8a8d499a
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.both.compat.c
@@ -0,0 +1,30 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+enum BindingType
+#ifdef __cplusplus
+  : uint32_t
+#endif // __cplusplus
+ {
+  Buffer = 0,
+  NotBuffer = 1,
+};
+#ifndef __cplusplus
+typedef uint32_t BindingType;
+#endif // __cplusplus
+
+typedef struct BindGroupLayoutEntry {
+  BindingType ty;
+} BindGroupLayoutEntry;
+
+#ifdef __cplusplus
+extern "C" {
+#endif // __cplusplus
+
+void root(struct BindGroupLayoutEntry entry);
+
+#ifdef __cplusplus
+} // extern "C"
+#endif // __cplusplus
diff --git a/tests/expectations/decl_name_conflicting.c b/tests/expectations/decl_name_conflicting.c
new file mode 100644
index 000000000..41a148e4d
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.c
@@ -0,0 +1,16 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+enum BindingType {
+  Buffer = 0,
+  NotBuffer = 1,
+};
+typedef uint32_t BindingType;
+
+typedef struct {
+  BindingType ty;
+} BindGroupLayoutEntry;
+
+void root(BindGroupLayoutEntry entry);
diff --git a/tests/expectations/decl_name_conflicting.compat.c b/tests/expectations/decl_name_conflicting.compat.c
new file mode 100644
index 000000000..a78700fb2
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.compat.c
@@ -0,0 +1,30 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+enum BindingType
+#ifdef __cplusplus
+  : uint32_t
+#endif // __cplusplus
+ {
+  Buffer = 0,
+  NotBuffer = 1,
+};
+#ifndef __cplusplus
+typedef uint32_t BindingType;
+#endif // __cplusplus
+
+typedef struct {
+  BindingType ty;
+} BindGroupLayoutEntry;
+
+#ifdef __cplusplus
+extern "C" {
+#endif // __cplusplus
+
+void root(BindGroupLayoutEntry entry);
+
+#ifdef __cplusplus
+} // extern "C"
+#endif // __cplusplus
diff --git a/tests/expectations/decl_name_conflicting.cpp b/tests/expectations/decl_name_conflicting.cpp
new file mode 100644
index 000000000..74c2ae1cb
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.cpp
@@ -0,0 +1,20 @@
+#include <cstdarg>
+#include <cstdint>
+#include <cstdlib>
+#include <ostream>
+#include <new>
+
+enum class BindingType : uint32_t {
+  Buffer = 0,
+  NotBuffer = 1,
+};
+
+struct BindGroupLayoutEntry {
+  BindingType ty;
+};
+
+extern "C" {
+
+void root(BindGroupLayoutEntry entry);
+
+} // extern "C"
diff --git a/tests/expectations/decl_name_conflicting.pyx b/tests/expectations/decl_name_conflicting.pyx
new file mode 100644
index 000000000..d7dbc5e64
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.pyx
@@ -0,0 +1,17 @@
+from libc.stdint cimport int8_t, int16_t, int32_t, int64_t, intptr_t
+from libc.stdint cimport uint8_t, uint16_t, uint32_t, uint64_t, uintptr_t
+cdef extern from *:
+  ctypedef bint bool
+  ctypedef struct va_list
+
+cdef extern from *:
+
+  cdef enum:
+    Buffer # = 0,
+    NotBuffer # = 1,
+  ctypedef uint32_t BindingType;
+
+  ctypedef struct BindGroupLayoutEntry:
+    BindingType ty;
+
+  void root(BindGroupLayoutEntry entry);
diff --git a/tests/expectations/decl_name_conflicting.tag.c b/tests/expectations/decl_name_conflicting.tag.c
new file mode 100644
index 000000000..7504e4aad
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.tag.c
@@ -0,0 +1,16 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+enum BindingType {
+  Buffer = 0,
+  NotBuffer = 1,
+};
+typedef uint32_t BindingType;
+
+struct BindGroupLayoutEntry {
+  BindingType ty;
+};
+
+void root(struct BindGroupLayoutEntry entry);
diff --git a/tests/expectations/decl_name_conflicting.tag.compat.c b/tests/expectations/decl_name_conflicting.tag.compat.c
new file mode 100644
index 000000000..601175650
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.tag.compat.c
@@ -0,0 +1,30 @@
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+enum BindingType
+#ifdef __cplusplus
+  : uint32_t
+#endif // __cplusplus
+ {
+  Buffer = 0,
+  NotBuffer = 1,
+};
+#ifndef __cplusplus
+typedef uint32_t BindingType;
+#endif // __cplusplus
+
+struct BindGroupLayoutEntry {
+  BindingType ty;
+};
+
+#ifdef __cplusplus
+extern "C" {
+#endif // __cplusplus
+
+void root(struct BindGroupLayoutEntry entry);
+
+#ifdef __cplusplus
+} // extern "C"
+#endif // __cplusplus
diff --git a/tests/expectations/decl_name_conflicting.tag.pyx b/tests/expectations/decl_name_conflicting.tag.pyx
new file mode 100644
index 000000000..82ebc2f01
--- /dev/null
+++ b/tests/expectations/decl_name_conflicting.tag.pyx
@@ -0,0 +1,17 @@
+from libc.stdint cimport int8_t, int16_t, int32_t, int64_t, intptr_t
+from libc.stdint cimport uint8_t, uint16_t, uint32_t, uint64_t, uintptr_t
+cdef extern from *:
+  ctypedef bint bool
+  ctypedef struct va_list
+
+cdef extern from *:
+
+  cdef enum:
+    Buffer # = 0,
+    NotBuffer # = 1,
+  ctypedef uint32_t BindingType;
+
+  cdef struct BindGroupLayoutEntry:
+    BindingType ty;
+
+  void root(BindGroupLayoutEntry entry);
diff --git a/tests/rust/decl_name_conflicting.rs b/tests/rust/decl_name_conflicting.rs
new file mode 100644
index 000000000..39e9658d3
--- /dev/null
+++ b/tests/rust/decl_name_conflicting.rs
@@ -0,0 +1,14 @@
+mod uhoh {
+    enum BindingType { Buffer, NotBuffer }
+}
+
+#[repr(u32)]
+pub enum BindingType { Buffer = 0, NotBuffer = 1 }
+
+#[repr(C)]
+pub struct BindGroupLayoutEntry {
+    pub ty: BindingType, // This is the repr(u32) enum
+}
+
+#[no_mangle]
+pub extern "C" fn root(entry: BindGroupLayoutEntry) {}