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: size layout should show "uninhabited" for empty enums, not 0 #87008

Closed
jyn514 opened this issue Jul 9, 2021 · 13 comments · Fixed by #108734
Closed

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

jyn514 opened this issue Jul 9, 2021 · 13 comments · Fixed by #108734
Labels
A-layout Area: Memory layout of types A-rustdoc-type-layout Area: `rustdoc --show-type-layout` (nightly-only) C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Jul 9, 2021

Originally posted by @jyn514 in #86263 (comment)

@jyn514 jyn514 added requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jul 9, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jul 9, 2021

cc @camelid @pickfire

@jyn514 jyn514 added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jul 9, 2021
@camelid
Copy link
Member

camelid commented Jul 9, 2021

I'm ambivalent, I don't think showing zero is necessarily wrong, especially since that's what's reported by layout_of.

@jonas-schievink jonas-schievink added the A-layout Area: Memory layout of types label Jul 9, 2021
@asquared31415
Copy link
Contributor

In my opinion, I think that while size 0 is not wrong, that showing that it's uninhabited is more right, or at least more clear and makes the distinction.

@camelid camelid added the A-rustdoc-type-layout Area: `rustdoc --show-type-layout` (nightly-only) label Jul 9, 2021
@pickfire
Copy link
Contributor

pickfire commented Jul 10, 2021

I don't understand uninhabited when I first see it. It is a hard word compared to 0. I think 0 is easier. Why not both? What if we show 0 (uninhabited)?

Also, what if the enum is not empty but have a single/multiple ZST like phantom type?

@GuillaumeGomez
Copy link
Member

I think displaying 0 is better as well, but we can do as @pickfire suggested and simply display both.

@jyn514
Copy link
Member Author

jyn514 commented Jul 10, 2021

0 is wrong. 0 means "this is a zero sized type", like () or NoneError; it can be created. Uninhabited types cannot be created, they are things like ! and enum Void {}. IMO posting 0 there is actively misleading. See also https://internals.rust-lang.org/t/size-of-uninhabited-types/7651.

I don't understand uninhabited when I first see it.

Ok, then we can link to some docs. I didn't see anything in the nomicon but I'm sure they'd take a PR adding it to "exotically sized types".

@jyn514
Copy link
Member Author

jyn514 commented Jul 10, 2021

I'm ambivalent, I don't think showing zero is necessarily wrong, especially since that's what's reported by layout_of.

This is a bug imo, it allows transmuting from ZSTs to uninhabited types which is UB.

@GuillaumeGomez
Copy link
Member

Even if they cannot be created, their size remain 0 as they don't "exist", however since they cannot be constructed, it makes sense to mention they're unhabited types. Well, I don't have a strong conviction on this in any case. I think we all agree for the mention of "unhabited" though.

@pickfire
Copy link
Contributor

Ok, then we can link to some docs. I didn't see anything in the nomicon but I'm sure they'd take a PR adding it to "exotically sized types".

Yeah, that solved the issue of 0 which is wrong. I do think it is better to link it to https://doc.rust-lang.org/stable/reference/glossary.html?highlight=uninhabited#uninhabited, but I think it's wrong to put that in "size" such that it should be in "type" instead, because it doesn't have a size to begin with.

Well, there are parts where rust sacrificed correctness for ease of understanding, one example is mut which means uniqueness not exactly mutable all the time.

@nerandell
Copy link

@rustbot claim

@scottmcm
Copy link
Member

scottmcm commented Oct 19, 2022

Note that uninhabited is orthogonal to size. You can have an uninhabited type of any size.

So I think showing uninhabitedness is good, but in addition to the size+align, not instead.

@camelid
Copy link
Member

camelid commented Oct 19, 2022

Note that uninhabited is orthogonal to size. You can have an uninhabited type of any size.

Hmm, I think that layout_of will report that the size is always 0 for an uninhabited type though, unless that's changed recently.

@scottmcm
Copy link
Member

That would surprise me, since that'd be different from what size_of and align_of do: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e1ad034d9812b5356493e4faefe4be61

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 A-rustdoc-type-layout Area: `rustdoc --show-type-layout` (nightly-only) C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants