shafik created this revision.
shafik added reviewers: martong, teemperor, balazske, aaron.ballman.
Herald added a subscriber: rnkovacs.

When importing classes we may add a CXXMethodDecl more than once to a 
CXXRecordDecl. This patch will fix the cases we currently know about and handle 
both the case where we are only dealing with declarations and when we have to 
merge a definition into an existing declaration.


https://reviews.llvm.org/D56936

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2233,6 +2233,174 @@
             }).match(ToTU, functionDecl()));
 }
 
+TEST_P(ImportFunctions, ImportOverriddenMethodTwice) {
+  auto Code =
+      R"(
+      struct B { virtual void f(); };
+      struct D:B { void f(); };
+      )";
+  auto BP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  auto DP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+
+  Decl *FromTU0 = getTuDecl(Code, Lang_CXX);
+  auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DP);
+  Import(D, Lang_CXX);
+
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *B = FirstDeclMatcher<CXXMethodDecl>().match(FromTU1, BP);
+  Import(B, Lang_CXX);
+
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DP), 1u);
+}
+
+TEST_P(ImportFunctions, ImportOverriddenMethodTwiceDefinitionFirst) {
+  auto CodeWithOutDef =
+      R"(
+      struct B { virtual void f(); };
+      struct D:B { void f(); };
+      )";
+  auto CodeWithDef =
+      R"(
+    struct B { virtual void f(){}; };
+    struct D:B { void f(){}; };
+  )";
+  auto BP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  auto DP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+  auto BFDef = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto DFDef = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto FDefAll = cxxMethodDecl(hasName("f"), isDefinition());
+
+  Decl *ImportedD;
+  {
+    Decl *FromTU = getTuDecl(CodeWithDef, Lang_CXX, "input0.cc");
+    auto *FromD = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, DP);
+    ImportedD = Import(FromD, Lang_CXX);
+  }
+  {
+    Decl *FromTU = getTuDecl(CodeWithOutDef, Lang_CXX, "input1.cc");
+    auto *FromB = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, BP);
+    Import(FromB, Lang_CXX);
+  }
+
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDef), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFDef), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, FDefAll), 2u);
+}
+
+TEST_P(ImportFunctions, ImportOverriddenMethodTwiceOutOfClassDef) {
+  auto Code =
+      R"(
+      struct B { virtual void f(); };
+      struct D:B { void f(); };
+      void B::f(){};
+      )";
+
+  auto BFP =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  auto BFPIsDef = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto DFP =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+
+  Decl *FromTU0 = getTuDecl(Code, Lang_CXX);
+  auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
+  Import(D, Lang_CXX);
+
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *B = FirstDeclMatcher<CXXMethodDecl>().match(FromTU1, BFP);
+  Import(B, Lang_CXX);
+
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFPIsDef), 0u);
+
+  auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
+      ToTU, cxxRecordDecl(hasName("B")));
+  auto *ToBFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, BFP);
+  auto *ToBFOutOfClass = FirstDeclMatcher<CXXMethodDecl>().match(
+      ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
+
+  // The definition should be out-of-class.
+  EXPECT_NE(ToBFInClass, ToBFOutOfClass);
+  EXPECT_NE(ToBFInClass->getLexicalDeclContext(),
+            ToBFOutOfClass->getLexicalDeclContext());
+  EXPECT_EQ(ToBFOutOfClass->getDeclContext(), ToB);
+  EXPECT_EQ(ToBFOutOfClass->getLexicalDeclContext(), ToTU);
+
+  // Check that the redecl chain is intact.
+  EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
+}
+
+TEST_P(ImportFunctions, ImportOverriddenMethodTwiceOutOfClassDefTwoTUs) {
+  auto CodeTU0 =
+      R"(
+      struct B { virtual void f(); };
+      struct D:B { void f(); };
+      )";
+  auto CodeTU1 =
+      R"(
+      struct B { virtual void f(); };
+      struct D:B { void f(); };
+      void B::f(){}
+      void D::f(){}
+      void foo(B &b) { b.f(); }
+      )";
+
+  auto BFP =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  auto BFPIsDef = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto DFP =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+  auto DFPIsDef = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto FooDef = functionDecl(hasName("foo"));
+
+  Decl *ImportedD;
+  {
+    Decl *FromTU0 = getTuDecl(CodeTU0, Lang_CXX, "input0.cc");
+    auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
+    ImportedD = Import(D, Lang_CXX);
+  }
+
+  {
+    Decl *FromTU1 = getTuDecl(CodeTU1, Lang_CXX, "input1.cc");
+    auto *Foo = FirstDeclMatcher<FunctionDecl>().match(FromTU1, FooDef);
+    Import(Foo, Lang_CXX);
+  }
+
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFPIsDef), 0u);
+
+  auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
+      ToTU, cxxRecordDecl(hasName("B")));
+  auto *ToBFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, BFP);
+  auto *ToBFOutOfClass = FirstDeclMatcher<CXXMethodDecl>().match(
+      ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
+
+  // The definition should be out-of-class.
+  EXPECT_NE(ToBFInClass, ToBFOutOfClass);
+  EXPECT_NE(ToBFInClass->getLexicalDeclContext(),
+            ToBFOutOfClass->getLexicalDeclContext());
+  EXPECT_EQ(ToBFOutOfClass->getDeclContext(), ToB);
+  EXPECT_EQ(ToBFOutOfClass->getLexicalDeclContext(), ToTU);
+
+  // Check that the redecl chain is intact.
+  EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2968,7 +2968,7 @@
   if (ToD)
     return ToD;
 
-  const FunctionDecl *FoundByLookup = nullptr;
+  FunctionDecl *FoundByLookup = nullptr;
   FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
 
   // If this is a function template specialization, then try to find the same
@@ -3039,6 +3039,27 @@
     }
   }
 
+  if (FoundByLookup) {
+    if (auto *MD = dyn_cast<CXXMethodDecl>(FoundByLookup)) {
+      if (D->getLexicalDeclContext() == D->getDeclContext()) {
+        if (!D->doesThisDeclarationHaveABody())
+          return cast<Decl>(const_cast<FunctionDecl *>(FoundByLookup));
+        else {
+          // Import the body of D and attach that to FoundByLookup.
+          // This should probably be refactored into a function since we do the
+          // same below too.
+          if (Stmt *FromBody = D->getBody()) {
+            if (ExpectedStmt ToBodyOrErr = import(FromBody))
+              FoundByLookup->setBody(*ToBodyOrErr);
+            else
+              return ToBodyOrErr.takeError();
+          }
+          return cast<Decl>(const_cast<FunctionDecl *>(FoundByLookup));
+        }
+      }
+    }
+  }
+
   DeclarationNameInfo NameInfo(Name, Loc);
   // Import additional name location/type info.
   if (Error Err = ImportDeclarationNameLoc(D->getNameInfo(), NameInfo))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to