bitflags: bandaid fix for repr(transparent) bitflags

[nullstalgia]

While working on #1169, I noticed that my bitflags were outputting invalid code/#defines when using repr(transparent) and creating a new flag/mask from other bitflags.

Namely, it looked like:

typedef uint8_t Flags;
#define Flags_A (uint8_t)(1 << 0)
#define Flags_B (uint8_t)(1 << 1)
#define Flags_C (uint8_t)(1 << 2)
#define Flags_D (uint8_t)(1 << 3)
#define Flags_ALL (uint8_t)((((Flags_A).bits | (Flags_B).bits) | (Flags_C).bits) | (Flags_D).bits) // <-- uint8_t has no fields

or even this (when using the "out-of-line" impl kind of bitflags! invocation):

#define Flags_ALL (uint8_t)((((Flags_A)._0 | (Flags_B)._0) | (Flags_C)._0) | (Flags_D)._0)

I recognize (via #1096) that this isn't unique to bitflags macro expansions, but I wanted to see how easily I could fix that so I could move on with my project without changing even more of my code, just to have cbindgen understand it.

(The lack of something like #1029 being merged makes it so any internal pub type/newtype within my Rust code gets turned into a typedef in the C header, which is annoying when I want to make a "hidden" Untrusted<T> pointer wrapper type but I need to instead name it something like NonNull so that it can get folded into a plain T* pointer in the header, without needing to rely on something like macro expansion.....)

This uses the same clippy-appeasing commit from #1169, just cherry-picked. Let me know if I should drop it to allow for a cleaner git history.

[emilio]

Could you rebase once your other patch is in main to remove some noise from the clippy changes (thanks for fixing those!)? In general, this feels a bit unfortunate, but I guess it's a fairly simple special-case for bitflags, at least...

The lack of something like #1029 being merged makes it so any internal pub type/newtype within my Rust code gets turned into a typedef in the C header

If you don't want that you can /// cbindgen:ignore it, right?

[nullstalgia]

Could you rebase once your other patch is in..?

Will do!

I guess it's a fairly simple special-case for bitflags, at least...

I echo the sentiment... Hopefully someone with more time can swoop in and make some deeper changes to make workarounds like this unnecessary.

If you don't want that you can /// cbindgen:ignore it, right?

Nope! That just makes it worse, actually (example at the bottom)!

Let's say I have either of the following:

pub type Untrusted<T> = Option<NonNull<T>>;

// or

#[repr(transparent)]
#[derive(Debug, Clone, Copy)]
pub struct Untrusted<T>(Option<NonNull<T>>);

My intention is two-fold:

1. Allow for more natural function signatures in both Rust and C.

• My normal instinct is to use Option<NonNull<T>> over *mut T, so that I'm forced to handle the None case.
• But if I'm accepting a Pointer to a Pointer, then my function signature might look like this (pretty verbose):

#[unsafe(no_mangle)]
unsafe extern "C" fn find_available_devices(
    dest: Option<NonNull<Option<NonNull<Vec<DeviceInfo>>>>>,
) -> libc::c_int {

• But my C header will look like this (perfectly acceptable!):

int find_available_devices(struct Vec_DeviceInfo **dest);

---

So I'd like to be able to do something like this instead in Rust:

#[unsafe(no_mangle)]
unsafe extern "C" fn find_available_devices(
    dest: Untrusted<Untrusted<Vec<DeviceInfo>>>
) -> libc::c_int {

But the issue is that (for both types and structs), this results in the following C header:

int find_available_devices(Untrusted_Untrusted_Vec_DeviceInfo dest);

I end up having internal implementation details leaked into the API, making the signature more complex/obscure than it really is to the end-user.

(This also happens with just one layer of Untrusted<T>, btw. I use two layers to show off my more common circumstance/complaint.)

---

2. Allowing me to put more checks on my pointer wrapper type or impl something like .flatten() for Untrusted<Untrusted<T>>.

• I think it's silly how Rust has the NonZero/NonNull types but nothing like how Zig does for specifying pointer alignment (outside of the target type with ptr::is_aligned()).
• While one has to make quite a mistake to pass a mis-aligned pointer into a library/function, I'd rather catch as many mishaps as soon as possible before simply trying to dereference garbage. (Who knows what hardware glitch could happen?)

---

"What using does /// cbindgen:ignore look like?"

Straight-up invalid code!

int get_device_list(Untrusted<Untrusted<Vec<DeviceInfo>>> dest);

WARN: Cannot find a mangling for generic path GenericPath { path: Path { name: "Untrusted" }, export_name: "Untrusted", generics: [Type(Path(GenericPath { path: Path { name: "Untrusted" }, export_name: "Untrusted", generics: [Type(Path(GenericPath { path: Path { name: "[redacted]" }, export_name: "[redacted]", generics: [], ctype: None }))], ctype: None }))], ctype: None }. This usually means that a type referenced by this generic was incompatible or not found.

This is why I've been relying on either (s l o w) macro expansion (to replace a Untrusted![] invocation with Option<NonNull<>>), or naming my types the same as what gets simplified explicitly, or waiting for #1029 to let me explicitly mark my type/struct as transparent to cbindgen's IR.

(I spent a good couple days this week wrestling with all this + the bitflags stuff. I'd almost want to try cheadergen, but it's still maturing and I'm unsure if anything like the bitflags expansion is actually possible with their methodology.)

[emilio]

I see, thanks. So let me see if I got your use case right. Given this rust file:

use std::ptr::NonNull;

struct DeviceInfo {
    foo: u32,
}

pub type Untrusted<T> = Option<NonNull<T>>;

#[unsafe(no_mangle)]
unsafe extern "C" fn find_available_devices(_dest: Untrusted<Untrusted<Vec<DeviceInfo>>>) -> i32 {
    todo!()
}

Right now cbindgen for C++ generates:

// headers omitted for brevity

struct DeviceInfo;

template<typename T = void>
struct Vec;

template<typename T>
using Untrusted = T*;

extern "C" {

int32_t find_available_devices(Untrusted<Untrusted<Vec<DeviceInfo>>> _dest);

}

Which is pretty fair. For C we generate:

// headers omitted for brevity.

typedef struct Vec_DeviceInfo Vec_DeviceInfo;

typedef struct Vec_DeviceInfo *Untrusted_Vec_DeviceInfo;

typedef Untrusted_Vec_DeviceInfo *Untrusted_Untrusted_Vec_DeviceInfo;

int32_t find_available_devices(Untrusted_Untrusted_Vec_DeviceInfo _dest);

Which is actually pretty fair as well, right?

As in, it's correct, just so happens that you care about the conciseness of the generated header, and I agree that struct Vec_DeviceInfo **dest would be nice to generate.

Re. #1029, I didn't get to review that and it fell down the queue, sorry, I can try to take a look. At a glance doing a whole other global pass over all types seems unfortunate. Might be better to try to do this during monomorphisation if this mostly affects generics / C?

Unfortunately I don't have that much time to dedicate to cbindgen outside of work, and for Firefox we mostly care about the C++ output...

[nullstalgia]

As in, it's correct, just so happens that you care about the conciseness of the generated header...

Mhm, you got it! I'm trying to have the headers as clean as possible for when I hand them off to those who need my library as a linkable object, which includes folks who know C well but not a lick of Rust. The more familiar/easily digestible it is, the better and faster we can get our work done without staring at autogenerated visual noise.

Might be better to try to do this during monomorphisation if this mostly affects generics / C? ... for Firefox we mostly care about the C++ output...

My C++ knowledge is near-zero, so I can't speak super confidently on how harshly this affects C++ comparatively, but given your own example and the one in #1029, having something like cbindgen:transparent-typedef should positively affect both C and C++ in terms of minimizing unnecessary typedefs/templates.

---

...it fell down the queue, sorry, I can try to take a look.

It happens, c'est la vie~

Thanks for at least entertaining me with my concerns and ideas today though. ❤️

Let me know if this PR (or my previous) need any further attention or explanation, should be good to go otherwise now I think (since it's mostly unrelated to the tangent at hand). :)