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

Add support for transparent typedefs #966

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
103 changes: 85 additions & 18 deletions docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,76 @@ fn bar() -> Foo { .. } // Will be emitted as `struct foo bar();`

### Struct Annotations

* field-names=\[field1, field2, ...\] -- sets the names of all the fields in the output struct. These names will be output verbatim, and are not eligible for renaming.
* field-names=\[field1, field2, ...\] -- sets the names of all the fields in the output
struct. These names will be output verbatim, and are not eligible for renaming.

* transparent-typedef -- when emitting the typedef for a transparent struct, mark it as
transparent. All references to the struct will be replaced with the type of its underlying NZST
field, effectively making the struct invisible on the FFI side. For example, consider the
following Rust code:

```rust
#[repr(transparent)]
pub struct Handle<T> {
ptr: NonNull<T>,
}

pub struct Foo { }

#[no_mangle]
pub extern "C" fn foo_operation(foo: Option<Handle<Foo>>) { }
```

By default, the exported C++ code would fail to compile, because the function takes `Option<...>`
(which is an opaque type) by value:

```cpp
template<typename T>
struct Option<T>;

template<typename T>
using Handle = T;

struct Foo;

void foo_operation(Option<Handle<Foo>> foo);
```

If we annotate `Handle` with `transparent-typedef` (leaving the rest of the code unchanged):
```rust
/// cbindgen:transparent-typedef
#[repr(transparent)]
pub struct Handle<T> {
ptr: NonNull<T>,
}
```

Then cbindgen is able to simplify the exported C++ code to just:
```cpp
struct Foo;

void foo_operation(Foo* foo);
```

NOTE: This annotation does _NOT_ affect user-defined type aliases for transparent structs. If we
we adjust the previous example to use a type alias:

```rust
type NullableFooHandle = Option<Handle<Foo>>;

#[no_mangle]
pub extern "C" fn foo_operation(foo: NullableFooHandle) { }
```

Then the exported code will use it as expected:

```cpp
struct Foo;

using NullableFooHandle = Foo*;

void foo_operation(NullableFooHandle foo);
```

The rest are just local overrides for the same options found in the cbindgen.toml:

Expand All @@ -316,27 +385,25 @@ The rest are just local overrides for the same options found in the cbindgen.tom
/ etc(if any). The idea is for this to be used to annotate the operator with
attributes, for example:

```rust
/// cbindgen:eq-attributes=MY_ATTRIBUTES
#[repr(C)]
pub struct Foo { .. }
```

Will generate something like:
```rust
/// cbindgen:eq-attributes=MY_ATTRIBUTES
#[repr(C)]
pub struct Foo { .. }
```

```
MY_ATTRIBUTES bool operator==(const Foo& other) const {
...
}
```
Will generate something like:

Combined with something like:
```
MY_ATTRIBUTES bool operator==(const Foo& other) const {
...
}
```

```
#define MY_ATTRIBUTES [[nodiscard]]
```
Combined with something like:

