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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to