Thanks, committed with testcase in r184883.

On Tue, Jun 25, 2013 at 2:44 PM, Richard Smith <[email protected]> wrote:
> On Tue, Jun 25, 2013 at 2:24 PM, Matt Beaumont-Gay <[email protected]> 
> wrote:
>> While investigating PR16292, I added "assert(!D->isInvalidDecl())" to
>> ASTContext::getASTRecordLayout. The problem I fixed in r184751 was one
>> of the two paths which tickled that assertion via existing tests; this
>> is the other. (The actual testcase which triggers the assertion is
>> SemaCXX/alignof.cpp.) This patch changes ASTContext::getDeclAlign to
>> skip the actual field alignment calculation (and return 1-byte
>> alignment in the general case) if the field's parent record is
>> invalid. The patch also adds the assertion in getASTRecordLayout,
>> though I can commit that separately if anybody's fussed.
>>
>> The careful reader will note that I took the opportunity to rename
>> local variables in the block which I touched to conform to proper LLVM
>> style, since getDeclAlign currently has a mix of styles and it made me
>> twitch.
>
> Change LGTM, but please also add a test which would fail even without
> the added assertion. Something like:
>
> struct S; struct T { S s; int k; } t; int n = __alignof__(T::k);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to