Skip to content

Commit

Permalink
ir: Deal with name conflicts correctly in declaration type resolution.
Browse files Browse the repository at this point in the history
So, basically, make opaque items last, and make previous names override
them.

Fixes #649
  • Loading branch information
emilio committed Jan 26, 2021
1 parent c47ee15 commit 7507458
Show file tree
Hide file tree
Showing 15 changed files with 258 additions and 36 deletions.
55 changes: 27 additions & 28 deletions src/bindgen/declarationtyperesolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)?
}
}
13 changes: 9 additions & 4 deletions src/bindgen/ir/enumeration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/bindgen/ir/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/bindgen/ir/typedef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
12 changes: 9 additions & 3 deletions src/bindgen/library.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
16 changes: 16 additions & 0 deletions tests/expectations/decl_name_conflicting.both.c
Original file line number Diff line number Diff line change
@@ -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);
30 changes: 30 additions & 0 deletions tests/expectations/decl_name_conflicting.both.compat.c
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions tests/expectations/decl_name_conflicting.c
Original file line number Diff line number Diff line change
@@ -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);
30 changes: 30 additions & 0 deletions tests/expectations/decl_name_conflicting.compat.c
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions tests/expectations/decl_name_conflicting.cpp
Original file line number Diff line number Diff line change
@@ -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"
17 changes: 17 additions & 0 deletions tests/expectations/decl_name_conflicting.pyx
Original file line number Diff line number Diff line change
@@ -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);
16 changes: 16 additions & 0 deletions tests/expectations/decl_name_conflicting.tag.c
Original file line number Diff line number Diff line change
@@ -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);
30 changes: 30 additions & 0 deletions tests/expectations/decl_name_conflicting.tag.compat.c
Original file line number Diff line number Diff line change
@@ -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
17 changes: 17 additions & 0 deletions tests/expectations/decl_name_conflicting.tag.pyx
Original file line number Diff line number Diff line change
@@ -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);
14 changes: 14 additions & 0 deletions tests/rust/decl_name_conflicting.rs
Original file line number Diff line number Diff line change
@@ -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) {}

0 comments on commit 7507458

Please sign in to comment.