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

don't emit #[repr(align(0))] for empty unions #1595

Merged
merged 2 commits into from Jul 23, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/ir/comp.rs
Expand Up @@ -1078,8 +1078,14 @@ impl CompInfo {
return None;
}

// empty union case
if self.fields().is_empty() {
return None;
}

let mut max_size = 0;
let mut max_align = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead, we should probably return None if we didn't find any of the field layouts, but this is fine too I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

returning None if none the field fields have a layout cause theses tests to fail internally with 'Unable to get layout information?', so leaving this as-is for now.

header_issue_493_1_0_hpp
header_issue_493_hpp
header_transform_op_hpp

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, thanks :)

// Don't allow align(0)
let mut max_align = 1;
for field in self.fields() {
let field_layout = field.layout(ctx);

Expand Down
19 changes: 19 additions & 0 deletions tests/expectations/tests/empty-union.rs
@@ -0,0 +1,19 @@
/* automatically generated by rust-bindgen */

#![allow(
dead_code,
non_snake_case,
non_camel_case_types,
non_upper_case_globals
)]

#[repr(C)]
#[derive(Copy, Clone)]
pub union a__bindgen_ty_1 {
pub _address: u8,
}
impl Default for a__bindgen_ty_1 {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
28 changes: 9 additions & 19 deletions tests/expectations/tests/issue-493_1_0.rs
@@ -1,6 +1,11 @@
/* automatically generated by rust-bindgen */

#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
#![allow(
dead_code,
non_snake_case,
non_camel_case_types,
non_upper_case_globals
)]

#[repr(C)]
pub struct __BindgenUnionField<T>(::std::marker::PhantomData<T>);
Expand Down Expand Up @@ -91,17 +96,12 @@ impl Default for basic_string___short {
}
}
#[repr(C)]
#[derive(Copy, Clone)]
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
pub struct basic_string___ulx {
pub __lx: __BindgenUnionField<basic_string___long>,
pub __lxx: __BindgenUnionField<basic_string___short>,
pub bindgen_union_field: [u8; 0usize],
}
impl Default for basic_string___ulx {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
pub const basic_string___n_words: basic_string__bindgen_ty_2 =
basic_string__bindgen_ty_2::__n_words;
#[repr(i32)]
Expand All @@ -120,25 +120,15 @@ impl Default for basic_string___raw {
}
}
#[repr(C)]
#[derive(Copy, Clone)]
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
pub struct basic_string___rep {
pub __bindgen_anon_1: basic_string___rep__bindgen_ty_1,
}
#[repr(C)]
#[derive(Copy, Clone)]
#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
pub struct basic_string___rep__bindgen_ty_1 {
pub __l: __BindgenUnionField<basic_string___long>,
pub __s: __BindgenUnionField<basic_string___short>,
pub __r: __BindgenUnionField<basic_string___raw>,
pub bindgen_union_field: [u8; 0usize],
}
impl Default for basic_string___rep__bindgen_ty_1 {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
impl Default for basic_string___rep {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
16 changes: 3 additions & 13 deletions tests/expectations/tests/transform-op.rs
Expand Up @@ -63,7 +63,7 @@ impl<T> Default for StylePoint<T> {
}
}
#[repr(C)]
#[derive(Copy, Clone)]
#[derive(Debug, Default, Copy, Clone)]
pub struct StyleFoo<T> {
pub __bindgen_anon_1: __BindgenUnionField<StyleFoo__bindgen_ty_1>,
pub foo: __BindgenUnionField<StyleFoo_Foo_Body<T>>,
Expand Down Expand Up @@ -125,13 +125,8 @@ impl Default for StyleFoo__bindgen_ty_1 {
unsafe { ::std::mem::zeroed() }
}
}
impl<T> Default for StyleFoo<T> {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
#[repr(C)]
#[derive(Copy, Clone)]
#[derive(Debug, Copy, Clone)]
pub struct StyleBar<T> {
pub tag: StyleBar_Tag,
pub __bindgen_anon_1: StyleBar__bindgen_ty_1<T>,
Expand Down Expand Up @@ -178,19 +173,14 @@ impl<T> Default for StyleBar_StyleBar3_Body<T> {
}
}
#[repr(C)]
#[derive(Copy, Clone)]
#[derive(Debug, Default, Copy, Clone)]
pub struct StyleBar__bindgen_ty_1<T> {
pub bar1: __BindgenUnionField<StyleBar_StyleBar1_Body<T>>,
pub bar2: __BindgenUnionField<StyleBar_StyleBar2_Body<T>>,
pub bar3: __BindgenUnionField<StyleBar_StyleBar3_Body<T>>,
pub bindgen_union_field: [u8; 0usize],
pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>>,
}
impl<T> Default for StyleBar__bindgen_ty_1<T> {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
impl<T> Default for StyleBar<T> {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
Expand Down
5 changes: 5 additions & 0 deletions tests/headers/empty-union.hpp
@@ -0,0 +1,5 @@
// bindgen-flags: --opaque-type "*"

template <int> class a {
union {};
};