[PATCH] Expose AST Record layout attributes to libclang

[Dmitri Gribenko]

On Tue, Feb 12, 2013 at 7:51 AM, Loïc Jaquemet <loic.jaquemet at gmail.com> wrote:
> Hello,
>
> This patch is 3 functions in libclang to get more Record Layout
> information from the AST when using libclang.
>
> I am trying to implement the python ctypeslib record generator by
> replacing gccxml with libclang.
>
> Files:
> 001:
>  include/clang-c/Index.h
>  tools/libclang/libclang.exports
>  tools/libclang/CXType.cpp
> 002:
>  test/Index/record-layout.cpp
>  test/Index/record-layout.c
>  test/Index/record-layout-virtual.cpp
>  tools/c-index-test/c-index-test.c
> 003:
>  bindings/python/clang/cindex.py
> 004:
>  bindings/python/tests/cindex/util.py
>  bindings/python/tests/cindex/test_type.py
>
>
> Here is the latest version following your comments.
> Given that I'm pretty sure the -m32/-m64 flag is not sufficient. lets
> say this is a draft for you to comment on.

After thinking a little more about it, I have a general question about
the approach here: wouldn't it be better to implement
clang_getTypeSize/clang_getTypeAlignment that work for all types?
These functions should work just like sizeof/alignof.  This should be
easy enough to implement: ASTContext::getTypeSize()/getTypeAlign() + a
few special cases, which are:

* sizeof(void), __alignof__(void), sizeof(function) = 1 as a gcc
extension (lib/AST/ExprConstant.cpp:1372)

* C++ [expr.sizeof]p2: "When applied to a reference or a reference
type, the result is the size of the referenced type."
(lib/AST/ExprConstant.cpp:5228).

* incomplete types -- just return an error for these.

Sorry for not thinking of this earlier!

The c-index-test look are very extensive, great!

+#define SA(n, p) int a##n[(p) ? 1 : -1]

I think you can drop this, it is not related to what we are testing here.

+// from Sema/ms_class_layout.cpp
+//
+#pragma pack(push, 8)

That #pragma was specific to some MS features, it is not needed here.

+// This needs only for building layouts.
+// Without this clang doesn`t dump record layouts.
+int main() {
+  // This avoid "Can't yet mangle constructors!" for MS ABI.

main() is not needed either.

I think you can concatenate test/Index/record-layout.cpp and
test/Index/record-layout-virtual.cpp.  If there are name clashes, you
can put related classes into namespaces.  Or you can just drop
test/Index/record-layout-virtual.cpp and just add one or two cases
with virtual functions and virtual inheritance to the first file.

+// CHECK64: StructDecl=A:3:8 (Definition) typekind=Record [size=8]
[alignment=4] [isPOD=1]

Hardcoding line numbers makes the test fragile (adding a line forces
us to change every CHECK line).  We have [[@LINE]] expressions to
reference line numbers relative to the current location.  For example:

+struct A {
+// CHECK64: StructDecl=A:[[@LINE-1]]:8 (Definition) typekind=Record
[size=8] [alignment=4] [isPOD=1]

This also has a nice property of putting the CHECK near the source it
is related to.

> PS:
> I get a assertion error if record-layout-virtual.cpp is renamed as
> record-layout-virtual.c.
> c-index-test: /home/jal/compil/llvm/llvm/tools/clang/lib/AST/RecordLayoutBuilder.cpp:2470:
> const clang::ASTRecordLayout&
> clang::ASTContext::getASTRecordLayout(const clang::RecordDecl*) const:
> Assertion `D && "Cannot get layout of forward declarations!"' failed.
>
> I am not sure that error ( provoked by c++ in a .c file ) is actually
> part of my code or not.
> I have no knowledge of forward declaration or solution to that problem.

Right, one can not ask the size of an incomplete type.  We need to
check for these, and we should probably have a separate error code for
incomplete types.

QualType QT = GetQualType(T);
if (QT->isIncompleteType()) ...

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>*/