Committed in r179251, thanks! On Apr 9, 2013, at 4:08 PM, Loïc Jaquemet <[email protected]> wrote:
> Damn. > Ok, I double checked this time. > I must have manually transferred the old expose-* patches from my build > machine instead of the more recent sizeof* last time. > > so this is the patches, as tested on top of r179132. > > > > 2013/4/9 Argyrios Kyrtzidis <[email protected]> > Are these right ? test/Index/print-type-size.cpp still fails on top of > r179099. > > On Apr 5, 2013, at 2:20 PM, Loïc Jaquemet <[email protected]> wrote: > >> Outch. >> Sorry for that. >> >> >> 2013/4/5 Argyrios Kyrtzidis <[email protected]> >> On Apr 4, 2013, at 6:33 PM, Loïc Jaquemet <[email protected]> wrote: >> >>> ah yes. >>> The lookup failure. I forgot that one instance. >>> >>> Attached patches pass tests on r178827. >> >> I think you meant to attach different files, these look like earlier >> versions of your patches (e.g. clang_getAlignOf, instead of >> clang_Type_getAlignOf) >> >>> >>> >>> 2013/4/4 Argyrios Kyrtzidis <[email protected]> >>> >>> On Apr 4, 2013, at 12:04 AM, Loïc Jaquemet <[email protected]> wrote: >>> >>>> >>>> >>>> >>>> 2013/4/1 Argyrios Kyrtzidis <[email protected]> >>>> >>>> + /** >>>> + * \brief One field in the record is an incomplete Type. >>>> + */ >>>> + CXTypeLayoutError_IncompleteFieldParent = -6, >>>> + /** >>>> + * \brief One field in the record is a dependent Type. >>>> + */ >>>> + CXTypeLayoutError_DependentFieldParent = -7 >>>> +}; >>>> >>>> This was a bit confusing until I read >>>> >>>> + * If in the record there is another field's type declaration that is >>>> + * an incomplete type, CXTypeLayoutError_IncompleteFieldParent is >>>> returned. >>>> + * If in the record there is another field's type declaration that is >>>> + * a dependent type, CXTypeLayoutError_DependentFieldParent is returned. >>>> + */ >>>> >>>> Could we change it to a simpler, "the parent record is >>>> incomplete/dependent" ? >>>> >>>> >>>> Given the radical code change, these confusing errors do not exists any >>>> more. >>>> >>>> >>>> >>>> +/** >>>> + * \brief Returns 1 if the cursor specifies a Record member that is a >>>> bitfield. >>>> + */ >>>> +CINDEX_LINKAGE unsigned clang_Cursor_isBitField(CXCursor C); >>>> >>>> the convention that we use is "Returns non-zero if ..." >>>> >>>> >>>> done >>>> >>>> >>>> >>>> +static long long visitRecordForNamedField(const RecordDecl *RD, >>>> + StringRef FieldName) { >>>> + for (RecordDecl::field_iterator I = RD->field_begin(), E = >>>> RD->field_end(); >>>> + I != E; ++I) { >>>> [..] >>>> + return visitRecordForNamedField(RD, FieldName); >>>> +} >>>> >>>> I think there is a simpler and more efficient way to handle fields in >>>> anonymous records, something like this: >>>> Inside clang_Type_getOffsetOf(): >>>> >>>> CXTranslationUnit TU = >>>> static_cast<CXTranslationUnit>(const_cast<void*>(PT.data[1])); >>>> ASTContext &Ctx = cxtu::getASTUnit(TU)->getASTContext(); >>>> IdentifierInfo *II = &Ctx.Idents.get(S); >>>> DeclarationName FieldName(II); >>>> RecordDecl::lookup_const_result Res = RD->lookup(FieldName); >>>> if (Res.size() != 1) >>>> return CXTypeLayoutError_InvalidFieldName; >>>> if (const FieldDecl *FD = dyn_cast<FieldDecl>(Res.front())) >>>> return getOffsetOfFieldDecl(FD); >>>> if (const IndirectFieldDecl *IFD = >>>> dyn_cast<IndirectFieldDecl>(Res.front())) >>>> return Ctx.getFieldOffset(IFD); // Change getOffsetOfFieldDecl() to >>>> accept IFD. >>>> >>>> return CXTypeLayoutError_InvalidFieldName; >>>> >>>> >>>> Thanks! That was exactly was I was looking for. >>>> >>>> >>>> In the process of implementing that new code, I stumble on some new crash >>>> tests cases. >>>> The RecordLayoutBuilder forces me to do a full validation of all records >>>> fields in a record. >>>> I have implemented a recursive validation function to do that. >>>> At the end, it does simplify the testing quite a lot. >>>> I do have to forget about the two previously confusing error types, as >>>> they would not be distinguishable. >>>> >>>> So, basically, this code is now simpler and more robust. >>>> I added some tests cases in the Incomplete namespace to demonstrate the >>>> several issues I uncovered. >>>> >>>> >>>> >>>>> >>>>> I also removed the duplicate clang_Cursor_getOffsetOf(). >>>>> After consideration, it did not make sense, especially in the >>>>> anonymous record situation. >>>> >>>> Not sure about this, clang_Cursor_getOffsetOf is arguable more useful than >>>> clang_Type_getOffsetOf. >>>> Let's say you have this use-case: "visit all fields in a record and get >>>> their offsets". To do this (as your changes in c-index-test show) you need >>>> to use this roundabout way where, you have the field, then you get its >>>> name, and pass it to clang_Type_getOffsetOf which looks for the same field. >>>> Can't clang_Cursor_getOffsetOf just work, for example if you have a cursor >>>> for "foo" in >>>> >>>> struct S { >>>> struct { >>>> int foo; >>>> }; >>>> }; >>>> >>>> it should just return the offset of "foo" inside "struct S". >>>> >>>> That was also my feeling at the beginning. >>>> But after several iteration on my own code, I see that my own use of this >>>> function is always in a context were I do have the record's type and the >>>> field's name at hand. >>>> On top of that, the C++ standard calls for a Type signature. >>>> So I will keep it to that. >>>> >>>> >>>> Please see attached diffs. >>> >>> test/Index/print-type-size.cpp failed when I applied the diffs on top of >>> r178800, could you take a look ? >>> >>>> >>>> * Implementation of sizeof, alignof and offsetof for libclang. >>>> * Unit Tests >>>> * Python bindings >>>> * Python tests >>>> >>>> -- >>>> Loïc Jaquemet >>>> <sizeof-alignof-offsetof-001><sizeof-alignof-offsetof-002-tests><sizeof-alignof-offsetof-003-python-bindings><sizeof-alignof-offsetof-004-python-bindings-tests> >>> >>> >>> >>> >>> -- >>> Loïc Jaquemet >>> <expose-ast-record-layout-001><expose-ast-record-layout-002-tests><expose-ast-record-layout-003-python-bindings><expose-ast-record-layout-004-python-bindings-tests> >> >> >> >> >> -- >> Loïc Jaquemet >> <sizeof-alignof-offsetof-001><sizeof-alignof-offsetof-002-tests><sizeof-alignof-offsetof-003-python-bindings><sizeof-alignof-offsetof-004-python-bindings-tests> > > > > > -- > Loïc Jaquemet > <sizeof-alignof-offsetof-001><sizeof-alignof-offsetof-002-tests><sizeof-alignof-offsetof-003-python-bindings><sizeof-alignof-offsetof-004-python-bindings-tests>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
