On Fri, 21 Jun 2019 at 19:38, Richard Smith <rich...@metafoo.co.uk> wrote: > > Thanks, should hopefully be fixed by r364081.
Problem fixed, Thanks richard. > On Fri, 21 Jun 2019 at 01:12, Yvan Roux via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
