r179251 - [libclang] Expose record layout info via new libclang functions:

[Argyrios Kyrtzidis]

On Jun 21, 2013, at 5:07 PM, Matt Beaumont-Gay <matthewbg at google.com> wrote:

> On Fri, Jun 21, 2013 at 3:24 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> +struct Test2 {
>> +  struct {
>> +    struct {
>> +//CHECK64: FieldDecl=foo:[[@LINE+1]]:11 (Definition) [type=int]
>> [typekind=Int] [sizeof=4] [alignof=4] [offsetof=0]
>> +      int foo;
>> +    };
>> +    struct {
>> +//CHECK64: FieldDecl=bar:[[@LINE+1]]:11 (Definition) [type=int]
>> [typekind=Int] [sizeof=4] [alignof=4] [offsetof=32]
>> +      int bar;
>> +    };
>> +    struct {
>> +        struct {
>> +//CHECK64: FieldDecl=foobar:[[@LINE+1]]:15 (Definition) [type=int]
>> [typekind=Int] [sizeof=4] [alignof=4] [offsetof=64]
>> +          int foobar;
>> +        };
>> +    };
>> +    struct inner {
>> +        struct {
>> +//CHECK64: FieldDecl=mybar:[[@LINE+1]]:15 (Definition) [type=int]
>> [typekind=Int] [sizeof=4] [alignof=4] [offsetof=0]
>> +          int mybar;
>> +        };
>> +//CHECK64: FieldDecl=mole:[[@LINE+1]]:7 (Definition) [type=struct inner]
>> [typekind=Unexposed] [sizeof=4] [alignof=4] [offsetof=96]
>> +    } mole;
>> +  };
>> +};
>> 
>> 
>> This struct is invalid:
>> tools/clang/test/Index/print-type-size.cpp:77:12: error: types cannot
>> be declared in an anonymous struct
>>   struct inner {
>>          ^
>> 
>> I noticed because it trips an assert I added to
>> ASTContext::getASTRecordLayout (asserting that the decl is valid). How
>> deeply do you care about this particular part of the test case? Can I
>> remove it, or is there a particular fix I should apply?
>> 
>> I have a patch which adds some invalid-decl checks to
>> clang_Type_getOffsetOf, but it breaks all of the existing test cases
>> which explicitly exercise invalid structs (not to mention Test2
>> above). It's obviously important that we don't crash on those inputs,
>> but I'm not sure I see the value of checking for any particular
>> output.
>> 
>> Thoughts?
>> 
>> 
>> I suggest adding the invalid-decl checks and leaving an invalid struct in
>> the test to make sure libclang does not hit the assertion; the rest can be
>> turned into valid decls so that the checks can work.
> 
> Great. I've taken the proverbial hatchet to the CHECKs in that test
> and added a new little case that resembles the original crasher; patch
> attached, please review.

LGTM.

> 
> -Matt
> <c-index-record-layout.patch>