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

Guarantee that some structs are zero sized #163

Merged
merged 5 commits into from
Jul 26, 2019

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Jul 8, 2019

This addresses part of #37, by guaranteeing that structs have zero size if they don't contain any non-zero-sized field.

@RalfJung
Copy link
Member

Why are all these size == 0important enough to list them here, wasting precious space and brain resources for every reader?

@RalfJung
Copy link
Member

One way we could avoid all of that, at the cost of a bigger commitment, is by saying that repr(Rust) is the same as repr(C) after the compiler did some field reordering. That would save us a lot of text. Maybe we can make it a bit more relaxed to give ourselves some more freedom for future changes?

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 13, 2019

Why are all these size == 0important enough to list them here, wasting precious space and brain resources for every reader?

Please, do suggest a better way to guarantee all these details, that does not have those downsides.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 13, 2019

is by saying that repr(Rust) is the same as repr(C) after the compiler did some field reordering.

It is unclear to me what the consequences of that are. This should deserve its own issues, where that can be explored. AFAICT we do guarantee that ZSTs in repr(C) structs have zero size, but we also do mention that they can introduce padding. It is unclear to me whether we do currently guarantee that a repr(C) struct containing only ZSTs (of possibly varying alignment) is zero-sized.

In particular, while clang do guarantee that zero-sized fields in C structs have zero size (but maybe add padding), I am not sure whether they give structs containing only ZSTs a size of zero.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 13, 2019

In particular, while clang do guarantee that zero-sized fields in C structs have zero size (but maybe add padding), I am not sure whether they give structs containing only ZSTs a size of zero.

They do in C (https://gcc.godbolt.org/z/Tcu49v), they don't do it in C++ (https://gcc.godbolt.org/z/tn9Mod), but that difference already exist.

@RalfJung
Copy link
Member

Please, do suggest a better way to guarantee all these details, that does not have those downsides.

A separate file would be a good start IMO.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 13, 2019

Shall we then remove all mentions of zero-sized types from all layout files and move them to a separate file?

@RalfJung
Copy link
Member

Hm, that also seems odd somehow as that one file will talk about enums and structus and unions, right?

Maybe having a clearly separated section on "questions around ZST" in each of these files is better. I am not sure.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 15, 2019

@RalfJung I agree with you in that we need to find a better solution, but I believe it will be easier to refactor the guarantees after we agree on what those should be first. It might well be that, due to the refactor, we end up guaranteeing a little bit more here and there due to making things more general.

I think that a section on each file is probably necessary to work out all of the details, but we also have a top-level layout document at the beginning of this chapter that is currently empty, where we could try to provide general rules that always apply.

@Lokathor
Copy link
Contributor

Lokathor commented Jul 15, 2019

Two step proposal:

  1. Place an entry for "Zero Sized Type (ZST)" in the Glossary. The definition can be very simple but particularly you want to introduce the acronym to people as part of the glossary entry.
  2. Place the info for how a struct/union/enum ends up being zero sized as a section on the pages for each category of type.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 24, 2019

It's unclear to me whether there is anything else to do with this PR.

The issue that needs resolving is whether this is something we want to guarantee or not, and it is unclear from the comments whether that is the case. If there is no consensus, we should close this PR and document in the issue that there was no consensus about guaranteeing this.

Trying to coalesce these guarantees for easier consumption somewhere is IMO pointless if the guarantees themselves don't have consensus.


Structs with default layout (`repr(Rust)`), layout with increased alignment
(`repr(align(N))`), packed layout (`repr(packed(N))`), or C-compatible layout
(`repr(C)`) are zero-sized, if they contain no fields of non-zero size. That is,
Copy link
Member

Choose a reason for hiding this comment

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

Is this an "if and only if"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so. If a struct contains a field of non-zero size, I don't see how it could end up being zero-sized.

Choose a reason for hiding this comment

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

I can't think of a counter-example, but it seems like only one direction is much commonly thought about and discussed, so I'd be a bit scared to commit to the other direction as well. Besides, it's usually more important to know when something definitely is a ZST (e.g., so you can ignore it for layout related questions), not when something definitely isn't a ZST.

Copy link
Member

Choose a reason for hiding this comment

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

Fair, but but then we should try to word this in a way that it is clear that only one direction is defined. In particular with the way that conditional and conclusion are reversed here, I think it is easy to interpret this as bidirectional.

English is such a bad language for precise specifications...

Copy link
Member

Choose a reason for hiding this comment

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

So, to make a concrete proposal (also removing a pointless double-negation):

If all fields of a struct have size 0, then the struct has size 0.

Your sentence lists a few representations explicitly, but is there any repr for which this does not make sense?

Copy link

@comex comex Jul 26, 2019

Choose a reason for hiding this comment

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

Well, that statement is trivially true if there struct has no fields. C++ allows structs with no fields but gives them a size of 1, and in theory there could be some kind of #[repr(C++)] that accounts for this.

I'm not sure how plausible #[repr(C++)] is, but another hypothetical use case could be to fix the ABI issue with passing small C++ classes by value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but is there any repr for which this does not make sense?

If we ever add a repr() for which this does not make sense, then this wouldn't hold for repr(transparent) either.

The current wording mentions all reprs that currently exist, except for repr(transparent) , for which this just follows from its rules.

Copy link
Member

Choose a reason for hiding this comment

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

If you really want to list the repr, I'd propose prepending something like:

For repr(Rust), repr(packed(N)), repr(align(N)) and repr(C):

(Btw, why is it packed but not aligned? Seems inconsistent...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reworded the guarantee with this feedback. Let me know if its ok now.

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

This looks good to me (other than the nit), but I'd prefer if some other WG member also r+'d.

@hanna-kruppe
Copy link

r=me as well modulo resolving Ralf's nit (one way or another), this was completely uncontroversial (unlike e.g., enums) and is clearly useful.

@RalfJung
Copy link
Member

Thanks! This sounds a bit clumsy, which I realize is my fault. But I think at least the meaning is clear. We can always improve the wording later.

@RalfJung RalfJung merged commit ea3dbdf into rust-lang:master Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants