-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Distinguish repr(C) ZSTs from others in ABI compatibility rules
#157973
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
31d03e7
34de188
41e5d13
a01211b
a9bed71
3e86bb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1841,7 +1841,15 @@ mod prim_ref {} | |||||||||
| /// call will be valid ABI-wise. The callee receives the result of transmuting the function pointer | ||||||||||
| /// from `fn()` to `fn(i32)`; that transmutation is itself a well-defined operation, it's just | ||||||||||
| /// almost certainly UB to later call that function pointer.) | ||||||||||
| /// - Any two types with size 0 and alignment 1 are ABI-compatible. | ||||||||||
| /// - Any two types fulfilling all the following conditions are ABI-compatible; | ||||||||||
| /// such types are said to have "trivial ABI": | ||||||||||
|
Comment on lines
+1844
to
+1845
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| /// - Size 0 | ||||||||||
| /// - Alignment 1 | ||||||||||
| /// - Not `repr(C)` | ||||||||||
| /// - Not a `repr(transparent)` wrapper around a type that fails to satisfy these conditions | ||||||||||
|
RalfJung marked this conversation as resolved.
Outdated
|
||||||||||
| /// - Not an array whose element type fails to satisfy these conditions | ||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We exempt arrays for 2 reasons:
We could also simplify this clause by saying merely:
Suggested change
The downside would be a larger breaking change. Note that
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with saying that "all arrays with trivial-ABI element type have trivial ABI". However, even that less-breaking version breaks EDIT: Ah, |
||||||||||
| /// - A `repr(transparent)` type is ABI-compatible with its unique field that does not have trivial ABI | ||||||||||
| /// (as defined above). If there is no such field, the type has trivial ABI. | ||||||||||
| /// - A `repr(transparent)` type `T` is ABI-compatible with its unique non-trivial field, i.e., the | ||||||||||
| /// unique field that doesn't have size 0 and alignment 1 (if there is such a field). | ||||||||||
|
Jules-Bertholet marked this conversation as resolved.
Outdated
|
||||||||||
| /// - `i32` is ABI-compatible with `NonZero<i32>`, and similar for all other integer types. | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the implication here is that
()is repr(C)? Am I reading that right? Where do we make that guarantee? Or is the thinking that the language here would make()and#[repr(C)] struct Foo;not ABI compatible?One callout is that ZSTs aren't (I think?) standardized -- C and C++ without extensions both require types to be non-ZST if I remember right (e.g., see https://stackoverflow.com/a/2632075). Maybe that has changed since then though?
It seems like at minimum, it would be nice to avoid weakening this guarantee for Rust ABI even if we do so for C ABIs as a result of the weird platforms.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this.
Correct.