shafik updated this revision to Diff 183251.
shafik added a comment.

Addressing comments on naming in the unit test and refactoring of duplicated 
code.


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

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,187 @@
             }).match(ToTU, functionDecl()));
 }
 
+TEST_P(ImportFunctions, ImportOverriddenMethodTwice) {
+  auto Code =
+      R"(
+      struct B { virtual void f(); };
+      struct D:B { void f(); };
+      )";
+  auto BFP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  auto DFP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+
+  Decl *FromTU0 = getTuDecl(Code, Lang_CXX);
+  auto *DF = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
+  Import(DF, Lang_CXX);
+
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *BF = FirstDeclMatcher<CXXMethodDecl>().match(FromTU1, BFP);
+  Import(BF, Lang_CXX);
+
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 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 BFP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  auto DFP = cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+  auto BFDefP = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto DFDefP = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("D"))), isDefinition());
+  auto FDefAllP = cxxMethodDecl(hasName("f"), isDefinition());
+
+  {
+    Decl *FromTU = getTuDecl(CodeWithDef, Lang_CXX, "input0.cc");
+    auto *FromD = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, DFP);
+    Import(FromD, Lang_CXX);
+  }
+  {
+    Decl *FromTU = getTuDecl(CodeWithoutDef, Lang_CXX, "input1.cc");
+    auto *FromB = FirstDeclMatcher<CXXMethodDecl>().match(FromTU, BFP);
+    Import(FromB, Lang_CXX);
+  }
+
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFDefP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFDefP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, FDefAllP), 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 BFPIsDefP = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto DFP =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))), unless(isDefinition()) );
+
+  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, BFPIsDefP), 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, ImportOverriddenMethodTwiceOutOfClassDefInSeparateCode) {
+  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, D &d) { b.f(); d.f(); }
+      )";
+
+  auto BFP =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  auto BFPIsDefP = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition());
+  auto DFP =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+  auto DFPIsDefP = cxxMethodDecl(
+      hasName("f"), hasParent(cxxRecordDecl(hasName("D"))), isDefinition());
+  auto FooDef = functionDecl(hasName("foo"));
+
+  {
+    Decl *FromTU0 = getTuDecl(CodeTU0, Lang_CXX, "input0.cc");
+    auto *D = FirstDeclMatcher<CXXMethodDecl>().match(FromTU0, DFP);
+    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, DFP), 1u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, BFPIsDefP), 0u);
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, DFPIsDefP), 0u);
+
+  auto *ToB = FirstDeclMatcher<CXXRecordDecl>().match(
+      ToTU, cxxRecordDecl(hasName("B")));
+  auto *ToD = FirstDeclMatcher<CXXRecordDecl>().match(
+      ToTU, cxxRecordDecl(hasName("D")));
+  auto *ToBFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, BFP);
+  auto *ToBFOutOfClass = FirstDeclMatcher<CXXMethodDecl>().match(
+      ToTU, cxxMethodDecl(hasName("f"), isDefinition()));
+  auto *ToDFInClass = FirstDeclMatcher<CXXMethodDecl>().match(ToTU, DFP);
+  auto *ToDFOutOfClass = LastDeclMatcher<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);
+
+  EXPECT_NE(ToDFInClass, ToDFOutOfClass);
+  EXPECT_NE(ToDFInClass->getLexicalDeclContext(),
+              ToDFOutOfClass->getLexicalDeclContext());
+  EXPECT_EQ(ToDFOutOfClass->getDeclContext(), ToD);
+  EXPECT_EQ(ToDFOutOfClass->getLexicalDeclContext(), ToTU);
+
+
+  // Check that the redecl chain is intact.
+  EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
+  EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -438,6 +438,8 @@
 
     Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
+    ExpectedStmt ImportFunctionDeclBody( FunctionDecl *FromFD, FunctionDecl *ToFD );
+
     bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
     bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
                            bool Complain = true);
@@ -2945,6 +2947,16 @@
   return FoundSpec;
 }
 
+ExpectedStmt ASTNodeImporter::ImportFunctionDeclBody( FunctionDecl *FromFD, FunctionDecl *ToFD ) {
+    if (Stmt *FromBody = FromFD->getBody()) {
+        if (ExpectedStmt ToBodyOrErr = import(FromBody))
+            ToFD->setBody(*ToBodyOrErr);
+        else
+            return ToBodyOrErr.takeError();
+    }
+    return ToFD->getBody();
+}
+
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
@@ -2968,7 +2980,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 +3051,24 @@
     }
   }
 
+  if (FoundByLookup) {
+    if (auto *MD = dyn_cast<CXXMethodDecl>(FoundByLookup)) {
+      if (D->getLexicalDeclContext() == D->getDeclContext()) {
+        if (!D->doesThisDeclarationHaveABody())
+          return Importer.MapImported(D, FoundByLookup);
+        else {
+          // Import the body of D and attach that to FoundByLookup.
+          ExpectedStmt ToBodyOrErr = ImportFunctionDeclBody( D, FoundByLookup );
+
+          if( !ToBodyOrErr)
+            return ToBodyOrErr.takeError();
+
+          return cast<Decl>(FoundByLookup);
+        }
+      }
+    }
+  }
+
   DeclarationNameInfo NameInfo(Name, Loc);
   // Import additional name location/type info.
   if (Error Err = ImportDeclarationNameLoc(D->getNameInfo(), NameInfo))
@@ -3184,12 +3214,10 @@
   }
 
   if (D->doesThisDeclarationHaveABody()) {
-    if (Stmt *FromBody = D->getBody()) {
-      if (ExpectedStmt ToBodyOrErr = import(FromBody))
-        ToFunction->setBody(*ToBodyOrErr);
-      else
-        return ToBodyOrErr.takeError();
-    }
+      ExpectedStmt ToBodyOrErr = ImportFunctionDeclBody( D, ToFunction );
+
+      if( !ToBodyOrErr)
+          return ToBodyOrErr.takeError();
   }
 
   // FIXME: Other bits to merge?
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to