Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

doc: improve documentation of packed type layout modifier #318

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

phip1611
Copy link

@phip1611 phip1611 commented Oct 7, 2021

The Rust language reference (and my experience, proved by tests) says, that packed(n) gives you the ability to align the struct too. This bug probably comes from a past version of Rust, where this wasn't supported.

Currently, the packed documentation doesn't mention the n-parameter.

https://doc.rust-lang.org/reference/type-layout.html

src/other-reprs.md Outdated Show resolved Hide resolved
src/other-reprs.md Outdated Show resolved Hide resolved
@phip1611 phip1611 force-pushed the doc-fix-repr-packed branch 2 times, most recently from ebdd236 to 20d86cd Compare November 9, 2021 15:53
@phip1611
Copy link
Author

phip1611 commented Nov 9, 2021

I updated the PR @ehuss . See the latest version here: https://github.com/rust-lang/nomicon/blob/20d86cd0331e1c54bfd85e25326e3e0e41fc979e/src/other-reprs.md

Any more comments?

src/other-reprs.md Outdated Show resolved Hide resolved
src/other-reprs.md Outdated Show resolved Hide resolved
src/other-reprs.md Outdated Show resolved Hide resolved
@phip1611 phip1611 force-pushed the doc-fix-repr-packed branch from c28f75e to 305ffdc Compare December 8, 2021 10:06
@phip1611
Copy link
Author

phip1611 commented Dec 8, 2021

Sorry for the circumstances on this, I obviously understood something wrong. I tried to improve the description in a way,
that others will not fall for the same misleading assumption as me. @ehuss

@phip1611 phip1611 force-pushed the doc-fix-repr-packed branch from 305ffdc to ae11a44 Compare December 8, 2021 10:11
@phip1611 phip1611 changed the title doc: fix deprecated doc of attribute packed doc: improve documentation of packed type layout modifier Dec 8, 2021
@phip1611 phip1611 force-pushed the doc-fix-repr-packed branch from ae11a44 to 93760d4 Compare December 8, 2021 10:12
side-effects.

In particular, most architectures *strongly* prefer values to be aligned. This
may mean the unaligned loads are penalized (x86), or even fault (some ARM
chips). For simple cases like directly loading or storing a packed field, the
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this from the PR if you want. This was necessary so that each paragraph has roughly the same width in the markdown file.

@phip1611 phip1611 requested a review from ehuss December 8, 2021 10:32
src/other-reprs.md Outdated Show resolved Hide resolved
@phip1611 phip1611 force-pushed the doc-fix-repr-packed branch from 93760d4 to fbb0bab Compare January 31, 2022 21:27
@phip1611
Copy link
Author

@ehuss I rebased the PR branch onto the latest master branch.

This commit improves the description of the packed type layout modifier.
@phip1611 phip1611 force-pushed the doc-fix-repr-packed branch from eab859a to c17dd17 Compare April 5, 2024 15:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants