From 9dedf005c88ef64976c50932a8a6ee0d3a9c4778 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 11 Oct 2022 10:49:59 -0500 Subject: [PATCH 1/2] Handle the `const struct *` and `struct *` patterns. Given that C keeps a different namespace for `struct`s and aliases. The following patterns ```c typedef const struct foo { void *inner; } *foo; typedef struct bar { void *inner; } *bar; ``` are valid C code and produces both a `struct` and a pointer called `foo` and `bar` in different namespaces. Given that Rust does not make this distinction, we add the `_ptr` prefix to the pointer type aliases to avoid any name collisions. --- .../tests/expectations/tests/struct_ptr.rs | 83 +++++++++++++++++++ bindgen-tests/tests/headers/struct_ptr.h | 12 +++ bindgen/ir/ty.rs | 29 ++++++- 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 bindgen-tests/tests/expectations/tests/struct_ptr.rs create mode 100644 bindgen-tests/tests/headers/struct_ptr.h diff --git a/bindgen-tests/tests/expectations/tests/struct_ptr.rs b/bindgen-tests/tests/expectations/tests/struct_ptr.rs new file mode 100644 index 0000000000..80e757f4d5 --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/struct_ptr.rs @@ -0,0 +1,83 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct foo { + pub inner: ::std::os::raw::c_char, +} +#[test] +fn bindgen_test_layout_foo() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 1usize, + concat!("Size of: ", stringify!(foo)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(foo)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).inner) as usize - ptr as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(foo), + "::", + stringify!(inner) + ) + ); +} +pub type foo_ptr = *const foo; +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct bar { + pub inner: ::std::os::raw::c_char, +} +#[test] +fn bindgen_test_layout_bar() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 1usize, + concat!("Size of: ", stringify!(bar)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(bar)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).inner) as usize - ptr as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(bar), + "::", + stringify!(inner) + ) + ); +} +pub type bar_ptr = *mut bar; +extern "C" { + pub fn takes_foo_ptr(arg1: foo_ptr); +} +extern "C" { + pub fn takes_foo_struct(arg1: foo); +} +extern "C" { + pub fn takes_bar_ptr(arg1: bar_ptr); +} +extern "C" { + pub fn takes_bar_struct(arg1: bar); +} diff --git a/bindgen-tests/tests/headers/struct_ptr.h b/bindgen-tests/tests/headers/struct_ptr.h new file mode 100644 index 0000000000..656f2aece5 --- /dev/null +++ b/bindgen-tests/tests/headers/struct_ptr.h @@ -0,0 +1,12 @@ +typedef const struct foo { + char inner; +} *foo; + +typedef struct bar { + char inner; +} *bar; + +void takes_foo_ptr(foo); +void takes_foo_struct(struct foo); +void takes_bar_ptr(bar); +void takes_bar_struct(struct bar); diff --git a/bindgen/ir/ty.rs b/bindgen/ir/ty.rs index 9edc43d419..726c743890 100644 --- a/bindgen/ir/ty.rs +++ b/bindgen/ir/ty.rs @@ -1094,9 +1094,9 @@ impl Type { } CXType_Typedef => { let inner = cursor.typedef_type().expect("Not valid Type?"); - let inner = + let inner_id = Item::from_ty_or_ref(inner, location, None, ctx); - if inner == potential_id { + if inner_id == potential_id { warn!( "Generating oqaque type instead of self-referential \ typedef"); @@ -1104,7 +1104,30 @@ impl Type { // within the clang parsing. TypeKind::Opaque } else { - TypeKind::Alias(inner) + // Check if this type definition is an alias to a pointer of a `const + // struct` with the same name and add the `_ptr` suffix to it to avoid name + // collisions. + if !ctx.options().c_naming { + if let Some(pointee_spelling) = + inner.pointee_type().map(|ty| ty.spelling()) + { + if let (Some(pointee_name), Some(name)) = ( + pointee_spelling + .strip_prefix("const struct ") + .or_else(|| { + pointee_spelling + .strip_prefix("struct ") + }), + name.as_mut(), + ) { + if pointee_name == name { + *name += "_ptr"; + } + } + } + } + + TypeKind::Alias(inner_id) } } CXType_Enum => { From a147de16e27e5c5cad7bc547d3f6d085921862de Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 10 Nov 2022 10:31:02 -0500 Subject: [PATCH 2/2] handle `enum` and `union` as well --- ...ruct_ptr.rs => typedef-pointer-overlap.rs} | 70 +++++++++++++++++++ bindgen-tests/tests/headers/struct_ptr.h | 12 ---- .../tests/headers/typedef-pointer-overlap.h | 30 ++++++++ bindgen/ir/ty.rs | 30 +++----- 4 files changed, 111 insertions(+), 31 deletions(-) rename bindgen-tests/tests/expectations/tests/{struct_ptr.rs => typedef-pointer-overlap.rs} (54%) delete mode 100644 bindgen-tests/tests/headers/struct_ptr.h create mode 100644 bindgen-tests/tests/headers/typedef-pointer-overlap.h diff --git a/bindgen-tests/tests/expectations/tests/struct_ptr.rs b/bindgen-tests/tests/expectations/tests/typedef-pointer-overlap.rs similarity index 54% rename from bindgen-tests/tests/expectations/tests/struct_ptr.rs rename to bindgen-tests/tests/expectations/tests/typedef-pointer-overlap.rs index 80e757f4d5..2d5c5aae8c 100644 --- a/bindgen-tests/tests/expectations/tests/struct_ptr.rs +++ b/bindgen-tests/tests/expectations/tests/typedef-pointer-overlap.rs @@ -69,6 +69,58 @@ fn bindgen_test_layout_bar() { ); } pub type bar_ptr = *mut bar; +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct baz { + _unused: [u8; 0], +} +pub type baz_ptr = *mut baz; +#[repr(C)] +#[derive(Copy, Clone)] +pub union cat { + pub standard_issue: ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_cat() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 4usize, + concat!("Size of: ", stringify!(cat)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(cat)) + ); + assert_eq!( + unsafe { + ::std::ptr::addr_of!((*ptr).standard_issue) as usize - ptr as usize + }, + 0usize, + concat!( + "Offset of field: ", + stringify!(cat), + "::", + stringify!(standard_issue) + ) + ); +} +impl Default for cat { + fn default() -> Self { + let mut s = ::std::mem::MaybeUninit::::uninit(); + unsafe { + ::std::ptr::write_bytes(s.as_mut_ptr(), 0, 1); + s.assume_init() + } + } +} +pub type cat_ptr = *mut cat; +pub const mad_scientist: mad = 0; +pub type mad = ::std::os::raw::c_uint; +pub type mad_ptr = *mut mad; extern "C" { pub fn takes_foo_ptr(arg1: foo_ptr); } @@ -81,3 +133,21 @@ extern "C" { extern "C" { pub fn takes_bar_struct(arg1: bar); } +extern "C" { + pub fn takes_baz_ptr(arg1: baz_ptr); +} +extern "C" { + pub fn takes_baz_struct(arg1: baz); +} +extern "C" { + pub fn takes_cat_ptr(arg1: cat_ptr); +} +extern "C" { + pub fn takes_cat_union(arg1: cat); +} +extern "C" { + pub fn takes_mad_ptr(arg1: mad_ptr); +} +extern "C" { + pub fn takes_mad_enum(arg1: mad); +} diff --git a/bindgen-tests/tests/headers/struct_ptr.h b/bindgen-tests/tests/headers/struct_ptr.h deleted file mode 100644 index 656f2aece5..0000000000 --- a/bindgen-tests/tests/headers/struct_ptr.h +++ /dev/null @@ -1,12 +0,0 @@ -typedef const struct foo { - char inner; -} *foo; - -typedef struct bar { - char inner; -} *bar; - -void takes_foo_ptr(foo); -void takes_foo_struct(struct foo); -void takes_bar_ptr(bar); -void takes_bar_struct(struct bar); diff --git a/bindgen-tests/tests/headers/typedef-pointer-overlap.h b/bindgen-tests/tests/headers/typedef-pointer-overlap.h new file mode 100644 index 0000000000..8c556c9b38 --- /dev/null +++ b/bindgen-tests/tests/headers/typedef-pointer-overlap.h @@ -0,0 +1,30 @@ +typedef const struct foo { + char inner; +} *foo; + +typedef struct bar { + char inner; +} *bar; + +typedef struct baz *baz; + +typedef union cat { + int standard_issue; +} *cat; + +typedef enum mad { scientist } *mad; + +void takes_foo_ptr(foo); +void takes_foo_struct(struct foo); + +void takes_bar_ptr(bar); +void takes_bar_struct(struct bar); + +void takes_baz_ptr(baz); +void takes_baz_struct(struct baz); + +void takes_cat_ptr(cat); +void takes_cat_union(union cat); + +void takes_mad_ptr(mad); +void takes_mad_enum(enum mad); diff --git a/bindgen/ir/ty.rs b/bindgen/ir/ty.rs index 726c743890..a5ce34c928 100644 --- a/bindgen/ir/ty.rs +++ b/bindgen/ir/ty.rs @@ -1104,29 +1104,21 @@ impl Type { // within the clang parsing. TypeKind::Opaque } else { - // Check if this type definition is an alias to a pointer of a `const - // struct` with the same name and add the `_ptr` suffix to it to avoid name - // collisions. - if !ctx.options().c_naming { - if let Some(pointee_spelling) = - inner.pointee_type().map(|ty| ty.spelling()) + // Check if this type definition is an alias to a pointer of a `struct` / + // `union` / `enum` with the same name and add the `_ptr` suffix to it to + // avoid name collisions. + if let Some(ref mut name) = name { + if inner.kind() == CXType_Pointer && + !ctx.options().c_naming { - if let (Some(pointee_name), Some(name)) = ( - pointee_spelling - .strip_prefix("const struct ") - .or_else(|| { - pointee_spelling - .strip_prefix("struct ") - }), - name.as_mut(), - ) { - if pointee_name == name { - *name += "_ptr"; - } + let pointee = inner.pointee_type().unwrap(); + if pointee.kind() == CXType_Elaborated && + pointee.declaration().spelling() == *name + { + *name += "_ptr"; } } } - TypeKind::Alias(inner_id) } }