-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Relaxed DST field ordering #3713
Open
clarfonthey
wants to merge
4
commits into
rust-lang:master
Choose a base branch
from
clarfonthey:relaxed-dst-field-ordering
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
- Feature Name: `relaxed_dst_field_ordering` | ||
- Start Date: 2024-10-18 | ||
- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) | ||
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Relax the requirements on struct field ordering for dynamically sized fields: | ||
|
||
* for `repr(Rust)`, `?Sized` fields can be anywhere in the field list, as long as there is only one | ||
* for `repr(C)`, `?Sized` fields only have to be the last non-ZST field, and can be followed by ZST fields | ||
* for `repr(transparent)`, apply both rules, since only one non-ZST field is allowed anyway | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
Rust allows creating structs with dynamically sized fields, but in a very limited way: since the size is dynamic, dynamically sized fields must be the last field in the struct, to avoid making the offsets of fields also dynamic. | ||
|
||
However, `repr(Rust)` allows reordering fields, and this is not not reflected in this rule for dynamically sized fields: no matter what you do, the dynamically sized field must be at the end. This is inconsistent with the rest of the language and limits the ability of authors to reorder the fields in a more natural way, like they can for statically sized structs. | ||
|
||
Additionally, Rust has fully committed to zero-sized fields being truly invisible to struct layout, encoding this in the definition of `repr(transparent)`. So, why are dynamically sized fields unable to be followed by zero-sized fields, or reordered among statically sized fields in a `repr(Rust)` struct? | ||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
Before, structs were allowed to have dynamically sized types (DSTs) in their last field only. Now, this restriction has been relaxed to allow at most one DST field, although it can occur anywhere inside the struct. | ||
|
||
For `repr(C)` structs specifically, an additional requirement is added that the DST must be the last field that is not a zero-sized type (ZST), which is still more permissive than the previous definition. | ||
|
||
The dynamically sized field will always be physically located at the end of the struct, although because `repr(Rust)` can reorder fields and because ZST fields do not affect layout, this doesn't have to be reflected in the actual struct definition. | ||
|
||
# Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
This feature should be relatively easy to implement: we already reorder fields in `repr(Rust)` structs explicitly, so, this is just ensuring that DST fields are always placed last without relying on the definition order. | ||
|
||
The code for `repr(transparent)` effectively doesn't change, and the code for ensuring that the DST is the last field can be mostly reused for `repr(C)`, assuming that ZST fields are still ignored. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
It's work to change the status quo. | ||
|
||
# Rationale and alternatives | ||
[rationale-and-alternatives]: #rationale-and-alternatives | ||
|
||
This arguably simplifies the language and makes the DST field more in line with the existing field ordering rules. | ||
|
||
But we could always not do it, I guess. | ||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
None currently. | ||
|
||
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
None currently. | ||
|
||
# Future possibilities | ||
[future-possibilities]: #future-possibilities | ||
|
||
In the future, we'll hopefully have the ability to define custom DSTs, but such extensions are very compatible with this RFC. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
pretty sure the alignment of a ZST is part of the layout and is visible (and using it in
#[repr(transparent)]
will indeed raise E0690 "transparent struct needs at most one field with non-trivial size or alignment")given that the Ref-level explanation assumed
#[repr(transparent)]
and#[repr(C)]
will be using the same logic i think you just need to mention "ZST" in this RFC is restricted to (size 0, align 1) only.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.
Hopefully the reworded explanation is better: the new rules apply to both
repr(Rust)
andrepr(transparent)
, but the restrictions on what ZSTs are allowed forrepr(transparent)
remain.