Skip to content
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

rustdoc: Add unstable CLI option to show basic type layout information #83501

Merged
merged 15 commits into from
May 12, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Mar 25, 2021

Closes #75988.

Right now it just shows the size.

@camelid camelid added A-layout Area: Memory layout of types T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 25, 2021
@rust-highfive
Copy link
Collaborator

r? @jyn514

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 25, 2021
@camelid
Copy link
Member Author

camelid commented Mar 25, 2021

I placed the section at the bottom of the page (below trait impls) so it doesn't get in the way of seeing the docs. It looks like this (for std::any::TypeId):

image

@camelid
Copy link
Member Author

camelid commented Mar 25, 2021

Weirdly, it doesn't work on many types (e.g. Vec, Option). I'm not sure why this is. Any ideas?

@jyn514
Copy link
Member

jyn514 commented Mar 26, 2021

cc @rust-lang/libs - I think this could be a reasonable change, but I also don't want to encourage people to use transmute() more because it happens to work on their platform. Is this info you're interested in documenting (assuming rustdoc adds some sort of unstable icon)? Are there any changes you'd like to see?

@camelid
Copy link
Member Author

camelid commented Mar 26, 2021

cc rust-lang/libs - I think this could be a reasonable change, but I also don't want to encourage people to use transmute() more because it happens to work on their platform. Is this info you're interested in documenting (assuming rustdoc adds some sort of unstable icon)? Are there any changes you'd like to see?

I also think a reasonable choice would be to add an unstable option, rustdoc -Z show-layout or something, that people could turn on for their own projects and would be off-by-default. I think this information would be useful more on projects you're developing (e.g., within rustdoc or rustc) rather than on libraries you're using. Although that's just my personal use case; maybe others find it useful to look at the size of library types for some reason I'm unaware of.

@jyn514
Copy link
Member

jyn514 commented Mar 26, 2021

rustdoc -Z show-layout

There's already -Z print-type-sizes, I don't think it should be duplicated in rustdoc.

@camelid
Copy link
Member Author

camelid commented Mar 26, 2021

There's already -Z print-type-sizes, I don't think it should be duplicated in rustdoc.

Interesting, I hadn't heard of that flag. But how is it duplication? The flags are very different; the rustc one prints to stdout, while the rustdoc one would add a section to the docs. The advantage of the rustdoc use case is that you can see all the information without running a local build, and that the data is well-structured. Based on testing the rustc flag just now, the output seems hard to digest since it's printing the size of all kinds of types that I did not define; e.g., the size of a BTreeMap node and the size of some type in a derived serde impl.

@BurntSushi
Copy link
Member

Is there a screenshot of what this looks like?

I agree that showing size info could be quite precarious due to platform differences. The obvious concern, I guess, is whether folks reading docs will interpret that size info as an API guarantee. I imagine that in most cases, it isn't.

Popping up a level, this kind of info seems useful in fairly limited contexts. Just thinking about my own experience, the number of times I've needed to care about information like this is very very small.

@camelid
Copy link
Member Author

camelid commented Mar 26, 2021

Is there a screenshot of what this looks like?

See #83501 (comment).

I agree that showing size info could be quite precarious due to platform differences. The obvious concern, I guess, is whether folks reading docs will interpret that size info as an API guarantee. I imagine that in most cases, it isn't.

Yeah, I'm planning to add a notice to the section saying that type layout is not an API guarantee unless otherwise stated by the library docs, as suggested in #75988 (comment).

Popping up a level, this kind of info seems useful in fairly limited contexts. Just thinking about my own experience, the number of times I've needed to care about information like this is very very small.

I think this sort of information is useful more as a library or application author, rather than user. That's why I suggested making it an off-by-default flag, that could be used like --document-private-items (#83501 (comment)).

This sort of information is useful when trying to shrink the sizes of types within the compiler or rustdoc itself, and I've also just been curious to see what the sizes are in my own projects. I could use something like rustc_data_structures::static_assert_size!, but I'd prefer being able to browse the docs and see the information nicely laid out for each type.

@the8472
Copy link
Member

the8472 commented Mar 27, 2021

Yeah, I'm planning to add a notice to the section saying that type layout is not an API guarantee unless otherwise stated by the library docs

Is this just meant for compiler-internal use? Otherwise library authors couldn't make that kind of promise for repr(Rust) in the first place since since it is unspecified.

@m-ou-se
Copy link
Member

m-ou-se commented Mar 27, 2021

People regularly use docs generated for a different platform (with different sizes) than they're targetting, such as the docs on docs.rs and on doc.rust-lang.org. So this should definitely be off by default.

As an off-by-default rustdoc feature, I don't think this really concerns the libs team.

Personally, I can see myself enabling this feature for some embedded projects.

@camelid
Copy link
Member Author

camelid commented Mar 28, 2021

Yeah, I'm planning to add a notice to the section saying that type layout is not an API guarantee unless otherwise stated by the library docs

Is this just meant for compiler-internal use? Otherwise library authors couldn't make that kind of promise for repr(Rust) in the first place since since it is unspecified.

No, it's meant for use by other projects too (I can see myself potentially using it on one of my projects). And yes, you're right that library authors can't guarantee repr(Rust) layout. I should have elaborated that I was planning on saying stuff about how layout is not guaranteed for repr(Rust).

@camelid camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 28, 2021
@jyn514 jyn514 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2021
@camelid
Copy link
Member Author