for example.
```
#define MY_ATTRIBUTES [[nodiscard]]
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changes seem unrelated? Seems ok, but maybe worth splitting into its own commit / PR? Or is it some sort of autoformatting i'm not aware of?

Copy link
Contributor Author

@scovich scovich Aug 12, 2024

Choose a reason for hiding this comment

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

I was just fixing broken indentation in the existing .md, so that bullets would render correctly. If you hide whitespace changes the diff disappears:
image

Happy to split it out if it's too noisy/annoying, tho.


### Enum Annotations

Expand Down
2 changes: 1 addition & 1 deletion src/bindgen/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ impl Builder {
let mut result = Parse::new();

if self.std_types {
result.add_std_types();
result.add_std_types(self.config.language);
}

for x in &self.srcs {
Expand Down
16 changes: 14 additions & 2 deletions src/bindgen/ir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use crate::bindgen::config::{Config, Language};
use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver;
use crate::bindgen::dependencies::Dependencies;
use crate::bindgen::ir::{
AnnotationSet, Cfg, ConditionWrite, Documentation, GenericParams, Item, ItemContainer, Path,
Struct, ToCondition, Type,
AnnotationSet, Cfg, ConditionWrite, Documentation, GenericArgument, GenericParams, Item,
ItemContainer, Path, Struct, ToCondition, TransparentTypeEraser, Type,
};
use crate::bindgen::language_backend::LanguageBackend;
use crate::bindgen::library::Library;
Expand Down Expand Up @@ -603,6 +603,18 @@ impl Item for Constant {
fn generic_params(&self) -> &GenericParams {
GenericParams::empty()
}

fn erase_transparent_types_inplace(
&mut self,
library: &Library,
eraser: &mut TransparentTypeEraser,
_generics: &[GenericArgument],
) {
// NOTE: We also need to simplify the literal initializer value to match the underlying
// type, but that is true for all transparent structs (not just transparent-typedef
// structs), and is handled by the `write` method below.
eraser.erase_transparent_types_inplace(library, &mut self.ty, &[]);
}
}

impl Constant {
Expand Down
38 changes: 25 additions & 13 deletions src/bindgen/ir/enumeration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::bindgen::dependencies::Dependencies;
use crate::bindgen::ir::{
AnnotationSet, AnnotationValue, Cfg, ConditionWrite, DeprecatedNoteKind, Documentation, Field,
GenericArgument, GenericParams, GenericPath, Item, ItemContainer, Literal, Path, Repr,
ReprStyle, Struct, ToCondition, Type,
ReprStyle, Struct, ToCondition, TransparentTypeEraser, Type,
};
use crate::bindgen::language_backend::LanguageBackend;
use crate::bindgen::library::Library;
Expand Down Expand Up @@ -247,12 +247,6 @@ impl EnumVariant {
}
}

fn simplify_standard_types(&mut self, config: &Config) {
if let VariantBody::Body { ref mut body, .. } = self.body {
body.simplify_standard_types(config);
}
}

fn add_dependencies(&self, library: &Library, out: &mut Dependencies) {
if let VariantBody::Body { ref body, .. } = self.body {
body.add_dependencies(library, out);
Expand Down Expand Up @@ -500,6 +494,30 @@ impl Item for Enum {
&self.generic_params
}

fn erase_transparent_types_inplace(
&mut self,
library: &Library,
eraser: &mut TransparentTypeEraser,
generics: &[GenericArgument],
) {
let mut skip_inline_tag_field = Self::inline_tag_field(&self.repr);
let generics = self.generic_params.defaulted_generics(generics);
let mappings = self.generic_params.call(self.name(), &generics);
for variant in self.variants.iter_mut() {
if let VariantBody::Body { ref mut body, .. } = variant.body {
for field in body.fields.iter_mut() {
// Ignore the inline Tag field, if any (it's always first)
if skip_inline_tag_field {
debug!("Skipping inline Tag field {:?}", field);
skip_inline_tag_field = false;
} else {
eraser.erase_transparent_types_inplace(library, &mut field.ty, &mappings);
}
}
}
}
}

fn rename_for_config(&mut self, config: &Config) {
config.export.rename(&mut self.export_name);

Expand Down Expand Up @@ -1493,10 +1511,4 @@ impl Enum {
}
}
}

pub fn simplify_standard_types(&mut self, config: &Config) {
for variant in &mut self.variants {
variant.simplify_standard_types(config);
}
}
}
23 changes: 15 additions & 8 deletions src/bindgen/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use syn::ext::IdentExt;
use crate::bindgen::config::{Config, Language};
use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver;
use crate::bindgen::dependencies::Dependencies;
use crate::bindgen::ir::{AnnotationSet, Cfg, Documentation, GenericPath, Path, Type};
use crate::bindgen::ir::{
AnnotationSet, Cfg, Documentation, GenericPath, Path, TransparentTypeEraser, Type,
};
use crate::bindgen::library::Library;
use crate::bindgen::monomorph::Monomorphs;
use crate::bindgen::rename::{IdentifierType, RenameRule};
Expand Down Expand Up @@ -115,13 +117,6 @@ impl Function {
&self.path
}

pub fn simplify_standard_types(&mut self, config: &Config) {
self.ret.simplify_standard_types(config);
for arg in &mut self.args {
arg.ty.simplify_standard_types(config);
}
}

pub fn add_dependencies(&self, library: &Library, out: &mut Dependencies) {
self.ret.add_dependencies(library, out);
for arg in &self.args {
Expand Down Expand Up @@ -150,6 +145,18 @@ impl Function {
}
}

// NOTE: No `generics` arg because Functions do not support generics and do not `impl Item`.
pub fn erase_transparent_types_inplace(
&mut self,
library: &Library,
eraser: &mut TransparentTypeEraser,
) {
eraser.erase_transparent_types_inplace(library, &mut self.ret, &[]);
for arg in &mut self.args {
eraser.erase_transparent_types_inplace(library, &mut arg.ty, &[]);
}
}

pub fn rename_for_config(&mut self, config: &Config) {
// Rename the types used in arguments
let generic_params = Default::default();
Expand Down
67 changes: 65 additions & 2 deletions src/bindgen/ir/generic_path.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;
use std::collections::HashMap;
use std::io::Write;
use std::ops::Deref;

Expand All @@ -8,16 +10,17 @@ use crate::bindgen::config::{Config, Language};
use crate::bindgen::declarationtyperesolver::{DeclarationType, DeclarationTypeResolver};
use crate::bindgen::ir::{ConstExpr, Path, Type};
use crate::bindgen::language_backend::LanguageBackend;
use crate::bindgen::library::Library;
use crate::bindgen::utilities::IterHelpers;
use crate::bindgen::writer::SourceWriter;

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum GenericParamType {
Type,
Const(Type),
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct GenericParam {
name: Path,
ty: GenericParamType,
Expand Down Expand Up @@ -103,6 +106,24 @@ impl GenericParams {
Ok(GenericParams(params))
}

/// If `generics` is empty, create a set of "default" generic arguments, which preserves the
/// existing parameter name. Useful to allow `call` to work when no generics are provided.
pub fn defaulted_generics<'a>(
&self,
generics: &'a [GenericArgument],
) -> Cow<'a, [GenericArgument]> {
if !self.is_empty() && generics.is_empty() {
Cow::Owned(
self.iter()
.map(|param| Type::Path(GenericPath::new(param.name.clone(), vec![])))
.map(GenericArgument::Type)
.collect(),
)
} else {
Cow::Borrowed(generics)
}
}

/// Associate each parameter with an argument.
pub fn call<'out>(
&'out self,
Expand Down Expand Up @@ -234,6 +255,48 @@ impl GenericArgument {
}
}

/// Helper for erasing transparent types, which memoizes already-seen types to avoid repeated work.
#[derive(Default)]
pub struct TransparentTypeEraser {
// Remember paths we've already visited, so we don't repeat unnecessary work.
// TODO: how to handle recursive types such as `struct Foo { next: Box<Foo> }`?
known_types: HashMap<Type, Option<Type>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really expect extremely long chains of transparent typedefs? Memoizing this seems over-kill without performance numbers, IMHO...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know about long chains, but I do expect frequent use (typedefs exist to eliminate repetition in typing out complex type names). Is frequent use not sufficient reason to memoize?

}

impl TransparentTypeEraser {
pub fn erase_transparent_types_inplace(
&mut self,
library: &Library,
target: &mut Type,
mappings: &[(&Path, &GenericArgument)],
) {
if let Some(erased_type) = self.erase_transparent_types(library, target, mappings) {
*target = erased_type;
}
}

#[must_use]
pub fn erase_transparent_types(
&mut self,
library: &Library,
target: &Type,
mappings: &[(&Path, &GenericArgument)],
) -> Option<Type> {
let known_type = self.known_types.get(target);
let unknown_type = known_type.is_none();
let erased_type = if let Some(ty) = known_type {
ty.clone()
} else {
target.erase_transparent_types(library, mappings, self)
};
if unknown_type {
debug!("Caching erasure of {:?} as {:?}", target, erased_type);
self.known_types.insert(target.clone(), erased_type.clone());
}
erased_type
}
}

#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct GenericPath {
path: Path,
Expand Down
Loading
Loading