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

Deduplicate the code that turns transparent structs into typedefs #991

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/bindgen/ir/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down
17 changes: 15 additions & 2 deletions src/bindgen/ir/typedef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
18 changes: 0 additions & 18 deletions src/bindgen/language_backend/clike.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
18 changes: 0 additions & 18 deletions src/bindgen/language_backend/cython.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
20 changes: 19 additions & 1 deletion src/bindgen/language_backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand Down
7 changes: 6 additions & 1 deletion tests/expectations/transparent.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -32,4 +36,5 @@ void root(TransparentComplexWrappingStructTuple a,
TransparentComplexWrapper_i32 e,
TransparentPrimitiveWrapper_i32 f,
TransparentPrimitiveWithAssociatedConstants g,
EnumWithAssociatedConstantInImpl h);
TransparentEmptyStructure h,
EnumWithAssociatedConstantInImpl i);
7 changes: 6 additions & 1 deletion tests/expectations/transparent.compat.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down
7 changes: 6 additions & 1 deletion tests/expectations/transparent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -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"
6 changes: 5 additions & 1 deletion tests/expectations/transparent.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -37,4 +40,5 @@ cdef extern from *:
TransparentComplexWrapper_i32 e,
TransparentPrimitiveWrapper_i32 f,
TransparentPrimitiveWithAssociatedConstants g,
EnumWithAssociatedConstantInImpl h);
TransparentEmptyStructure h,
EnumWithAssociatedConstantInImpl i);
7 changes: 6 additions & 1 deletion tests/expectations/transparent_both.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
7 changes: 6 additions & 1 deletion tests/expectations/transparent_both.compat.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down
7 changes: 6 additions & 1 deletion tests/expectations/transparent_tag.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
7 changes: 6 additions & 1 deletion tests/expectations/transparent_tag.compat.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ typedef uint32_t TransparentPrimitiveWithAssociatedConstants;
#define TransparentPrimitiveWithAssociatedConstants_ZERO 0
#define TransparentPrimitiveWithAssociatedConstants_ONE 1

struct TransparentEmptyStructure {

};

#define EnumWithAssociatedConstantInImpl_TEN 10

#ifdef __cplusplus
Expand All @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion tests/expectations/transparent_tag.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -37,4 +40,5 @@ cdef extern from *:
TransparentComplexWrapper_i32 e,
TransparentPrimitiveWrapper_i32 f,
TransparentPrimitiveWithAssociatedConstants g,
EnumWithAssociatedConstantInImpl h);
TransparentEmptyStructure h,
EnumWithAssociatedConstantInImpl i);
8 changes: 7 additions & 1 deletion tests/rust/transparent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -63,5 +68,6 @@ pub extern "C" fn root(
e: TransparentComplexWrapper<i32>,
f: TransparentPrimitiveWrapper<i32>,
g: TransparentPrimitiveWithAssociatedConstants,
h: EnumWithAssociatedConstantInImpl,
h: TransparentEmptyStructure,
i: EnumWithAssociatedConstantInImpl,
) { }
Loading