-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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 all commits
54459df
2f9ad44
f64a23a
207c801
1178cd3
45dfb46
f832c93
fd75d00
3653c35
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1858,9 +1858,22 @@ 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. | ||||||
| /// - 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). | ||||||
| /// - Any two types with "trivial ABI" are ABI-compatible. | ||||||
| /// A type has trivial ABI if is satisfies all of the following: | ||||||
| /// - It has size 0. | ||||||
| /// - It has alignment 1. | ||||||
| /// - One of the following apply: | ||||||
| /// - It is a `repr(Rust)` (implicitly or explicitly) `struct`, `enum`, `union` (regardless of its fields, | ||||||
| /// and possibly with additional `repr` modifiers such as `packed`). | ||||||
| /// - It is a [tuple][prim_tuple] (regardless of its fields, and including [`()`][prim_unit]). | ||||||
|
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. Less parens, please.
Suggested change
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. Rephrasing without the specific note about parens: I think reminding people that I initially considered suggesting just removing every "regardless of..." case and letting the absence of qualifier speak for itself. There are good reasons to not, however. |
||||||
| /// - It is a `repr(transparent)` `struct`, `enum`, or `union`, and all fields have trivial ABI. | ||||||
| /// - It is an array, and its element type has trivial ABI. (This requirement applies even to arrays of length 0.) | ||||||
|
workingjubilee marked this conversation as resolved.
|
||||||
| /// - It is [the never type `!`][prim_never]. | ||||||
|
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. Are we completely sure that we want
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. People tell me that that's fine, and I don't know enough about what ABIs do here to be able to point at potential problems. Cc @workingjubilee @chorman0773 @CAD97 Note that if we don't want this, we'll need to change more than just remove this last item in the list. For instance, we'd also have to declare
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 not aware of any problems with doing it ... there's no separate signature for
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. One could imagine a custom rust ABI that omits the information required to return from the function if we know it anyway doesn't return. OTOH at least in LLVM the concept of an uninhabited type does not exist and
Contributor
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. Notably, you can call a _Noreturn/[[noreturn]] function via a pointer to
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. At worst, if we ever want to do FFI with some hypothetical language whose ABI does make a distinction, we can have a separate |
||||||
| // - It is a pattern type or an unsafe binder type, and the inner type has trivial ABI. | ||||||
| // (These are still unstable so intentionally not included in the user-visible doc comment, as their rules may change.) | ||||||
| /// - It is a function item type or closure type. | ||||||
|
Comment on lines
+1870
to
+1874
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 think this change is reasonable. We have to change these rules because they're just wrong and this is unfixable. The rules as stated seem like a reasonable way to fix it. My main question would be: Why include arrays? I get a bit worried about that one because array arguments have weird properties in C. We don't want to run into another case in the future where Rust cannot represent certain kinds of C ABIs involving arrays.
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. See discussion thread at #157973 (comment). The "and its element type has trivial ABI" requirement should exclude array types with C equivalents.
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. Hrm, I guess you are right that the element type having a trivial ABI requirement is probably sufficient.
Jules-Bertholet marked this conversation as resolved.
|
||||||
| /// - A `repr(transparent)` type is ABI-compatible with its unique field that does not have trivial ABI | ||||||
| /// (as defined above), if such a field exists. (Note that if no such field exists, then the `repr(transparent)` type itself has trivial ABI, so the case above applies.) | ||||||
| /// - `i32` is ABI-compatible with `NonZero<i32>`, and similar for all other integer types. | ||||||
| /// - If `T` is guaranteed to be subject to the [null pointer | ||||||
| /// optimization](option/index.html#representation), and `E` is an enum satisfying the following | ||||||
|
|
||||||
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.