Deduplicate the code that turns transparent structs into typedefs

All zero-sized structs now trigger a warning about undefined behavior, and an empty struct definition is emitted regardless of whether the user asked for repr(C) or repr(transparent).
mozilla · Aug 14, 2024 · 3ed9434 · 3ed9434
1 parent 58c6156
commit 3ed9434
Show file tree

Hide file tree

Showing 15 changed files with 119 additions and 51 deletions.
diff --git 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;
@@ -122,11 +122,27 @@ impl Struct {
         has_tag_field: bool,
         is_enum_variant_body: bool,
         alignment: Option<ReprAlign>,
-        is_transparent: bool,
+        mut is_transparent: bool,
         cfg: Option<Cfg>,
         annotations: AnnotationSet,
         documentation: Documentation,
     ) -> 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 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
+        // [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,
@@ -150,6 +166,14 @@ impl Struct {
         }
     }
 
+    /// Attempts to convert this struct to a typedef (only works for transparent structs).
+    pub fn as_typedef(&self) -> Option<Typedef> {
+        match self.fields.first() {
+            Some(field) if self.is_transparent => Some(Typedef::new_from_struct_field(self, field)),
+            _ => 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
@@ -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<Path, AnnotationSet>) {
         if self.annotations.is_empty() {
             return;

diff --git 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<W: Write>(&mut self, out: &mut SourceWriter<W>, 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
@@ -159,24 +159,6 @@ impl LanguageBackend for CythonLanguageBackend<'_> {
     }
 
     fn write_struct<W: Write>(&mut self, out: &mut SourceWriter<W>, 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
@@ -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<W: Write>(
+        &mut self,
+        out: &mut SourceWriter<W>,
+        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<W: Write>(&mut self, out: &mut SourceWriter<W>, 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),

diff --git 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
@@ -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
@@ -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<int32_t> e,
           TransparentPrimitiveWrapper<int32_t> f,
           TransparentPrimitiveWithAssociatedConstants g,
-          EnumWithAssociatedConstantInImpl h);
+          TransparentEmptyStructure h,
+          EnumWithAssociatedConstantInImpl i);
 
 }  // extern "C"
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -41,6 +41,11 @@ impl TransparentPrimitiveWithAssociatedConstants {
 #[repr(transparent)]
 struct TransparentPrimitiveWithAssociatedConstants { bits: u32 }
 
+// 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;
+
 // Associated constant declared after struct declaration.
 impl TransparentPrimitiveWithAssociatedConstants {
     pub const ONE: TransparentPrimitiveWithAssociatedConstants = TransparentPrimitiveWithAssociatedConstants {
@@ -63,5 +68,6 @@ pub extern "C" fn root(
     e: TransparentComplexWrapper<i32>,
     f: TransparentPrimitiveWrapper<i32>,
     g: TransparentPrimitiveWithAssociatedConstants,
-    h: EnumWithAssociatedConstantInImpl,
+    h: TransparentEmptyStructure,
+    i: EnumWithAssociatedConstantInImpl,
 ) { }