[ASTImporter] Fix import of unnamed structs

Summary:
D48773 simplified ASTImporter nicely, but it introduced a new error: Unnamed
structs are not imported correctly, if they appear in a recursive context.
This patch provides a fix for structural equivalency.

Reviewers: a.sidorin, a_sidorin, balazske, gerazo

Subscribers: rnkovacs, dkrupp, cfe-commits

Differential Revision: https://reviews.llvm.org/D49296

llvm-svn: 337267

3 files changed, 124 insertions, 8 deletions

diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 77e0d9e61bd..1da7b849487 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -924,7 +924,7 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
     return false;
   }
 
-  if (D1->isAnonymousStructOrUnion() && D2->isAnonymousStructOrUnion()) {
+  if (!D1->getDeclName() && !D2->getDeclName()) {
     // If both anonymous structs/unions are in a record context, make sure
     // they occur in the same location in the context records.
     if (Optional<unsigned> Index1 =
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index c59c2cd1253..84bfdadae2d 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -273,6 +273,11 @@ public:
   }
 };
 
+template <typename T> RecordDecl *getRecordDecl(T *D) {
+  auto *ET = cast<ElaboratedType>(D->getType().getTypePtr());
+  return cast<RecordType>(ET->getNamedType().getTypePtr())->getDecl();
+};
+
 // This class provides generic methods to write tests which can check internal
 // attributes of AST nodes like getPreviousDecl(), isVirtual(), etc. Also,
 // this fixture makes it possible to import from several "From" contexts.
@@ -1755,11 +1760,6 @@ TEST_P(ASTImporterTestBase, ObjectsWithUnnamedStructType) {
       )",
       Lang_CXX, "input0.cc");
 
-  auto getRecordDecl = [](VarDecl *VD) {
-    auto *ET = cast<ElaboratedType>(VD->getType().getTypePtr());
-    return cast<RecordType>(ET->getNamedType().getTypePtr())->getDecl();
-  };
-
   auto *Obj0 =
       FirstDeclMatcher<VarDecl>().match(FromTU, varDecl(hasName("object0")));
   auto *From0 = getRecordDecl(Obj0);
@@ -2580,6 +2580,38 @@ TEST_P(ASTImporterTestBase, ImportOfNonEquivalentMethod) {
   EXPECT_NE(ToM1, ToM2);
 }
 
+TEST_P(ASTImporterTestBase, ImportUnnamedStructsWithRecursingField) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      struct A {
+        struct {
+          struct A *next;
+        } entry0;
+        struct {
+          struct A *next;
+        } entry1;
+      };
+      )",
+      Lang_C, "input0.cc");
+  auto *From =
+      FirstDeclMatcher<RecordDecl>().match(FromTU, recordDecl(hasName("A")));
+
+  Import(From, Lang_C);
+
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  auto *Entry0 =
+      FirstDeclMatcher<FieldDecl>().match(ToTU, fieldDecl(hasName("entry0")));
+  auto *Entry1 =
+      FirstDeclMatcher<FieldDecl>().match(ToTU, fieldDecl(hasName("entry1")));
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+  EXPECT_NE(R0, R1);
+  EXPECT_TRUE(MatchVerifier<RecordDecl>().match(
+      R0, recordDecl(has(fieldDecl(hasName("next"))))));
+  EXPECT_TRUE(MatchVerifier<RecordDecl>().match(
+      R1, recordDecl(has(fieldDecl(hasName("next"))))));
+}
+
 struct DeclContextTest : ASTImporterTestBase {};
 
 TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
diff --git a/clang/unittests/AST/StructuralEquivalenceTest.cpp b/clang/unittests/AST/StructuralEquivalenceTest.cpp
index 551986fc1c9..19d2e334fc7 100644
--- a/clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ b/clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -42,6 +42,21 @@ struct StructuralEquivalenceTest : ::testing::Test {
     return std::make_tuple(D0, D1);
   }
 
