[PATCH] D22270: [ASTImporter] Properly report the locations of anonymous structs declared as part of named fields

[Manman Ren via cfe-commits]

manmanren added a comment.

I am not sure if we should handle this inside findAnonymousStructOrUnionIndex. Here is the definition of anonymous structure I found: An unnamed member whose type specifier is a structure specifier with no tag is called an anonymous structure.

Cheers,
Manman


================
Comment at: lib/AST/ASTImporter.cpp:1054
@@ +1053,3 @@
+      QualType FieldType = F->getType();
+      if (const RecordType *RecType = dyn_cast<RecordType>(FieldType)) {
+        const RecordDecl *RecDecl = RecType->getDecl();
----------------
Use const auto * here?

================
Comment at: lib/AST/ASTImporter.cpp:1058
@@ +1057,3 @@
+            !RecDecl->getIdentifier()) {
+          if (Context.hasSameType(FieldType, AnonTy)) {
+            break;
----------------
Combining the two ifs?

================
Comment at: lib/AST/ASTImporter.cpp:1062
@@ +1061,3 @@
+        }
+      }
+    }
----------------
Should we continue here instead of increasing the Index?

Maybe we can reorder here to reduce nesting?
if (F->isAnonymousStructOrUnion() && ...)
  break;
if (F->isAnonymousStructOrUnion())
  Index++;
  continue;
// If the field looks like this: ...


================
Comment at: test/ASTMerge/anonymous-fields.cpp:3
@@ +2,3 @@
+// RUN: %clang_cc1 -emit-pch -o %t.2.ast %S/Inputs/anonymous-fields2.cpp
+// RUN: %clang_cc1 -emit-obj -o /dev/null -ast-merge %t.1.ast -ast-merge %t.2.ast %s
+// expected-no-diagnostics
----------------
Does this test fail without the change in ASTImporter.cpp? Should we check the output of the AST merging?


Repository:
  rL LLVM

http://reviews.llvm.org/D22270