On Wed, Oct 14, 2020 at 4:28 AM Lénárd Szolnoki via Std-Discussion
<std-discussion_at_[hidden]> wrote:
>
> On Tue, 13 Oct 2020 16:24:06 -0400
> Jason McKesson via Std-Discussion <std-discussion_at_[hidden]>
> wrote:
>
> > > Following the standard wording A1 and A2 are layout compatible [1].
> > > Similarly B1 and B2 are layout compatible. However the ch member in
> > > B1 has different offset than the ch member in B2.
> >
> > That is not legal. Layout compatible types cannot have offsets for
> > corresponding member subobjects that are different. That would break
> > the rules of accessing inactive members of a `union`.
>
> Yes, this is the problem. B1 and B2 should be layout compatible
> according to the C++20 standard, but the Itanium ABI is not compliant.
>
> > > In the Itanium ABI A1 is not POD for the purpose of layout while A2
> > > is, this affects the offset of the ch member.
> >
> > Can you explain why the Itanium ABI considers `A1` to not be a "POD
> > for the purpose of layout"? Is it because it has private members?
>
> Yes, although there are other things that affects "POD for the purpose
> of layout", (any user-declared constructor seems to opt out from this,
> even defaulted ones).
>
> > If so, I don't see how `no_unique_address` matters here; the problem
> > would still manifest if B1::a and B2::b2 were both not declared with
> > this qualifier.
>
> It wouldn't manifest, since then they wouldn't be
> potentially overlapping. "POD for the purpose of layout" only affects
> the offset of subsequent siblings of potentially overlapping
> subobjects, it doesn't affect the layout within the class itself.
>
> For a non-overlapping member the offset of its next sibling is a
> function of the size of the current member and the alignment of the
> next member. "POD for the purpose of layout" doesn't affect either.
>
> > > Two possible directions to fix this:
> > >
> > > * Modify the standard to disallow [[no_unique_address]] non-empty
> > > non-static data members to be part of a common initial sequence
> > > (or maybe only allow as the last element of a common initial
> > > sequence).
> > > * Modify the ABI to disable overlapping of [[no_unique_address]]
> > > non-empty data members in standard-layout classes.
> >
> > Again, I'm not sure how this resolves the issue. If there is
> > overlapping, it should apply equally to `B1::a` and `B2::b2`. Not
> > unless the ABI is doing something special because `A1` has private
> > members. And if it is, then *that* is what needs to change.
>
> The first suggestion solves the problem by making B1 and B2 not
> layout-compatible. The common initial sequence wouldn't contain 'ch',
> at most it would only contain the first data member of both classes.
>
> The second suggestion is fixing the ABI, basically treating
> [[no_unique_address]] non-empty data members specially in
> standard-layout classes and not making them actually overlapping. This
> would make the layout of B1 and B2 the same.
>
> Do we want to make B1 and B2 layout compatible? I don't know.

I see no reason why they *shouldn't* be. A1 and A2 are layout
compatible and have been since C++11 defined the concept. Two types
that are otherwise layout compatible that contain A1 and A2 as
corresponding members should also be layout compatible. And adding
[[no_unique_address]] statements to corresponding members should not
affect layout compatibility in any way. If they were layout compatible
without the attribute, they should be layout compatible with it.

The only reason to want to change the standard is that the Itanium ABI
treats standard layout types with private members differently from
equivalent types with public members. And [[no_unique_address]]
happened to cause this problem to manifest itself.

The language makes sense. Changing it as you suggest would make it
make less sense. This is a problem caused by the Itanium ABI; it
should be solved *by* the Itanium ABI. Whether they do it by treating
private members correctly in accord with the standard, or by doing
what you suggest, it's on them to implement the standard as it
currently is.