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

Conversation

scovich
Copy link
Contributor

@scovich scovich commented May 24, 2024

Following #967, introduce /// cbindgen:transparent-typedef annotation that causes cbindgen to replace transparent structs and typedefs with their underlying type.

This allows the following:

/// cbindgen:transparent-typedef
#[repr(transparent)]
struct Foo(NonNull<i32>);

/// cbindgen:transparent-typedef
type Function = extern "C" fn(i: i32) -> i64;

type NullableFoo = Option<Foo>;
type NullableFunction = Option<Function>;

to export as

typedef i32 *NullableFoo;
typedef int64_t (*NullableFunction)(int32_t);

instead of today's output:

template<typename T = void>
struct Option;

using Foo = int32_t*;
using Function = int64_t(*)(int32_t i);

using NullableFoo = Option<Foo>;
using NullableFunction = Option<Function>;

@scovich scovich changed the title Prototype: transparent typedefs Add support for transparent typedefs Aug 12, 2024
@scovich scovich marked this pull request as ready for review August 12, 2024 17:55
Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Need to take a closer look but over all looks reasonable.

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.

src/bindgen/ir/ty.rs Outdated Show resolved Hide resolved
} => {
// Predict that ret+args will all be erased, but decrement the count whenever we're
// wrong. If the count drops to 0, then type erasure was a no-op after all.
let mut num_erased = 1 + args.len();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it'd be a bit clearer to do it the other way around. In fact, wouldn't it be a bit nicer to just:

let mut erased_any = false;
macro_rules! try_erase {
    ($item:tt) => {
        if let Some(erased) = eraser.erase_transparent_types(library, $item, mappings) {
            erased
        } else {
            $item.clone()
        }
    }

Then:

let ret = try_erase!(ret);
...

if erased_any {
    return Some(..);
}

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.

Ah, nice suggestion. I implemented it with a lambda instead of a macro tho.

@scovich
Copy link
Contributor Author

scovich commented Aug 18, 2024

I just noticed that this PR overlaps significantly with #699, and probably fixes both #690 (NonZero) and #326 (Option)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants