[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: test/Index/print-type-size.c:22
+
+// RUN: c-index-test -test-print-type-size %s | FileCheck %s
+// CHECK: FieldDecl=size:2:9 (Definition) [type=int] [typekind=Int] [sizeof=4] [alignof=4] [offsetof=0]
----------------
This should be the first line of the file. I don't have strong opinions on where the CHECK lines go, but I usually prefer seeing them near the construct being checked.


================
Comment at: test/Index/print-type-size.c:31
+// CHECK: FieldDecl=data2:18:15 (Definition) [type=void *[]] [typekind=IncompleteArray] [sizeof=-2] [alignof=8] [offsetof=64/0]
\ No newline at end of file

----------------
Please add a newline to the end of the file.


================
Comment at: tools/libclang/CXType.cpp:898
     QT = QT.getNonReferenceType();
-  if (QT->isIncompleteType())
+  if (QT->isIncompleteType() && !QT->isIncompleteArrayType()) // IAT is okay here
     return CXTypeLayoutError_Incomplete;
----------------
I don't think IAT is a common enough acronym to use; I'd probably just drop the comment as it adds little value.


================
Comment at: tools/libclang/CXType.cpp:956
     QualType FQT = I->getType();
-    if (FQT->isIncompleteType())
+    if (FQT->isIncompleteType() && !FQT->isIncompleteArrayType()) // IAT is okay here
       return CXTypeLayoutError_Incomplete;
----------------
Same here.


================
Comment at: tools/libclang/CXType.cpp:956
     QualType FQT = I->getType();
-    if (FQT->isIncompleteType())
+    if (FQT->isIncompleteType() && !FQT->isIncompleteArrayType()) // IAT is okay here
       return CXTypeLayoutError_Incomplete;
----------------
aaron.ballman wrote:
> Same here.
I sort of wonder whether we want a helper function like `isTypeIncompleteForLayout()` or something, and then using that helper directly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61239/new/

https://reviews.llvm.org/D61239