Yes, it fixes both of those test cases. -Eli
On Mon, Jul 15, 2013 at 5:49 PM, Richard Smith <[email protected]> wrote: > It looks like this fixes PR14564. Does it also fix the case in PR14564 > comment#1? > > > On Mon, Jul 15, 2013 at 5:21 PM, Eli Friedman <[email protected]> > wrote: >> >> Author: efriedma >> Date: Mon Jul 15 19:21:28 2013 >> New Revision: 186370 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=186370&view=rev >> Log: >> Fix alignment of class derived from empty class. >> >> The record layout code didn't properly take into account that >> an empty class at offset 0 can have an alignment greater than 1. >> >> Patch by Andrea Di Biagio. >> >> Added: >> cfe/trunk/test/SemaCXX/alignment-of-derived-class.cpp >> Modified: >> cfe/trunk/lib/AST/RecordLayoutBuilder.cpp >> >> Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=186370&r1=186369&r2=186370&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original) >> +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Mon Jul 15 19:21:28 2013 >> @@ -1532,18 +1532,19 @@ CharUnits RecordLayoutBuilder::LayoutBas >> } >> } >> >> + CharUnits UnpackedBaseAlign = Layout.getNonVirtualAlign(); >> + CharUnits BaseAlign = (Packed) ? CharUnits::One() : UnpackedBaseAlign; >> + >> // If we have an empty base class, try to place it at offset 0. >> if (Base->Class->isEmpty() && >> (!HasExternalLayout || Offset == CharUnits::Zero()) && >> EmptySubobjects->CanPlaceBaseAtOffset(Base, CharUnits::Zero())) { >> setSize(std::max(getSize(), Layout.getSize())); >> + UpdateAlignment(BaseAlign, UnpackedBaseAlign); >> >> return CharUnits::Zero(); >> } >> >> - CharUnits UnpackedBaseAlign = Layout.getNonVirtualAlign(); >> - CharUnits BaseAlign = (Packed) ? CharUnits::One() : UnpackedBaseAlign; >> - >> // The maximum field alignment overrides base align. >> if (!MaxFieldAlignment.isZero()) { >> BaseAlign = std::min(BaseAlign, MaxFieldAlignment); >> >> Added: cfe/trunk/test/SemaCXX/alignment-of-derived-class.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/alignment-of-derived-class.cpp?rev=186370&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/alignment-of-derived-class.cpp (added) >> +++ cfe/trunk/test/SemaCXX/alignment-of-derived-class.cpp Mon Jul 15 >> 19:21:28 2013 >> @@ -0,0 +1,41 @@ >> +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 >> +// expected-no-diagnostics >> + >> +// Test that the alignment of a empty direct base class is correctly >> +// inherited by the derived class. >> + >> +struct A { >> +} __attribute__ ((aligned(16))); >> + >> +static_assert(__alignof(A) == 16, "A should be aligned to 16 bytes"); >> + >> +struct B1 : public A { >> +}; >> + >> +static_assert(__alignof(B1) == 16, "B1 should be aligned to 16 bytes"); >> + >> +struct B2 : public A { >> +} __attribute__ ((aligned(2))); >> + >> +static_assert(__alignof(B2) == 16, "B2 should be aligned to 16 bytes"); >> + >> +struct B3 : public A { >> +} __attribute__ ((aligned(4))); >> + >> +static_assert(__alignof(B3) == 16, "B3 should be aligned to 16 bytes"); >> + >> +struct B4 : public A { >> +} __attribute__ ((aligned(8))); >> + >> +static_assert(__alignof(B4) == 16, "B4 should be aligned to 16 bytes"); >> + >> +struct B5 : public A { >> +} __attribute__ ((aligned(16))); >> + >> +static_assert(__alignof(B5) == 16, "B5 should be aligned to 16 bytes"); >> + >> +struct B6 : public A { >> +} __attribute__ ((aligned(32))); >> + >> +static_assert(__alignof(B6) == 32, "B6 should be aligned to 32 bytes"); >> + >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
