[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute

[Yvan Roux via llvm-commits]

Hi Richard,

This commit broke ARM bots, logs are available here:

http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/13576/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Atail-padding.cpp

Thanks,
Yvan

On Thu, 20 Jun 2019 at 23:06, John McCall via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> rjmccall added a comment.
>
> In D63451#1552609 <https://reviews.llvm.org/D63451#1552609>, @rsmith wrote:
>
> > In D63451#1549563 <https://reviews.llvm.org/D63451#1549563>, @rjmccall wrote:
> >
> > > Can this attribute not be applied to a base class, or to a type?
> >
> >
> > The standard attribute forbids that right now. We could add a custom attribute that permits it, but we're required to diagnose application of the standard attribute to a type -- though a warning would suffice to satisfy that requirement. (Because this gives "same as a base class" layout, adding it to a base class wouldn't have any effect right now, but we could certainly say that the attribute on a class type also implies the class is not POD for the purpose of layout, to permit tail padding reuse.)
>
>
> I think we're talking about slightly different things.
>
> You're trying to make sure that fields can take advantage of the layout optimizations available to base classes so that library code doesn't need to contort to e.g. use the empty-base-class optimization just to avoid wasting an entire pointer to store an empty allocator.  This attribute seems well-targeted for that.
>
> Totally separately from that — but perhaps better-related to the name of the attribute — I know that some projects have, in the past, had surprising problems with the rule that class layout can't place empty base classes at the same offset as other objects of the same type.  For example, IIRC WebKit used to have a `Noncopyable` class that deleted its copy constructor and copy-assignment operators, and there was an idiom to inherit from this in order to disable copying, and at one point they discovered that some fairly important class was quite a bit larger than expected because of this rule — I think they were also using the base class redundantly throughout the hierarchy because they didn't expect there to be any harm to doing so.  They would've no doubt appreciated the ability to just put the attribute on `Noncopyable`.  But of course they fixed that years ago, so I can't say that it's a serious issue for them now.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D63451/new/
>
> https://reviews.llvm.org/D63451
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits