[PATCH] Expose AST Record layout attributes to libclang

Loïc Jaquemet loic.jaquemet at gmail.com
Mon Feb 11 18:44:44 PST 2013


> This looks better now, but still not there yet.
>
> + * \brief Return the alignment value for a record, in bytes.
>
> "Return the alignment of a record (struct or class), in bytes" ?
>
> + * \brief Return the offset of a field in a record in bits.
>
> What does this return for bitfields?  Is it possible that return value
> is not a multiple of 8?
>
> [..]

I fixed whitespace,
Added bitfield tests - yes the return value can be anything.
Fixed trailing whitespace ( is there a code checker in the lvm tool suite ? )
Reorderer functions.

>
> Important stuff: the actual checking is missing.  Since this is going
> to be a target-specific test, it is better to put it into a separate
> file rather than to convert an existing target-independent test.  In
> order to make the test pass on all hosts, just explicitly specify the
> triple.  (Grep for 'triple' in tests/ for examples that run clang that
> way.)
>

ok

> More comments: please add a test with bitfields, and tests with C++
> records (don't forget about virtual functions, inheritance and virtual
> inheritance).  Since the scope for these functions was not limited in
> the documentation, we assume they should work for all cases.  It is
> perfectly reasonable to limit ourselves only to POD, but we need to
> document that (& rename functions to include 'POD').
>

I will try c++ records and assess difficulty.


>      assert all(x.translation_unit is not None for x in fields)
> +····
> +    align = teststruct.type.get_record_alignment()
>
> Trailing whitespace.
>
> assert fields[2].get_record_field_offset() == 8*(2*ctypes.sizeof(ctypes.c_int))
>
> Sorry, but this (and other lines) is wrong (there might be padding).
> The proposed functions are being added for precisely that reason -- we
> can not compute member offsets *simply by adding sizes of members*.
> This test should specify a triple, too, and check for hardcoded
> values.

So, should I drop this offset test case ?
or is there a expected behaviour list for each triple somewhere ?

test/CodeGen*/*layout* is not really helpful.
./Sema/arm-layout.c and align-*.c has some related things that I am
not quite sure to understand.

I do not understand the 'Hardcoded values' part.

>
>> Yet I have no idea how to correctly implement the test of a record
>> size, given it is dependent of the tester's arch.
>> is there some lit.py magic for that ?
>
> You should to specify a '-triple' to c-index-test.  See above.
>

So I never managed to get -triple working.
My understanding is that its use is reserved to cc1, which is not
leveraged in this context.
But -m64 or -m32 works .

example :
# ok
c-index-test -test-print-typekind local record-layout.c -m32

# not ok
c-index-test -test-print-typekind local record-layout.c -triple
"i386-pc-linux-gnu"
warning: argument unused during compilation: '-triple
i386-pc-linux-gnu' [-Wunused-command-line-argument]


I'm sorry to ask (lack of knowledge on my part) but will this be
enough or does the packing/sizes depends on the full triple ?

// RUN: c-index-test -test-print-typekind %s -m64 | FileCheck
-check-prefix=CHECK64 %s
// RUN: c-index-test -test-print-typekind %s -m32 | FileCheck
-check-prefix=CHECK32 %s

Or is there something else on the clang command line I could use ?


> 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




More information about the cfe-commits mailing list