cbindgen icon indicating copy to clipboard operation
cbindgen copied to clipboard

`repr(transparent)` on a tuple struct wrapping an array

Open mversic opened this issue 2 years ago • 7 comments

1: With the latest cbindgen, the following code:

#[repr(C)]
pub struct K([u64; 2]);
#[no_mangle]
pub unsafe extern "C" fn function(arg: K) -> K {
    a
}

correctly produces:

typedef struct K {
  uint64_t _0[2];
} K;
struct K function(struct K arg);

whereas the following code:

#[repr(transparent)]
pub struct K([u64; 2]);
#[no_mangle]
pub unsafe extern "C" fn function(arg: K) -> K {
    a
}

incorrectly produces:

typedef uint64_t K[2];
K function(K arg);

The 2nd example is incorrect because arrays in C decay to pointers (generated typedef is just an alias for an array) which means that the generated binding doesn't have the same ABI as it's rust equivalent. I see no reason to forbid this conversion (as is done for [u64;2]), rather I would expect that the same code is generated as in the 1st example

2: As a side note, I would also like to ask why is there no workaround to support raw [u64;2] in cbindgen like:

#[no_mangle]
pub unsafe extern "C" fn function(arg: [u64; 2]) -> [u64; 2] {
    a
}

Possible solutions to support this are to generate a custom wrapper struct on the C side, or maybe add a cbindgen.toml option

Somewhat related to #171 but this issue is, as far as I can see, resolved and should be closed

mversic avatar Aug 16 '22 11:08 mversic

related to #528

mversic avatar Aug 16 '22 11:08 mversic

Is the ABI of [T; N] exactly equivalent in all cases to struct { T member[N]; };? I don't think so. I think this is one of those cases where we should probably just refuse to generate the type as an argument (so, much like #528 indeed).

emilio avatar Aug 16 '22 12:08 emilio

are you saying that improper_ctypes lint has a bug and should be trigger in this example as well?

#[repr(transparent)]
pub struct K([u64; 2]);
#[no_mangle]
pub unsafe extern "C" fn function(arg: K) -> K {
    a
}

mversic avatar Aug 16 '22 13:08 mversic

Yes, I don't see how that's safer than just pub type K = [u64; 2];

emilio avatar Aug 16 '22 13:08 emilio

Is the ABI of [T; N] exactly equivalent in all cases to struct { T member[N]; };? I don't think so.

well, I think so. I think the main fuss about arrays is that they decay into pointers in C. Other then that, I would expect the ABI to be the same as C ABI. Therefore, if you generate an array wrapped in a struct in the C header file, you should be able to use arrays in FFI on Rust side

Yes, I don't see how that's safer than just pub type K = [u64; 2];

Seems this is something we're going to have to ask on a Rust channel. I'll see about that

mversic avatar Aug 16 '22 14:08 mversic

well, I think so. I think the main fuss about arrays is that they decay into pointers in C. Other then that, I would expect the ABI to be the same as C ABI. Therefore, if you generate an array wrapped in a struct in the C header file, you should be able to use arrays in FFI on Rust side

Sure, as long as the Rust side also wraps them on a struct. But #[repr(transparent)] is not a struct ABI-wise, so I don't see what guarantees they have the same ABI.

emilio avatar Aug 16 '22 14:08 emilio

Sure, as long as the Rust side also wraps them on a struct. But #[repr(transparent)] is not a struct ABI-wise, so I don't see what guarantees they have the same ABI.

from the looks of it, you might be correct. All in all, I agree with your suggestion now, and that is to refuse to generate repr(transparent) struct that wraps array. At least until proven otherwise. Whether or not calling convention for an array is the same as for struct, I don't know

mversic avatar Aug 16 '22 14:08 mversic

I think this was a rust bug, and it triggers improper_ctypes now.

emilio avatar Apr 15 '24 01:04 emilio