Skip to content

Documentation of std::mem::size_of confusingly only details the #[repr(C)] case #55115

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

Open
vincentdephily opened this issue Oct 16, 2018 · 6 comments
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-repr Area: the `#[repr(stuff)]` attribute C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority

Comments

@vincentdephily
Copy link
Contributor

The Size of Structs paragraph bases its explanation of struct sizes on the idea that field are "ordered by declaration order", which is no longer the case since #37429 got merged. This might prompt a user to pointlessly manually order fields to optimize for size.

Instead, the documentation should mention the issue of field alignments, and explain that the compiler will reorder fields to minimize padding. The exact algorithm might be a bit too complex and/or subject to change, so I don't think that it needs to be described completely. It's probably enough to say something like "the compiler reorders fields for optimisation, don't expect a particular order unless using one of the repr specifier".

@vincentdephily vincentdephily changed the title Documentation of std::mem::size_of is incorrect for struct size, since fields can be reordered by the compiler. Documentation of std::mem::size_of confusingly only details the #[repr(C)] case Oct 16, 2018
@Havvy
Copy link
Contributor

Havvy commented Oct 16, 2018

You're misreading the documentation. The Size of Structs paragraph is a sub-heading of the "Size of #[repr(C)] items" section. But the difference in heading size makes it easy to miss that fact.

@vincentdephily
Copy link
Contributor Author

Thanks for pointing this out. Goes to show that the docs could be improved. It'd be nice to get paragraphs for other #[repr(...)] settings. Changed bug title.

@Havvy Havvy added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Oct 16, 2018
@scottmcm
Copy link
Member

I wonder if this text should just be moved into the unsafe guidelines instead? Are there any cases in safe code where the exact layout matters for correctness (not just performance)?

@vincentdephily
Copy link
Contributor Author

Well #[repr(C)] is safe and about correctness rather than performance.

I don't think that having all the possible memory layouts detailed on the size_of page is a must-have, maybe that should be left to the nomicon or to wherever else repr variants are documented. The size_of docs could then explain why repr and other factors matter, briefly list the various options, and link to the relevant doc for each. But don't detail repr(C) while leaving everything else unmentioned.

@Havvy
Copy link
Contributor

Havvy commented Oct 17, 2018

@scottmcm Pointer shenanigans and transmutes. FWIW, this information does exist on the reference, though IMO, it almost makes sense to document here since they're the explanation of the output of why the output of the function is what it says.

@steveklabnik steveklabnik added the P-medium Medium priority label Dec 27, 2018
@DevQps
Copy link
Contributor

DevQps commented Mar 30, 2019

We could maybe try to make the size of structs a bit more verbose by changing:

For structs, the size is determined by the following algorithm.
to
For structs in C representation, the size is determined by the following algorithm.

And then adding to the introduction of the sizeof operator a small paragraph stating that the ordering of fields is not defined by the default Rust representation (not sure if this has a name?), and that the compiler is allows to reorder the fields and apply padding as it sees fit. Or instead of only editing the introduction we could introduce a new section for Rust's default representation.

I can create a PR if there isn't anyone else that wants to pick this up!

@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Mar 15, 2020
@GuillaumeGomez GuillaumeGomez removed the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 14, 2022
@workingjubilee workingjubilee added the A-repr Area: the `#[repr(stuff)]` attribute label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-repr Area: the `#[repr(stuff)]` attribute C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

8 participants