Rustdoc: Report Layout of enum variants #86263

fee1-dead · 2021-06-13T09:52:26Z

Followup of #83501, Fixes #86253.

cc @camelid

@rustbot label A-rustdoc

rust-highfive · 2021-06-13T09:52:29Z

r? @ollie27

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

src/librustdoc/html/render/print_item.rs

jyn514 · 2021-07-08T12:44:20Z

src/librustdoc/html/render/print_item.rs

@@ -1571,6 +1573,38 @@ fn document_type_layout(w: &mut Buffer, cx: &Context<'_>, ty_def_id: DefId) {
                    pl = if bytes == 1 { "" } else { "s" },
                );
            }
+            if let Variants::Multiple { variants, .. } = &ty_layout.layout.variants {
+                if !variants.is_empty() {


Can you test what happens for the empty case? Does it show 0 bytes or unsized? I think ideally it would show "uninhabited" or something.

It shows 0 bytes. I think it would be more suitable for a separate PR as a followup of the original PR.

src/librustdoc/html/render/print_item.rs

jyn514

r=me with the test case updated

src/test/rustdoc/type-layout.rs

src/librustdoc/html/render/print_item.rs

pickfire · 2021-07-10T05:25:46Z

src/librustdoc/html/render/print_item.rs

+            w.write_str("<p><strong>Size:</strong> ");
+            write_size_of_layout(w, ty_layout.layout);
+            writeln!(w, "</p>");
+            if let Variants::Multiple { variants, .. } = &ty_layout.layout.variants {


What about Variants::Single?

Then it wouldn't matter because its size is already reported previously.

pickfire · 2021-07-10T05:26:40Z

@fee1-dead Do you have a sample screenshot on how it looks like?

src/librustdoc/html/render/print_item.rs

fee1-dead · 2021-07-10T13:40:24Z

@fee1-dead Do you have a sample screenshot on how it looks like?

@rustbot label -S-waiting-on-author S-waiting-on-review

jyn514 · 2021-07-10T21:59:32Z

@fee1-dead hmm, that doesn't seem super helpful - I assume those are all 0 because they don't have fields? Could we avoid showing the size in that case and only report it for fields?

Actually at that point I'm not really sure how much purpose this serves - why wouldn't the user just look at the size shown for the type in the field?

jyn514 · 2021-08-30T21:18:08Z

@camelid do you have opinions on #86263 (comment) ?

camelid · 2021-08-30T21:21:47Z

@camelid do you have opinions on #86263 (comment) ?

I find it annoying to have to click on each field's type to try to manually compute its size, especially if the type is something like Option<(DefId, Foo)>, where I have to manually add up its components, try to figure out what niches there are, and estimate alignment. And it gets especially bothersome for types with many variants like clean::Type.

I think it'd be useful to have a summary of the sizes of each variant.

Is that what you were asking about, or was it something else?

jyn514 · 2021-08-30T21:28:20Z

especially if the type is something like Option<(DefId, Foo)>

👍 makes sense, I hadn't thought about anonymous types. Ok, I think this makes sense then.

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

src/librustdoc/html/render/print_item.rs

src/test/rustdoc/type-layout.rs

GuillaumeGomez · 2021-09-02T18:09:47Z

src/test/rustdoc/type-layout.rs

+// @has type_layout/enum.Variants.html 'Size: '
+// @has - '2 bytes'
+// @has - '<code>A</code>: 0 bytes'
+// @has - '<code>B</code>: 1 byte'


Just to confirm: this checks that this "text" is in the raw HTML directly, right?

What's odd is I tried @has - '<strong>Size:</strong> 8 bytes on master on an existing test case, but it failed. I'm not sure why these tests are passing.

camelid · 2021-09-06T17:53:28Z

Thanks!

@bors r+ rollup

bors · 2021-09-06T17:53:29Z

📌 Commit c0451f7 has been approved by camelid

Rollup of 9 pull requests Successful merges: - rust-lang#86263 (Rustdoc: Report Layout of enum variants) - rust-lang#88541 (Add regression test for rust-lang#74400) - rust-lang#88553 (Improve diagnostics for unary plus operators (rust-lang#88276)) - rust-lang#88594 (More symbolic doc aliases) - rust-lang#88648 (Correct “copies” to “moves” in `<Option<T> as From<T>>::from` doc, and other copyediting) - rust-lang#88691 (Add a regression test for rust-lang#88649) - rust-lang#88694 (Drop 1.56 stabilizations from 1.55 release notes) - rust-lang#88712 (Fix docs for `uX::checked_next_multiple_of`) - rust-lang#88726 (Fix typo in `const_generics` replaced with `adt_const_params` note) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Add layout info for enum variant and locals The size of enum variant is what rustdoc shows (rust-lang/rust#86263). I also added layout info for locals since it helps finding size of unnameable types like closures inside other structs or impl traits.

rust-highfive assigned ollie27 Jun 13, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2021

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the rustdoc-layout-variants branch from a6747c4 to 7c87a39 Compare June 13, 2021 10:17

Urgau reviewed Jun 13, 2021

View reviewed changes

src/librustdoc/html/render/print_item.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the rustdoc-layout-variants branch from 7c87a39 to 4a2e8ad Compare June 13, 2021 10:31

LeSeulArtichaut reviewed Jun 13, 2021

View reviewed changes

src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved

fee1-dead force-pushed the rustdoc-layout-variants branch from 4a2e8ad to 1ab9596 Compare June 14, 2021 02:46

rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jun 21, 2021

fee1-dead changed the title ~~Report Layout of enum variants~~ Rustdoc: Report Layout of enum variants Jun 23, 2021

jyn514 suggested changes Jul 8, 2021

View reviewed changes

jyn514 added the requires-nightly This issue requires a nightly compiler in some way. label Jul 8, 2021

This comment has been minimized.

Sign in to view

jyn514 approved these changes Jul 9, 2021

View reviewed changes

src/test/rustdoc/type-layout.rs Outdated Show resolved Hide resolved

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 Jul 9, 2021

jyn514 assigned jyn514 and unassigned ollie27 Jul 9, 2021

jyn514 mentioned this pull request Jul 9, 2021

Rustdoc: size layout should show "uninhabited" for empty enums, not 0 #87008

Closed

camelid reviewed Jul 9, 2021

View reviewed changes

src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved

pickfire reviewed Jul 10, 2021

View reviewed changes

src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved

rustbot 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 Jul 10, 2021

jyn514 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 11, 2021

fee1-dead added 4 commits August 31, 2021 05:24

Report Layout of enum variants

aff4cd5

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

Apply suggestions

c349b79

Report variant size without the discriminant

8096910

Apply suggestions

5f1505e

fee1-dead force-pushed the rustdoc-layout-variants branch from ae9e562 to 5f1505e Compare August 31, 2021 05:59

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 Aug 31, 2021

camelid reviewed Sep 2, 2021

View reviewed changes

src/librustdoc/html/render/print_item.rs Show resolved Hide resolved

camelid reviewed Sep 2, 2021

View reviewed changes

src/test/rustdoc/type-layout.rs Show resolved Hide resolved

GuillaumeGomez reviewed Sep 2, 2021

View reviewed changes

camelid 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 Sep 2, 2021

rustbot 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 Sep 6, 2021

Correctly handle niche of enum

c0451f7

fee1-dead force-pushed the rustdoc-layout-variants branch from 5065be6 to c0451f7 Compare September 6, 2021 07:11

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 Sep 6, 2021

jackh726 mentioned this pull request Sep 8, 2021

Rollup of 9 pull requests #88750

Merged

bors merged commit 2f2aed1 into rust-lang:master Sep 8, 2021

rustbot added this to the 1.57.0 milestone Sep 8, 2021

HKalbasi mentioned this pull request May 18, 2023

feat: Add layout info for enum variant and locals rust-lang/rust-analyzer#14845

Merged

fee1-dead deleted the rustdoc-layout-variants branch July 19, 2024 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rustdoc: Report Layout of enum variants #86263

Rustdoc: Report Layout of enum variants #86263

fee1-dead commented Jun 13, 2021 •

edited

Loading

rust-highfive commented Jun 13, 2021

This comment has been minimized.

This comment has been minimized.

jyn514 Jul 8, 2021

fee1-dead Jul 9, 2021

This comment has been minimized.

jyn514 left a comment

pickfire Jul 10, 2021

fee1-dead Jul 10, 2021

pickfire commented Jul 10, 2021

fee1-dead commented Jul 10, 2021

jyn514 commented Jul 10, 2021

jyn514 commented Aug 30, 2021 •

edited

Loading

camelid commented Aug 30, 2021

jyn514 commented Aug 30, 2021

GuillaumeGomez Sep 2, 2021

camelid Sep 2, 2021

camelid commented Sep 6, 2021

bors commented Sep 6, 2021

Rustdoc: Report Layout of enum variants #86263

Rustdoc: Report Layout of enum variants #86263

Conversation

fee1-dead commented Jun 13, 2021 • edited Loading

rust-highfive commented Jun 13, 2021

This comment has been minimized.

This comment has been minimized.

jyn514 Jul 8, 2021

Choose a reason for hiding this comment

fee1-dead Jul 9, 2021

Choose a reason for hiding this comment

This comment has been minimized.

jyn514 left a comment

Choose a reason for hiding this comment

pickfire Jul 10, 2021

Choose a reason for hiding this comment

fee1-dead Jul 10, 2021

Choose a reason for hiding this comment

pickfire commented Jul 10, 2021

fee1-dead commented Jul 10, 2021

jyn514 commented Jul 10, 2021

jyn514 commented Aug 30, 2021 • edited Loading

camelid commented Aug 30, 2021

jyn514 commented Aug 30, 2021

GuillaumeGomez Sep 2, 2021

Choose a reason for hiding this comment

camelid Sep 2, 2021

Choose a reason for hiding this comment

camelid commented Sep 6, 2021

bors commented Sep 6, 2021

fee1-dead commented Jun 13, 2021 •

edited

Loading

jyn514 commented Aug 30, 2021 •

edited

Loading