-
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
RFC: Add #[repr(pack = "N")] #1399
RFC: Add #[repr(pack = "N")] #1399
Conversation
Signed-off-by: Peter Atashian <retep998@gmail.com>
Signed-off-by: Peter Atashian <retep998@gmail.com>
Signed-off-by: Peter Atashian <retep998@gmail.com>
* Packing values must be a power of two. | ||
|
||
By specifying this attribute, the alignment of the struct would be the smaller | ||
of the specified packing and the default alignment of the struct otherwise. The |
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.
s/otherwise//
Signed-off-by: Peter Atashian <retep998@gmail.com>
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
This would unfortunately make my life easier even though one of the unstated |
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.
Please remove this paragraph.
Signed-off-by: Peter Atashian <retep998@gmail.com>
# Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
* The behavior specified here should match the behavior of MSVC at least. Does |
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.
This must be resolved. Verify against the behavior of clang (or gcc).
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.
Specifically clang or gcc on a platform other than Windows. On Windows they are supposed to match the msvc ABI.
The feature is necessary. If this behavior is in line with common C compilers, |
[motivation]: #motivation | ||
|
||
Many C/C++ compilers allow a packing to be specified for structs which | ||
effectivally lowers the alignment for a struct and its fields (for example with |
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.
s/effectivally/effectively/
For reference, here's an example of Clang doing this. |
Signed-off-by: Peter Atashian <retep998@gmail.com>
Signed-off-by: Peter Atashian <retep998@gmail.com>
See this comment for an interesting interaction edge case with the alignment RFC regarding required alignment. |
Hear ye, hear ye! This RFC is now entering final comment period. |
This seems right to have. I think the best is |
@ubsan It could become valid with #1559, though, or as @nrc mentions in a comment, the procedural macros RFC. |
I don't mind the syntax either way, as long as I can have this attribute at all. |
I think it should just extend |
Hmm, I think I too prefer |
One area the RFC did not (explicitly) address -- or I missed it -- is what to do if there are multiple Is there any reason one might want multiple specifiers? Like, independent macros might add tighter and tighter requirements? (However, unlike with alignment, it's not obvious to me whether you would want the max or min packing that was specified.) |
I think if we did allow specifying the packing multiple times, that the right thing to do would be to use the smallest packing specified. The alignment of any given field is As for the align RFC, there'd need to be a decision on how it interacts with packing. Whether it does the msvc style |
Huzzah! The @rust-lang/lang team has decided to accept this RFC, with the caveat that the attribute be renamed to We discussed a few other things:
|
Tracking issue: rust-lang/rust#33158 If you'd like to keep following the development of this feature, please subscribe to that issue, thanks! :) |
also adjust from `#[repr(pack = "N")]` to `#[repr(packed = "N")]`
Rendered