From ba96d907f817537018b7bf1b51f9e5b5f0741113 Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Fri, 17 May 2024 09:13:49 -0700 Subject: [PATCH 1/6] Deduplicate the code that turns transparent structs into typedefs --- src/bindgen/ir/structure.rs | 19 ++++++++++++++++++- src/bindgen/ir/typedef.rs | 17 +++++++++++++++-- src/bindgen/language_backend/clike.rs | 18 ------------------ src/bindgen/language_backend/cython.rs | 18 ------------------ src/bindgen/language_backend/mod.rs | 20 +++++++++++++++++++- 5 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index 49ae5da85..c2eb95670 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -11,7 +11,7 @@ use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver; use crate::bindgen::dependencies::Dependencies; use crate::bindgen::ir::{ AnnotationSet, Cfg, Constant, Documentation, Field, GenericArgument, GenericParams, Item, - ItemContainer, Path, Repr, ReprAlign, ReprStyle, Type, + ItemContainer, Path, Repr, ReprAlign, ReprStyle, Type, Typedef, }; use crate::bindgen::library::Library; use crate::bindgen::mangle; @@ -150,6 +150,23 @@ impl Struct { } } + /// Attempts to convert this struct to a typedef (only works for transparent structs). + pub fn as_typedef(&self) -> Option { + if self.is_transparent { + // NOTE: A `#[repr(transparent)]` struct with 2+ NZT fields fails to compile, but 0 + // fields is allowed for some strange reason. Don't emit the typedef in that case. + if let Some(field) = self.fields.first() { + return Some(Typedef::new_from_struct_field(self, field)); + } else { + error!( + "Cannot convert empty transparent struct {} to typedef", + self.name() + ); + } + } + None + } + pub fn add_monomorphs(&self, library: &Library, out: &mut Monomorphs) { // Generic structs can instantiate monomorphs only once they've been // instantiated. See `instantiate_monomorph` for more details. diff --git a/src/bindgen/ir/typedef.rs b/src/bindgen/ir/typedef.rs index 58095878d..e775a4e80 100644 --- a/src/bindgen/ir/typedef.rs +++ b/src/bindgen/ir/typedef.rs @@ -10,8 +10,8 @@ use crate::bindgen::config::Config; use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver; use crate::bindgen::dependencies::Dependencies; use crate::bindgen::ir::{ - AnnotationSet, Cfg, Documentation, GenericArgument, GenericParams, Item, ItemContainer, Path, - Type, + AnnotationSet, Cfg, Documentation, Field, GenericArgument, GenericParams, Item, ItemContainer, + Path, Struct, Type, }; use crate::bindgen::library::Library; use crate::bindgen::mangle; @@ -70,6 +70,19 @@ impl Typedef { self.aliased.simplify_standard_types(config); } + // Used to convert a transparent Struct to a Typedef. + pub fn new_from_struct_field(item: &Struct, field: &Field) -> Self { + Self { + path: item.path().clone(), + export_name: item.export_name().to_string(), + generic_params: item.generic_params.clone(), + aliased: field.ty.clone(), + cfg: item.cfg().cloned(), + annotations: item.annotations().clone(), + documentation: item.documentation().clone(), + } + } + pub fn transfer_annotations(&mut self, out: &mut HashMap) { if self.annotations.is_empty() { return; diff --git a/src/bindgen/language_backend/clike.rs b/src/bindgen/language_backend/clike.rs index 5da079c3d..bf5a39fb9 100644 --- a/src/bindgen/language_backend/clike.rs +++ b/src/bindgen/language_backend/clike.rs @@ -540,24 +540,6 @@ impl LanguageBackend for CLikeLanguageBackend<'_> { } fn write_struct(&mut self, out: &mut SourceWriter, s: &Struct) { - if s.is_transparent { - let typedef = Typedef { - path: s.path.clone(), - export_name: s.export_name.to_owned(), - generic_params: s.generic_params.clone(), - aliased: s.fields[0].ty.clone(), - cfg: s.cfg.clone(), - annotations: s.annotations.clone(), - documentation: s.documentation.clone(), - }; - self.write_type_def(out, &typedef); - for constant in &s.associated_constants { - out.new_line(); - constant.write(self.config, self, out, Some(s)); - } - return; - } - let condition = s.cfg.to_condition(self.config); condition.write_before(self.config, out); diff --git a/src/bindgen/language_backend/cython.rs b/src/bindgen/language_backend/cython.rs index 5c85b2d02..8497f3cd0 100644 --- a/src/bindgen/language_backend/cython.rs +++ b/src/bindgen/language_backend/cython.rs @@ -159,24 +159,6 @@ impl LanguageBackend for CythonLanguageBackend<'_> { } fn write_struct(&mut self, out: &mut SourceWriter, s: &Struct) { - if s.is_transparent { - let typedef = Typedef { - path: s.path.clone(), - export_name: s.export_name.to_owned(), - generic_params: s.generic_params.clone(), - aliased: s.fields[0].ty.clone(), - cfg: s.cfg.clone(), - annotations: s.annotations.clone(), - documentation: s.documentation.clone(), - }; - self.write_type_def(out, &typedef); - for constant in &s.associated_constants { - out.new_line(); - constant.write(self.config, self, out, Some(s)); - } - return; - } - let condition = s.cfg.to_condition(self.config); condition.write_before(self.config, out); diff --git a/src/bindgen/language_backend/mod.rs b/src/bindgen/language_backend/mod.rs index adb7d8507..065bade20 100644 --- a/src/bindgen/language_backend/mod.rs +++ b/src/bindgen/language_backend/mod.rs @@ -139,6 +139,24 @@ pub trait LanguageBackend: Sized { } } + /// If the struct is transparent, emit a typedef of its NZST field type instead. + fn write_struct_or_typedef( + &mut self, + out: &mut SourceWriter, + s: &Struct, + b: &Bindings, + ) { + if let Some(typedef) = s.as_typedef() { + self.write_type_def(out, &typedef); + for constant in &s.associated_constants { + out.new_line(); + constant.write(&b.config, self, out, Some(s)); + } + } else { + self.write_struct(out, s); + } + } + fn write_items(&mut self, out: &mut SourceWriter, b: &Bindings) { for item in &b.items { if item @@ -155,7 +173,7 @@ pub trait LanguageBackend: Sized { ItemContainer::Constant(..) => unreachable!(), ItemContainer::Static(..) => unreachable!(), ItemContainer::Enum(ref x) => self.write_enum(out, x), - ItemContainer::Struct(ref x) => self.write_struct(out, x), + ItemContainer::Struct(ref x) => self.write_struct_or_typedef(out, x, b), ItemContainer::Union(ref x) => self.write_union(out, x), ItemContainer::OpaqueItem(ref x) => self.write_opaque_item(out, x), ItemContainer::Typedef(ref x) => self.write_type_def(out, x), From f6d956dc9294bc351c1d1ccc6b2994fb91221e9f Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Mon, 12 Aug 2024 15:28:33 -0700 Subject: [PATCH 2/6] add missing test case for empty transparent struct --- src/bindgen/ir/structure.rs | 16 ++++++++-------- tests/expectations/transparent.c | 7 ++++++- tests/expectations/transparent.compat.c | 7 ++++++- tests/expectations/transparent.cpp | 7 ++++++- tests/expectations/transparent.pyx | 6 +++++- tests/expectations/transparent_both.c | 7 ++++++- tests/expectations/transparent_both.compat.c | 7 ++++++- tests/expectations/transparent_tag.c | 7 ++++++- tests/expectations/transparent_tag.compat.c | 7 ++++++- tests/expectations/transparent_tag.pyx | 6 +++++- tests/rust/transparent.rs | 8 +++++++- 11 files changed, 67 insertions(+), 18 deletions(-) diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index c2eb95670..1082ea37b 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -153,16 +153,16 @@ impl Struct { /// Attempts to convert this struct to a typedef (only works for transparent structs). pub fn as_typedef(&self) -> Option { if self.is_transparent { - // NOTE: A `#[repr(transparent)]` struct with 2+ NZT fields fails to compile, but 0 - // fields is allowed for some strange reason. Don't emit the typedef in that case. + // NOTE: The rust compiler rejects a `#[repr(transparent)]` struct with 2+ NZST fields, + // but accepts an empty struct (see https://github.com/rust-lang/rust/issues/129029). + // Don't emit the typedef in that case. if let Some(field) = self.fields.first() { return Some(Typedef::new_from_struct_field(self, field)); - } else { - error!( - "Cannot convert empty transparent struct {} to typedef", - self.name() - ); } + error!( + "Cannot convert empty transparent struct {} to typedef", + self.name() + ); } None } @@ -288,7 +288,7 @@ impl Item for Struct { } fn collect_declaration_types(&self, resolver: &mut DeclarationTypeResolver) { - if self.is_transparent { + if self.is_transparent && self.fields.len() == 1 { resolver.add_none(&self.path); } else { resolver.add_struct(&self.path); diff --git a/tests/expectations/transparent.c b/tests/expectations/transparent.c index 4743b506f..3354bc294 100644 --- a/tests/expectations/transparent.c +++ b/tests/expectations/transparent.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 void root(TransparentComplexWrappingStructTuple a, @@ -32,4 +36,5 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent.compat.c b/tests/expectations/transparent.compat.c index c271f8167..1ebe9a037 100644 --- a/tests/expectations/transparent.compat.c +++ b/tests/expectations/transparent.compat.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 #ifdef __cplusplus @@ -36,7 +40,8 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/transparent.cpp b/tests/expectations/transparent.cpp index 22b5f17b9..aa6091ca6 100644 --- a/tests/expectations/transparent.cpp +++ b/tests/expectations/transparent.cpp @@ -26,6 +26,10 @@ using TransparentPrimitiveWithAssociatedConstants = uint32_t; constexpr static const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ZERO = 0; constexpr static const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ONE = 1; +struct TransparentEmptyStructure { + +}; + constexpr static const TransparentPrimitiveWrappingStructure EnumWithAssociatedConstantInImpl_TEN = 10; extern "C" { @@ -37,6 +41,7 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper e, TransparentPrimitiveWrapper f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); } // extern "C" diff --git a/tests/expectations/transparent.pyx b/tests/expectations/transparent.pyx index ac28206f0..839c77427 100644 --- a/tests/expectations/transparent.pyx +++ b/tests/expectations/transparent.pyx @@ -28,6 +28,9 @@ cdef extern from *: const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ZERO # = 0 const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ONE # = 1 + ctypedef struct TransparentEmptyStructure: + pass + const TransparentPrimitiveWrappingStructure EnumWithAssociatedConstantInImpl_TEN # = 10 void root(TransparentComplexWrappingStructTuple a, @@ -37,4 +40,5 @@ cdef extern from *: TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent_both.c b/tests/expectations/transparent_both.c index 6b3c576b0..df9b8a57d 100644 --- a/tests/expectations/transparent_both.c +++ b/tests/expectations/transparent_both.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct TransparentEmptyStructure { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 void root(TransparentComplexWrappingStructTuple a, @@ -32,4 +36,5 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent_both.compat.c b/tests/expectations/transparent_both.compat.c index 8d56046fd..4acd6677d 100644 --- a/tests/expectations/transparent_both.compat.c +++ b/tests/expectations/transparent_both.compat.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +typedef struct TransparentEmptyStructure { + +} TransparentEmptyStructure; + #define EnumWithAssociatedConstantInImpl_TEN 10 #ifdef __cplusplus @@ -36,7 +40,8 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/transparent_tag.c b/tests/expectations/transparent_tag.c index c76d2a368..2f6dea68e 100644 --- a/tests/expectations/transparent_tag.c +++ b/tests/expectations/transparent_tag.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +struct TransparentEmptyStructure { + +}; + #define EnumWithAssociatedConstantInImpl_TEN 10 void root(TransparentComplexWrappingStructTuple a, @@ -32,4 +36,5 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); diff --git a/tests/expectations/transparent_tag.compat.c b/tests/expectations/transparent_tag.compat.c index 8329ca8a0..508d6ad67 100644 --- a/tests/expectations/transparent_tag.compat.c +++ b/tests/expectations/transparent_tag.compat.c @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants; #define TransparentPrimitiveWithAssociatedConstants_ZERO 0 #define TransparentPrimitiveWithAssociatedConstants_ONE 1 +struct TransparentEmptyStructure { + +}; + #define EnumWithAssociatedConstantInImpl_TEN 10 #ifdef __cplusplus @@ -36,7 +40,8 @@ void root(TransparentComplexWrappingStructTuple a, TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - struct EnumWithAssociatedConstantInImpl h); + struct TransparentEmptyStructure h, + struct EnumWithAssociatedConstantInImpl i); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/transparent_tag.pyx b/tests/expectations/transparent_tag.pyx index db0f5d93c..15d66f74c 100644 --- a/tests/expectations/transparent_tag.pyx +++ b/tests/expectations/transparent_tag.pyx @@ -28,6 +28,9 @@ cdef extern from *: const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ZERO # = 0 const TransparentPrimitiveWithAssociatedConstants TransparentPrimitiveWithAssociatedConstants_ONE # = 1 + cdef struct TransparentEmptyStructure: + pass + const TransparentPrimitiveWrappingStructure EnumWithAssociatedConstantInImpl_TEN # = 10 void root(TransparentComplexWrappingStructTuple a, @@ -37,4 +40,5 @@ cdef extern from *: TransparentComplexWrapper_i32 e, TransparentPrimitiveWrapper_i32 f, TransparentPrimitiveWithAssociatedConstants g, - EnumWithAssociatedConstantInImpl h); + TransparentEmptyStructure h, + EnumWithAssociatedConstantInImpl i); diff --git a/tests/rust/transparent.rs b/tests/rust/transparent.rs index 6dc77c1e4..1af6e3f07 100644 --- a/tests/rust/transparent.rs +++ b/tests/rust/transparent.rs @@ -41,6 +41,11 @@ impl TransparentPrimitiveWithAssociatedConstants { #[repr(transparent)] struct TransparentPrimitiveWithAssociatedConstants { bits: u32 } +// https://github.com/rust-lang/rust/issues/129029 the rust compiler (wrongly?) accepts this, but +// cbindgen should nevertheless emit the struct definition instead of a typedef. +#[repr(transparent)] +struct TransparentEmptyStructure; + // Associated constant declared after struct declaration. impl TransparentPrimitiveWithAssociatedConstants { pub const ONE: TransparentPrimitiveWithAssociatedConstants = TransparentPrimitiveWithAssociatedConstants { @@ -63,5 +68,6 @@ pub extern "C" fn root( e: TransparentComplexWrapper, f: TransparentPrimitiveWrapper, g: TransparentPrimitiveWithAssociatedConstants, - h: EnumWithAssociatedConstantInImpl, + h: TransparentEmptyStructure, + i: EnumWithAssociatedConstantInImpl, ) { } From 0f4f3c3b06e2932a9cbe22a927a3524f6bdf933e Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Mon, 12 Aug 2024 16:41:11 -0700 Subject: [PATCH 3/6] handle empty transparent struct in a better way --- src/bindgen/ir/structure.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index 1082ea37b..30c509894 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -122,11 +122,18 @@ impl Struct { has_tag_field: bool, is_enum_variant_body: bool, alignment: Option, - is_transparent: bool, + mut is_transparent: bool, cfg: Option, annotations: AnnotationSet, documentation: Documentation, ) -> Self { + // WARNING: The rust compiler rejects a `#[repr(transparent)]` struct with 2+ NZST fields, but + // accepts an empty struct (see https://github.com/rust-lang/rust/issues/129029). We must + // not emit a typedef in that case, because there is no underlying type. + if is_transparent && fields.len() != 1 { + error!("Illegal empty transparent struct {}", &path); + is_transparent = false; + } let export_name = path.name().to_owned(); Self { path, @@ -152,19 +159,10 @@ impl Struct { /// Attempts to convert this struct to a typedef (only works for transparent structs). pub fn as_typedef(&self) -> Option { - if self.is_transparent { - // NOTE: The rust compiler rejects a `#[repr(transparent)]` struct with 2+ NZST fields, - // but accepts an empty struct (see https://github.com/rust-lang/rust/issues/129029). - // Don't emit the typedef in that case. - if let Some(field) = self.fields.first() { - return Some(Typedef::new_from_struct_field(self, field)); - } - error!( - "Cannot convert empty transparent struct {} to typedef", - self.name() - ); + match self.fields.first() { + Some(field) if self.is_transparent => Some(Typedef::new_from_struct_field(self, field)), + _ => None, } - None } pub fn add_monomorphs(&self, library: &Library, out: &mut Monomorphs) { @@ -288,7 +286,7 @@ impl Item for Struct { } fn collect_declaration_types(&self, resolver: &mut DeclarationTypeResolver) { - if self.is_transparent && self.fields.len() == 1 { + if self.is_transparent { resolver.add_none(&self.path); } else { resolver.add_struct(&self.path); From 4935dd019f774fb0082e6787bfb2974e02feed8f Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Mon, 12 Aug 2024 16:48:15 -0700 Subject: [PATCH 4/6] improve code comments about transparent empty structs --- src/bindgen/ir/structure.rs | 6 +++--- tests/rust/transparent.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index 30c509894..2f5941b7a 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -127,9 +127,9 @@ impl Struct { annotations: AnnotationSet, documentation: Documentation, ) -> Self { - // WARNING: The rust compiler rejects a `#[repr(transparent)]` struct with 2+ NZST fields, but - // accepts an empty struct (see https://github.com/rust-lang/rust/issues/129029). We must - // not emit a typedef in that case, because there is no underlying type. + // https://github.com/rust-lang/rust/issues/129029: The rust compiler accepts an empty + // `#[repr(transparent)]` struct. We must not emit a typedef in that case, because there is + // no underlying type it could refer to. if is_transparent && fields.len() != 1 { error!("Illegal empty transparent struct {}", &path); is_transparent = false; diff --git a/tests/rust/transparent.rs b/tests/rust/transparent.rs index 1af6e3f07..4d37bda92 100644 --- a/tests/rust/transparent.rs +++ b/tests/rust/transparent.rs @@ -41,8 +41,8 @@ impl TransparentPrimitiveWithAssociatedConstants { #[repr(transparent)] struct TransparentPrimitiveWithAssociatedConstants { bits: u32 } -// https://github.com/rust-lang/rust/issues/129029 the rust compiler (wrongly?) accepts this, but -// cbindgen should nevertheless emit the struct definition instead of a typedef. +// https://github.com/rust-lang/rust/issues/129029: The rust compiler (wrongly?) accepts a +// transparent empty struct, but cbindgen should NOT emit a typedef in that case. #[repr(transparent)] struct TransparentEmptyStructure; From 1659532909ea837d5c2785daa0fd389d0051fc91 Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Tue, 13 Aug 2024 09:56:15 -0700 Subject: [PATCH 5/6] revise ZST handling again --- src/bindgen/ir/structure.rs | 20 +++++++++++++++----- tests/rust/transparent.rs | 4 ++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index 2f5941b7a..2fa5b4686 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -127,13 +127,23 @@ impl Struct { annotations: AnnotationSet, documentation: Documentation, ) -> Self { - // https://github.com/rust-lang/rust/issues/129029: The rust compiler accepts an empty - // `#[repr(transparent)]` struct. We must not emit a typedef in that case, because there is - // no underlying type it could refer to. - if is_transparent && fields.len() != 1 { - error!("Illegal empty transparent struct {}", &path); + // WARNING: Zero-sized transparent structs are legal rust [1], but zero-sized types of any + // repr are "best avoided entirely" [2] because they "will be nonsensical or problematic if + // passed through the FFI boundary" [3]. Further, because there no well-defined underlying + // native type exists for a ZST, we cannot emit a typedef for it and must treat it as an + // empty repr(C) struct instead. + // + // [1] https://github.com/rust-lang/rust/issues/77841#issuecomment-716575747 + // [2] https://github.com/rust-lang/rust/issues/77841#issuecomment-716796313 + // [3] https://doc.rust-lang.org/nomicon/other-reprs.html + if fields.is_empty() { + warn!( + "Passing zero-sized struct {} across the FFI boundary is undefined behavior", + &path + ); is_transparent = false; } + let export_name = path.name().to_owned(); Self { path, diff --git a/tests/rust/transparent.rs b/tests/rust/transparent.rs index 4d37bda92..e35200cf1 100644 --- a/tests/rust/transparent.rs +++ b/tests/rust/transparent.rs @@ -41,8 +41,8 @@ impl TransparentPrimitiveWithAssociatedConstants { #[repr(transparent)] struct TransparentPrimitiveWithAssociatedConstants { bits: u32 } -// https://github.com/rust-lang/rust/issues/129029: The rust compiler (wrongly?) accepts a -// transparent empty struct, but cbindgen should NOT emit a typedef in that case. +// Transparent zero-sized structs are legal rust, but there's no way to emit a typedef for one, so +// cbindgen should treat it as repr(C) instead and emit an empty struct definition. #[repr(transparent)] struct TransparentEmptyStructure; From bb92f0071052f27b80cb6e779f6be70d8facca86 Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Tue, 13 Aug 2024 10:01:59 -0700 Subject: [PATCH 6/6] fix typo --- src/bindgen/ir/structure.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index 2fa5b4686..9b33a15c7 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -129,9 +129,8 @@ impl Struct { ) -> Self { // WARNING: Zero-sized transparent structs are legal rust [1], but zero-sized types of any // repr are "best avoided entirely" [2] because they "will be nonsensical or problematic if - // passed through the FFI boundary" [3]. Further, because there no well-defined underlying - // native type exists for a ZST, we cannot emit a typedef for it and must treat it as an - // empty repr(C) struct instead. + // passed through the FFI boundary" [3]. Further, because no well-defined underlying native + // type exists for a ZST, we cannot emit a typedef and must define an empty struct instead. // // [1] https://github.com/rust-lang/rust/issues/77841#issuecomment-716575747 // [2] https://github.com/rust-lang/rust/issues/77841#issuecomment-716796313