r186370 - Fix alignment of class derived from empty class.

[Eli Friedman]

Yes, it fixes both of those test cases.

-Eli

On Mon, Jul 15, 2013 at 5:49 PM, Richard Smith <richard at metafoo.co.uk> 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 <eli.friedman at gmail.com>
> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>