-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tracking issue for RFC 1758: Specify repr(transparent)
#43036
Comments
repr(transparent)
Mentoring instructions unavailable. This is sadly not that easy, as we have too much code that still relies on assumptions that such types do not exist. I have an in-progress branch which requires implementing the necessary support for "newtype unpacking" but no time frame for completion. |
Is making types like |
@RalfJung For a change at that level, a PR approved by the libs team suffices. |
Using bitflags for FFI is unsound in linux32, and it causes crashes like https://bugzilla.mozilla.org/show_bug.cgi?id=1406952. This is pretty similar to rust-lang/rust-bindgen#439. For this to be properly type safe we need to wait for rust-lang/rust#43036. Hopefully that's not for too long.
Using bitflags for FFI is unsound in linux32, and it causes crashes like https://bugzilla.mozilla.org/show_bug.cgi?id=1406952. This is pretty similar to rust-lang/rust-bindgen#439. For this to be properly type safe we need to wait for rust-lang/rust#43036. Hopefully that's not for too long.
The RFC makes contradictory statements about the interaction with
#[repr(transparent, align = "128")]
struct BogusAlign(f64);
#[repr(transparent, packed)]
struct BogusPacked(f64); ... while the Detailed Design section says:
#[repr(transparent, align = "128")]
struct OverAligned(f64); // Behaves as a bare f64 with 128 bits alignment.
#[repr(C, transparent)]
struct BogusRepr(f64); // Nonsensical, repr cannot be C and transparent. AFAICT the RFC initially prohibited repr(transparent, align), then in the discussion some people asked for it to be allowed, but later the rationale for that was called into question and there was no consensus or (explicit) decision. So it's hard to see which alternative was intended to be the final state by the RFC author and the lang team. |
I'd disallow it since it significantly complicates the implementation IMO. Is there any motivation for allowing the combination? |
Discussion is mostly rust-lang/rfcs#1758 (comment) plus the immediate replies. The motivation is not immediately clear to me, just that a nested wrapper type with |
@nox tells me on IRC they have no use case for it. Let's amend the RFC to consistently reject repr(transparent, align). If someone does find a use case, they can deal with the implementation effort and amend the RFC again. |
Sorry, it's been forever since we had that conversation. Maybe I'm overlooking something, but I think #[repr(transparent, align=16)]
struct Aes256Key([u8; 32]);
#[repr(transparent, align=16)]
struct AesNonce([u8; 16]);
// Keep this in sync with the C definition in aes.h
#[repr(C)]
struct Aes256Context {
key: Aes256Key,
...
nonce: AesNonce,
...
}
extern aes256Encrypt(context: &mut Aes256Context, ...); I'm not sure how one would express this otherwise. |
Is there any ABI where you need EDIT: Also, are you ever passing or returning |
Consider: typedef uint8_t AES256Key[32];
typedef uint8_t AES128Key[16];
typedef uint8_t AESNonce[16];
struct Aes256Context {
alignas(16) AES256Key key;
alignas(16) AES256Nonce nonce;
}; Notice in particular that in this C code
Is that true? Maybe I'm misunderstanding the purpose of struct Outer1 {
struct Wrapper value1;
struct Wrapper value2;
};
struct Wrapper {
uint8_t value[12];
};
struct Outer2 {
uint8_t value1[12];
uint8_t value2[12];
}; Are these structures guaranteed, by C, to have the same memory layout? I don't remember the details but I think the answer is "no". And that's why
I think a struct with one field, like |
@briansmith That would imply EDIT: also, it looks like the correct translation of the C code would be |
The final comment period, with a disposition to merge, as per the review above, is now complete. |
… as the feature is ready for stabilization: rust-lang/rust#43036 (comment)
Stabilization PR: #51395 |
Tracking issue FCP: rust-lang#43036 (comment) Reference PR: rust-lang/reference#353
Stabilize #[repr(transparent)] Tracking issue FCP: #43036 (comment) Reference PR: rust-lang/reference#353
Oops, this should have been closed by #51562. |
Add #[repr(transparent)] to some libcore types * `UnsafeCell` * `Cell` * `NonZero*` * `NonNull` * `Unique` CC #43036
Add #[repr(transparent)] to some libcore types * `UnsafeCell` * `Cell` * `NonZero*` * `NonNull` * `Unique` CC #43036
Could I ask here for some eyes on a suggestion to add |
Using bitflags for FFI is unsound in linux32, and it causes crashes like https://bugzilla.mozilla.org/show_bug.cgi?id=1406952. This is pretty similar to rust-lang/rust-bindgen#439. For this to be properly type safe we need to wait for rust-lang/rust#43036. Hopefully that's not for too long.
RFC
This is a tracking issue for the RFC "Specify
repr(transparent)
" (rust-lang/rfcs#1758).Steps:
The text was updated successfully, but these errors were encountered: