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-comm...@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-comm...@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
