[PATCH] Expose AST Record layout attributes to libclang

[Dmitri Gribenko]

On Wed, Mar 6, 2013 at 6:07 AM, Loïc Jaquemet <loic.jaquemet at gmail.com> wrote:
> Hi,
>
> So here are new patches.
>
> Should sizeOf return -1 when the type is a bitfield ? [expr.sizeof] p1]

Yes.

> 2013/2/13 Dmitri Gribenko <gribozavr at gmail.com>:
>>>> * 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.
>>>>
>
> +  // 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.

> ASTContext says sizeof(function) is 32 bits

sizeof() is 1, both for clang and gcc.

But for alignof(), we return different results...  Need to file a bug
about that.  http://llvm.org/bugs/show_bug.cgi?id=15511

> +  // [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.

-#define CINDEX_VERSION_MINOR 12
+#define CINDEX_VERSION_MINOR 13

Please refresh your patches to apply to trunk (we are at version 15 now :)

 /**
+ * \brief Return the alignment of a type in bytes as per
C++[expr.alignof] standard.

80 cols.

+ *
+ * If the type declaration is invalid, -1 is returned.
+ * If the type declaration is incomplete or a dependent type, -2 is returned.

... an incomplete...

+ */
+CINDEX_LINKAGE long long clang_getTypeAlignOf(CXType T);
+
+/**
+ * \brief Return the size of a type in bytes as per C++[expr.sizeof] standard.
+ *
+ * If the type declaration is invalid, -1 is returned.
+ * If the type declaration is incomplete, a dependent type or non
constant size, -2 is returned.

... an incomplete...

+ */
+CINDEX_LINKAGE long long clang_getTypeSizeOf(CXType T);
+
+/**
+ * \brief Return the offset of a field in a record in bits as
returned by the AST Record Layout.
+ *
+ * If the cursor is not a record field declaration, -1 is returned.
+ * If the field type declaration is incomplete, a dependent type or
non constant size, -2 is returned.

... an incomplete...

80 cols.

+ */
+CINDEX_LINKAGE long long clang_getRecordFieldOffsetInBits(CXCursor C);

It would help to add an enum with error codes.

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.

+long long clang_getRecordFieldOffset(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+    return -1;
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const FieldDecl *FD = dyn_cast<FieldDecl>(D);
+  if (!FD)
+    return -1;
+
+  ASTContext &Ctx = cxcursor::getCursorContext(C);
+  unsigned FieldNo = FD->getFieldIndex();
+  const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(FD->getParent());
+  return Layout.getFieldOffset(FieldNo);
+}

I think that this routine (from one of your previous patches) will do
what you need.  Just please add tests that check:
* it does not break badly in presence of dependent types, etc.
* it returns the correct size for references (the size of reference,
not the size of the referred type).

About clang_getRecordAlignment, I think that clang_getTypeAlignOf will
return the same value, won't it?

+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.

+static enum CXChildVisitResult PrintTypeSize(CXCursor cursor, CXCursor p,
+                                         CXClientData d) {

Please align 'CXClientData' to 'CXCursor'.

+static enum CXChildVisitResult PrintTypeSize(CXCursor cursor, CXCursor p,
+                                         CXClientData d) {
+  if (!clang_isInvalid(clang_getCursorKind(cursor))) {

Reverse the condition and de-nest:

  if (clang_isInvalid(clang_getCursorKind(cursor)))
    return ...;
  ...

+union u {
+  int u1;
+  long long u2;
+  struct simple s1;
+};

Why no CHECKs here?

+class A;
+class B {
+  A* a1;
+  A& a2;
+};
+
+class C;
+class D {
+  C c1;
+};
+class C {
+  int c;
+};

And here?

+struct simple {
+  int a;
+  char b;
+  int c:3;
+  long d;
+  int e:5;
+  int f:4;

And here?

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.

Python bindings look good.

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