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>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
