[PATCH] [libclang] getSizeOf/getAlignOf/getOffsetOf (was [PATCH] Expose AST Record layout attributes to libclang)

Loïc Jaquemet loic.jaquemet at gmail.com
Fri Mar 15 00:08:11 PDT 2013


Thanks for your time Dmitri. I'm sorry its taking so long for me to
get that right.


>> Should sizeOf return -1 when the type is a bitfield ? [expr.sizeof] p1]
>
> Yes.

Well, given the current signature of the function (CXType), we will
never be evaluating a field ( CXCursor). Only types.
So my guess, there is no specific case here after all.

>> +  // Exceptions by GCC extension - see ASTContext.cpp:1313 getTypeInfoImpl
>> +  // if (QT->isFunctionType()) return 4;
>>
>> I could not find the gcc extension reference document.
>
> http://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html
>
> But apparently it does not cover this extension.

Do I need to change the current situation for alignof? And to what ?
I expect the #15511 bug fix to fix it automatically.
Current output:
CXXMethod=foo:212:16 (Definition) (virtual) [type=void ()]
[typekind=FunctionProto] [sizeof=1] [alignof=4]


>> +  // [expr.sizeof] p1: return -1 on: func, incomplete, bitfield,
>> incomplete enumeration
>> +  // [expr.sizeof] p3: pointer ok, function not ok.
>> +  // [gcc extension] lib/AST/ExprConstant.cpp:1372 HandleSizeof : vla == error
>> +  if (QT->isIncompleteType() || QT->isDependentType() ||
>> !QT->isConstantSizeType())
>> +    return -2;
>>
>> I have to test for incompleteType and dependentType ( and constant
>> size type in sizeof) otherwise it asserts on templates.
>
> Could you use different error codes for these situations?  A dumb
> client can check return value >= 0, while a smart one can get more
> information.

Done.

>
> -#define CINDEX_VERSION_MINOR 12
> +#define CINDEX_VERSION_MINOR 13
>
> Please refresh your patches to apply to trunk (we are at version 15 now :)
>

Done


>  /**
> + * \brief Return the alignment of a type in bytes as per
> C++[expr.alignof] standard.
>
> 80 cols.
>
[.. and bunch of others ]

done

> + */
> +CINDEX_LINKAGE long long clang_getRecordFieldOffsetInBits(CXCursor C);
>
> It would help to add an enum with error codes.

Done

>
> But, there's other concern: what you need is to expose the record
> layout.  While a general sizeof() is useful, it will not work for you,
> exactly because of all these exceptions.  For example,
> sizeof(reference type) is sizeof(type), but you need to know the
> number of bytes the *reference itself* takes.

Yes. I though about that, and for what I want to my achieve, I feel
that  do not need an extra libclang function after all.
I will always query the type of the field, and have specific code for
reference and pointers anyway.
Other exceptions do not apply in the context.
So no need to clutter libclang with that.

I dropped the getRecordXXX functions in favor of sizeof/alignof/offsetof
I did put in two function for offsetof, because it can be queried both
with the Cursor and with the standard parent + fieldname combination.
And I do feel its an added value.


>
> +long long clang_getRecordFieldOffsetInBits(CXCursor C) {
> +  if (!clang_isDeclaration(C.kind))
> +    return -1;
> +  const FieldDecl *FD =
> dyn_cast_or_null<FieldDecl>(cxcursor::getCursorDecl(C));
> +  if (!FD)
> +    return -1;
> +  QualType QT = GetQualType(clang_getCursorType(C));
> +  if (QT->isIncompleteType() || QT->isDependentType() ||
> !QT->isConstantSizeType())
>
> I don't see a test for isConstantSizeType case.

Sadly I have no idea of an example for a non ConstantSizeType.
Any hint ?

>
> +static enum CXChildVisitResult PrintTypeSize(CXCursor cursor, CXCursor p,
> +                                         CXClientData d) {
>
> Please align 'CXClientData' to 'CXCursor'.

done

>
> +static enum CXChildVisitResult PrintTypeSize(CXCursor cursor, CXCursor p,
> +                                         CXClientData d) {
> +  if (!clang_isInvalid(clang_getCursorKind(cursor))) {
>
> Reverse the condition and de-nest:

done

Added tests for bitfields too.


>
> The tests could be reduced or reorganized to make them more targeted
> (to check a specific thing).  There's a lot of stuff that actually
> checks MS struct layout algorithms.

Cleaned up tests to be only composed of useful tests. Got rid of old
pasted tests.

>
> Python bindings look good.

added bitfields tests and offsetof

>
> Dmitri
>
> --
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/



-- 
Loïc Jaquemet
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sizeof-alignof-offsetof-001
Type: application/octet-stream
Size: 9747 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130315/478a24a2/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sizeof-alignof-offsetof-002-tests
Type: application/octet-stream
Size: 13395 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130315/478a24a2/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sizeof-alignof-offsetof-003-python-bindings
Type: application/octet-stream
Size: 2505 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130315/478a24a2/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sizeof-alignof-offsetof-004-python-bindings-tests
Type: application/octet-stream
Size: 2778 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130315/478a24a2/attachment-0003.obj>


More information about the cfe-commits mailing list