[ASTImporter] Do not import FunctionTemplateDecl in record twice.

[balazske]

Summary

For functions there is a check to not duplicate the declaration if it is in a
record (class). For function templates there was no similar check, if a
template (in the same class) was imported multiple times the
FunctionTemplateDecl was created multiple times with the same templated
FunctionDecl. This can result in problems with the declaration chain.

[balazske]

Added missing test fixture, removed not a related change.

[martong]

@a_sidorin This is a polite Ping.

[a_sidorin]

LGTM, thanks!

clang/lib/AST/ASTImporter.cpp

ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   // because AST clients like VTableBuilder asserts on this. VTableBuilder
   // assumes there is only one in-class declaration. Building a redecl
   // chain would result in more than one in-class declaration for
   // overrides (even if they are part of the same redecl chain inside the
   // derived class.)
   if (FoundByLookup) {
     if (isa<CXXMethodDecl>(FoundByLookup)) {
       if (D->getLexicalDeclContext() == D->getDeclContext()) {
-        if (!D->doesThisDeclarationHaveABody())
+        if (!D->doesThisDeclarationHaveABody()) {
+          if (FunctionTemplateDecl *DescribedD =
+                  D->getDescribedFunctionTemplate()) {
+            // Handle a "templated" function together with its described
+            // template. This avoids need for a similar check at import of the
+            // described template.
+            assert(FoundByLookup->getDescribedFunctionTemplate() &&
+                   "Templated function mapped to non-templated?");
+            Importer.MapImported(DescribedD,
+                                 FoundByLookup->getDescribedFunctionTemplate());
+          }
           return Importer.MapImported(D, FoundByLookup);
-        else {
+        } else {
           // Let's continue and build up the redecl chain in this case.
           // FIXME Merge the functions into one decl.
         }
       }
     }
   }
 
   DeclarationNameInfo NameInfo(Name, Loc);
for (auto *FoundDecl : FoundDecls) {
             break;
           }
           // TODO: handle conflicting names
         }
       }
     }
   }
 
-  auto ParamsOrErr = import(D->getTemplateParameters());
-  if (!ParamsOrErr)
-    return ParamsOrErr.takeError();
-
   FunctionDecl *TemplatedFD;
   if (Error Err = importInto(TemplatedFD, D->getTemplatedDecl()))
     return std::move(Err);
 
+  auto ParamsOrErr = import(D->getTemplateParameters());
+  if (!ParamsOrErr)
+    return ParamsOrErr.takeError();
+
   FunctionTemplateDecl *ToFunc;
   if (GetImportedOrCreateDecl(ToFunc, D, Importer.getToContext(), DC, Loc, Name,
                               *ParamsOrErr, TemplatedFD))
     return ToFunc;
 
   TemplatedFD->setDescribedFunctionTemplate(ToFunc);
 
   ToFunc->setAccess(D->getAccess());

clang/unittests/AST/ASTImporterTest.cpp

TEST_P(ImportFunctions,
   auto *ToD = Import(FromD, Lang_CXX);
   EXPECT_TRUE(ToD);
   Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   EXPECT_TRUE(MatchVerifier<TranslationUnitDecl>().match(
       ToTU, translationUnitDecl(hasDescendant(
                 functionDecl(hasName("f"), hasDescendant(declRefExpr()))))));
 }
 
+TEST_P(ImportFunctionTemplates, ImportFunctionTemplateInRecordDeclTwice) {
+  auto Code =
+      R"(
+      class X {
+        template <class T>
+        void f(T t);
+      };
+      )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
+TEST_P(ImportFunctionTemplates,
+       ImportFunctionTemplateWithDefInRecordDeclTwice) {
+  auto Code =
+      R"(
+      class X {
+        template <class T>
+        void f(T t);
+      };
+      template <class T>
+      void X::f(T t) {};
+      )";
+  Decl *FromTU1 = getTuDecl(Code, Lang_CXX, "input1.cc");
+  auto *FromD1 = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU1, functionTemplateDecl(hasName("f")));
+  auto *ToD1 = Import(FromD1, Lang_CXX);
+  Decl *FromTU2 = getTuDecl(Code, Lang_CXX, "input2.cc");
+  auto *FromD2 = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU2, functionTemplateDecl(hasName("f")));
+  auto *ToD2 = Import(FromD2, Lang_CXX);
+  EXPECT_EQ(ToD1, ToD2);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
   auto Pattern = functionDecl(hasName("f"));
 
   Decl *FromTU = getTuDecl("struct X { friend void f(); };"
                            "void f();",
                            Lang_CXX,