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

repr(transparent) on a tuple struct wrapping an array #777

Closed
mversic opened this issue Aug 16, 2022 · 8 comments
Closed

repr(transparent) on a tuple struct wrapping an array #777

mversic opened this issue Aug 16, 2022 · 8 comments

Comments

@mversic
Copy link
Contributor

mversic commented Aug 16, 2022

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
Copy link
Contributor Author

mversic commented Aug 16, 2022

related to #528

@emilio
Copy link
Collaborator

emilio commented Aug 16, 2022

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).

@mversic
Copy link
Contributor Author

mversic commented Aug 16, 2022

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
}

@emilio
Copy link
Collaborator

emilio commented Aug 16, 2022

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

@mversic
Copy link
Contributor Author

mversic commented Aug 16, 2022

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

@emilio
Copy link
Collaborator

emilio commented Aug 16, 2022

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.

@mversic
Copy link
Contributor Author

mversic commented Aug 16, 2022

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

@emilio
Copy link
Collaborator

emilio commented Apr 15, 2024

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

@emilio emilio closed this as completed Apr 15, 2024
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

No branches or pull requests

2 participants