camelid commented Apr 8, 2021

Rebased so I can use the fancy new download-rustc = "if-unchanged" option 😁

@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 13, 2021
src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved
src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved
src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved
src/librustdoc/lib.rs Show resolved Hide resolved
@camelid
Copy link
Member Author

camelid commented Apr 13, 2021

The current version looks like this:

image

@jyn514 jyn514 changed the title rustdoc: Show basic type layout information rustdoc: Add unstable CLI option to show basic type layout information Apr 13, 2021
src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved
src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved
src/test/rustdoc/type-layout.rs Show resolved Hide resolved
src/test/rustdoc/type-layout.rs Outdated Show resolved Hide resolved
src/test/rustdoc/type-layout.rs Show resolved Hide resolved
src/test/rustdoc/type-layout.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2021
@GuillaumeGomez
Copy link
Member

Still looks good to me. Let's try again! :D

@bors: r=jyn514,GuillaumeGomez

@bors
Copy link
Contributor

bors commented May 11, 2021

📌 Commit d43701c has been approved by jyn514,GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request May 11, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#83501 (rustdoc: Add unstable CLI option to show basic type layout information)
 - rust-lang#85018 (shrinking the deprecated method span)
 - rust-lang#85124 (rustdoc: remove explicit boolean comparisons.)
 - rust-lang#85136 (Change param name (k to key and v to value) in std::env module)
 - rust-lang#85162 (Fix typo in variable name)
 - rust-lang#85187 (Use .name_str() to format primitive types in error messages)
 - rust-lang#85191 (Improve rustdoc gui tester)
 - rust-lang#85196 (Revert "Auto merge of rust-lang#84797 - richkadel:cover-unreachable-statements…)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 40be1d3 into rust-lang:master May 12, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 12, 2021
@camelid camelid deleted the rustdoc-layout branch May 13, 2021 19:34
@pickfire
Copy link
Contributor

@camelid I wanted to try this out but I can't seemed to try it. I did env RUSTDOCFLAGS='-Z unstable-options --show-type-layout' ./x.py doc library/std/ but it does not show the layout stuff like it was shown here, did I do anything wrong? My config.toml

profile = "tools"
changelog-seen = 2

I am using f58631b which was yesterday's commit.

@jyn514
Copy link
Member

jyn514 commented May 30, 2021

@pickfire you need --stage 1

@pickfire
Copy link
Contributor

Wait, I tried it again with --stage 1 and I found it. It wasn't at the place I thought it would be, it's at the bottom of the page, no wonder I can't find it last time. I think it will work even if I build it without --stage 1.
image

fee1-dead added a commit to fee1-dead-contrib/rust that referenced this pull request Aug 31, 2021
jackh726 added a commit to jackh726/rust that referenced this pull request Sep 8, 2021
…r=camelid

Rustdoc: Report Layout of enum variants

Followup of rust-lang#83501, Fixes rust-lang#86253.

cc `@camelid`

`@rustbot` label A-rustdoc
camelid added a commit to camelid/rust that referenced this pull request Dec 9, 2021
At first, you might think, "Why not just click through to the aliased
type?" But, if a type alias instantiates all of the generic parameters
of the aliased type, then it can show layout info even though the
aliased type cannot (because we can't compute layout for generic types).
So, I think it's useful to show layout info for type aliases.

This is a followup of 78d4b453ad2e19d44011b26fc55c949bff5dba3d
(originally part of rust-lang#83501).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2021
rustdoc: Show type layout for type aliases

Fixes rust-lang#91265.

At first, you might think, "Why not just click through to the aliased
type?" But, if a type alias instantiates all of the generic parameters
of the aliased type, then it can show layout info even though the
aliased type cannot (because we can't compute layout for generic types).
So, I think it's useful to show layout info for type aliases.

This is a followup of 78d4b453ad2e19d44011b26fc55c949bff5dba3d
(originally part of rust-lang#83501).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 10, 2021
rustdoc: Show type layout for type aliases

Fixes rust-lang#91265.

At first, you might think, "Why not just click through to the aliased
type?" But, if a type alias instantiates all of the generic parameters
of the aliased type, then it can show layout info even though the
aliased type cannot (because we can't compute layout for generic types).
So, I think it's useful to show layout info for type aliases.

This is a followup of 78d4b453ad2e19d44011b26fc55c949bff5dba3d
(originally part of rust-lang#83501).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 11, 2021
rustdoc: Show type layout for type aliases

Fixes rust-lang#91265.

At first, you might think, "Why not just click through to the aliased
type?" But, if a type alias instantiates all of the generic parameters
of the aliased type, then it can show layout info even though the
aliased type cannot (because we can't compute layout for generic types).
So, I think it's useful to show layout info for type aliases.

This is a followup of 78d4b453ad2e19d44011b26fc55c949bff5dba3d
(originally part of rust-lang#83501).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 11, 2021
rustdoc: Show type layout for type aliases

Fixes rust-lang#91265.

At first, you might think, "Why not just click through to the aliased
type?" But, if a type alias instantiates all of the generic parameters
of the aliased type, then it can show layout info even though the
aliased type cannot (because we can't compute layout for generic types).
So, I think it's useful to show layout info for type aliases.

This is a followup of 78d4b453ad2e19d44011b26fc55c949bff5dba3d
(originally part of rust-lang#83501).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Area: Memory layout of types S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rustdoc show rustc_layout