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

[Argyrios Kyrtzidis]

On Mar 25, 2013, at 5:24 PM, Loïc Jaquemet <loic.jaquemet at gmail.com> wrote:

>>> Does this handle anonymous structs/unions ? For example:
>>> 
>>> struct Test {
>>>  struct {
>>>    union {
>>>      int foo;
>>>    };
>>>  };
>>> };
>>> 
>>> If I pass "struct Test", "foo" will it work ? If yes, could you add a
>>> related test ?
>>> 
>> 
>> It does not work.
>> I will look into that.
> 
> And now it does handle anonymous records.
> I added some tests cases and changed the unit test code.

+  /**
+   * \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" ?


+/**
+ * \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 ..."


+static long long visitRecordForNamedField(const RecordDecl *RD,
+                                          StringRef FieldName) {
+  for (RecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end();
+       I != E; ++I) {
+    // handle normal fieldname, fieldname == '' == anonymous record, and
+    // field name in a anonymous record
+    if (FieldName.equals((*I)->getName())) {
+      return getOffsetOfFieldDecl((*I));
+    } else if ((*I)->isAnonymousStructOrUnion()) {
+      const RecordType *ChildType = (*I)->getType()->getAs<RecordType>();
+      if (const RecordDecl *Child = ChildType->getDecl()) {
+        long long Offset = visitRecordForNamedField(Child, FieldName);
+        if (Offset == CXTypeLayoutError_InvalidFieldName) {
+            continue;
+        } else if (Offset < 0) {
+            return Offset;
+        } else {
+            // result is relative to anonymous struct offset
+            long long ParentOffset = getOffsetOfFieldDecl((*I));
+            if (ParentOffset < 0)
+                return ParentOffset;
+            return Offset+ParentOffset;
+        }
+      } // else try next field
+    }
+  }
+  return CXTypeLayoutError_InvalidFieldName;
+}
+
+long long clang_Type_getOffsetOf(CXType PT, const char *S) {
+  // get the parent record type declaration
+  CXCursor PC = clang_getTypeDeclaration(PT);
+  if (clang_isInvalid(PC.kind))
+    return CXTypeLayoutError_Invalid;
+  const RecordDecl *RD =
+        dyn_cast_or_null<RecordDecl>(cxcursor::getCursorDecl(PC));
+  if (!RD)
+    return CXTypeLayoutError_Invalid;
+  RD = RD->getDefinition();
+  if (!RD)
+    return CXTypeLayoutError_IncompleteFieldParent;
+  QualType RT = GetQualType(PT);
+  if (RT->isIncompleteType())
+    return CXTypeLayoutError_IncompleteFieldParent;
+  if (RT->isDependentType())
+    return CXTypeLayoutError_DependentFieldParent;
+  // iterate the fields to get the matching name
+  StringRef FieldName = StringRef(S);
+  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;


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

> 
> Please see attached diffs.
> 
> * 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>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130401/4fb200dc/attachment.html>