+  std::tuple<TranslationUnitDecl *, TranslationUnitDecl *> makeTuDecls(
+      const std::string &SrcCode0, const std::string &SrcCode1, Language Lang) {
+    this->Code0 = SrcCode0;
+    this->Code1 = SrcCode1;
+    ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+    const char *const InputFileName = "input.cc";
+
+    AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
+    AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+
+    return std::make_tuple(AST0->getASTContext().getTranslationUnitDecl(),
+                           AST1->getASTContext().getTranslationUnitDecl());
+  }
+
   // Get a pair of node pointers into the synthesized AST from the given code
   // snippets. The same matcher is used for both snippets.
   template <typename NodeType, typename MatcherType>
@@ -62,7 +77,7 @@ struct StructuralEquivalenceTest : ::testing::Test {
     return makeDecls<NamedDecl>(SrcCode0, SrcCode1, Lang, Matcher);
   }
 
-  bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
+  bool testStructuralMatch(Decl *D0, Decl *D1) {
     llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
     StructuralEquivalenceContext Ctx(
         D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls,
@@ -70,7 +85,7 @@ struct StructuralEquivalenceTest : ::testing::Test {
     return Ctx.IsStructurallyEquivalent(D0, D1);
   }
 
-  bool testStructuralMatch(std::tuple<NamedDecl *, NamedDecl *> t) {
+  bool testStructuralMatch(std::tuple<Decl *, Decl *> t) {
     return testStructuralMatch(get<0>(t), get<1>(t));
   }
 };
@@ -468,6 +483,11 @@ TEST_F(StructuralEquivalenceCXXMethodTest, OutOfClass2) {
 }
 
 struct StructuralEquivalenceRecordTest : StructuralEquivalenceTest {
+  // FIXME Use a common getRecordDecl with ASTImporterTest.cpp!
+  RecordDecl *getRecordDecl(FieldDecl *FD) {
+    auto *ET = cast<ElaboratedType>(FD->getType().getTypePtr());
+    return cast<RecordType>(ET->getNamedType().getTypePtr())->getDecl();
+  };
 };
 
 TEST_F(StructuralEquivalenceRecordTest, Name) {
@@ -535,6 +555,70 @@ TEST_F(StructuralEquivalenceRecordTest, Match) {
   EXPECT_TRUE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceRecordTest, UnnamedRecordsShouldBeInequivalent) {
+  auto t = makeTuDecls(
+      R"(
+      struct A {
+        struct {
+          struct A *next;
+        } entry0;
+        struct {
+          struct A *next;
+        } entry1;
+      };
+      )",
+      "", Lang_C);
+  auto *TU = get<0>(t);
+  auto *Entry0 =
+      FirstDeclMatcher<FieldDecl>().match(TU, fieldDecl(hasName("entry0")));
+  auto *Entry1 =
+      FirstDeclMatcher<FieldDecl>().match(TU, fieldDecl(hasName("entry1")));
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+TEST_F(StructuralEquivalenceRecordTest,
+       UnnamedRecordsShouldBeInequivalentEvenIfTheSecondIsBeingDefined) {
+  auto Code =
+      R"(
+      struct A {
+        struct {
+          struct A *next;
+        } entry0;
+        struct {
+          struct A *next;
+        } entry1;
+      };
+      )";
+  auto t = makeTuDecls(Code, Code, Lang_C);
+
+  auto *FromTU = get<0>(t);
+  auto *Entry1 =
+      FirstDeclMatcher<FieldDecl>().match(FromTU, fieldDecl(hasName("entry1")));
+
+  auto *ToTU = get<1>(t);
+  auto *Entry0 =
+      FirstDeclMatcher<FieldDecl>().match(ToTU, fieldDecl(hasName("entry0")));
+  auto *A =
+      FirstDeclMatcher<RecordDecl>().match(ToTU, recordDecl(hasName("A")));
+  A->startDefinition(); // Set isBeingDefined, getDefinition() will return a
+                        // nullptr. This may be the case during ASTImport.
+
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+
 TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) {
   auto t = makeNamedDecls(
       "struct A{ }; struct B{ }; void foo(A a, A b);",