[Clang][ASTImporter] Added visibility check for FunctionTemplateDecl.

[balazske]

Summary

ASTImporter makes now difference between function templates with same
name in different translation units if these are not visible outside.

[a_sidorin]

Looks good!

clang/lib/AST/ASTImporter.cpp

ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
   if (!LexicalDC->isFunctionOrMethod()) {
     unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend;
     auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
     for (auto *FoundDecl : FoundDecls) {
       if (!FoundDecl->isInIdentifierNamespace(IDNS))
         continue;
 
       if (auto *FoundTemplate = dyn_cast<FunctionTemplateDecl>(FoundDecl)) {
-        if (FoundTemplate->hasExternalFormalLinkage() &&
-            D->hasExternalFormalLinkage()) {
-          if (IsStructuralMatch(D, FoundTemplate)) {
-            FunctionTemplateDecl *TemplateWithDef =
-                getTemplateDefinition(FoundTemplate);
-            if (D->isThisDeclarationADefinition() && TemplateWithDef) {
-              return Importer.MapImported(D, TemplateWithDef);
-            }
-            FoundByLookup = FoundTemplate;
-            break;
-          }
+        if (!hasSameVisibilityContext(FoundTemplate, D))
+          continue;
+          if (D->isThisDeclarationADefinition() && TemplateWithDef)
+
           // TODO: handle conflicting names
         }
       }
     }
   }
 
   auto ParamsOrErr = import(D->getTemplateParameters());
   if (!ParamsOrErr)

clang/unittests/AST/ASTImporterVisibilityTest.cpp

 struct GetEnumPattern {
   using DeclTy = EnumDecl;
   BindableMatcher<Decl> operator()() { return enumDecl(hasName("E")); }
 };
 struct GetTypedefNamePattern {
   using DeclTy = TypedefNameDecl;
   BindableMatcher<Decl> operator()() { return typedefNameDecl(hasName("T")); }
 };
+struct GetFunTemplPattern {
+  using DeclTy = FunctionTemplateDecl;
+  BindableMatcher<Decl> operator()() {
+    return functionTemplateDecl(hasName("f"));
+  }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
 const auto *ExternF = "void f();";
 const auto *StaticF = "static void f();";
 const auto *AnonF = "namespace { void f(); }";
 // VarDecl:
 const auto *ExternV = "extern int v;";
 const auto *StaticV = "static int v;";
 const auto *AnonV = "namespace { extern int v; }";
 // CXXRecordDecl:
 const auto *ExternC = "class X;";
 const auto *AnonC = "namespace { class X; }";
 // EnumDecl:
 const auto *ExternE = "enum E {};";
 const auto *AnonE = "namespace { enum E {}; }";
 // TypedefNameDecl:
 const auto *ExternTypedef = "typedef int T;";
 const auto *AnonTypedef = "namespace { typedef int T; }";
 const auto *ExternUsing = "using T = int;";
 const auto *AnonUsing = "namespace { using T = int; }";
+// FunctionTemplateDecl:
+const auto *ExternFT = "template <class> void f();";
+const auto *StaticFT = "template <class> static void f();";
+const auto *AnonFT = "namespace { template <class> void f(); }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
 using ImportVisibilityChainParams =
     ::testing::WithParamInterface<std::tuple<ArgVector, const char *>>;
 // Fixture to test the redecl chain of Decls with the same visibility. Gtest
 // makes it possible to have either value-parameterized or type-parameterized
 // fixtures. However, we cannot have both value- and type-parameterized test
void TypedTest_ImportChain() {
     EXPECT_EQ(ToD1->getPreviousDecl(), ToD0);
   }
 };
 
 // Manual instantiation of the fixture with each type.
 using ImportFunctionsVisibilityChain = ImportVisibilityChain<GetFunPattern>;
 using ImportVariablesVisibilityChain = ImportVisibilityChain<GetVarPattern>;
 using ImportClassesVisibilityChain = ImportVisibilityChain<GetClassPattern>;
+using ImportFunctionTemplatesVisibilityChain =
+    ImportVisibilityChain<GetFunTemplPattern>;
+
 // Value-parameterized test for functions.
 TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
 // Value-parameterized test for variables.
 TEST_P(ImportVariablesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
 // Value-parameterized test for classes.
 TEST_P(ImportClassesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
+// Value-parameterized test for function templates.
+TEST_P(ImportFunctionTemplatesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
 
 // Automatic instantiation of the value-parameterized tests.
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
                         ::testing::Combine(
                            DefaultTestValuesForRunOptions,
                            ::testing::Values(ExternF, StaticF, AnonF)), );
 INSTANTIATE_TEST_CASE_P(
     ParameterizedTests, ImportVariablesVisibilityChain,
     ::testing::Combine(
         DefaultTestValuesForRunOptions,
         // There is no point to instantiate with StaticV, because in C++ we can
         // forward declare a variable only with the 'extern' keyword.
         // Consequently, each fwd declared variable has external linkage.  This
         // is different in the C language where any declaration without an
         // initializer is a tentative definition, subsequent definitions may be
         // provided but they must have the same linkage.  See also the test
         // ImportVariableChainInC which test for this special C Lang case.
         ::testing::Values(ExternV, AnonV)), );
 INSTANTIATE_TEST_CASE_P(
     ParameterizedTests, ImportClassesVisibilityChain,
     ::testing::Combine(
         DefaultTestValuesForRunOptions,
         ::testing::Values(ExternC, AnonC)), );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests,
+                        ImportFunctionTemplatesVisibilityChain,
+                        ::testing::Combine(DefaultTestValuesForRunOptions,
+                                           ::testing::Values(ExternFT, StaticFT,
+                                                             AnonFT)), );
 
 // First value in tuple: Compile options.
 // Second value in tuple: Tuple with informations for the test.
 // Code for first import (or initial code), code to import, whether the `f`
 // functions are expected to be linked in a declaration chain.
 // One value of this tuple is combined with every value of compile options.
 // The test can have a single tuple as parameter only.
 using ImportVisibilityParams = ::testing::WithParamInterface<
EXPECT_EQ(0u, ToF0->getTranslationUnitDecl()
                       .getNumWarnings());
   }
 };
 using ImportFunctionsVisibility = ImportVisibility<GetFunPattern>;
 using ImportVariablesVisibility = ImportVisibility<GetVarPattern>;
 using ImportClassesVisibility = ImportVisibility<GetClassPattern>;
 using ImportEnumsVisibility = ImportVisibility<GetEnumPattern>;
 using ImportTypedefNameVisibility = ImportVisibility<GetTypedefNamePattern>;
+using ImportFunctionTemplatesVisibility = ImportVisibility<GetFunTemplPattern>;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
   TypedTest_ImportAfter();
 }
 TEST_P(ImportFunctionsVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImport();
 }

 }
 // TypedefNameDecl.
 TEST_P(ImportTypedefNameVisibility, ImportAfter) {
   TypedTest_ImportAfterWithMerge();
 }
 TEST_P(ImportTypedefNameVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImportWithMerge();
 }
+// FunctionTemplateDecl.
+TEST_P(ImportFunctionTemplatesVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportFunctionTemplatesVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
 
 const bool ExpectLinkedDeclChain = true;
 const bool ExpectUnlinkedDeclChain = false;
 
 INSTANTIATE_TEST_CASE_P(
     ParameterizedTests, ImportFunctionsVisibility,
     ::testing::Combine(
         DefaultTestValuesForRunOptions,
::testing::Combine(
             std::make_tuple(AnonUsing, ExternTypedef, ExpectUnlinkedDeclChain),
             std::make_tuple(AnonUsing, AnonTypedef, ExpectUnlinkedDeclChain),
 
             std::make_tuple(ExternTypedef, ExternUsing, ExpectLinkedDeclChain),
             std::make_tuple(ExternTypedef, AnonUsing, ExpectUnlinkedDeclChain),
             std::make_tuple(AnonTypedef, ExternUsing, ExpectUnlinkedDeclChain),
             std::make_tuple(AnonTypedef, AnonUsing,
                             ExpectUnlinkedDeclChain))), );
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportFunctionTemplatesVisibility,
+    ::testing::Combine(
+        DefaultTestValuesForRunOptions,
+        ::testing::Values(
+            std::make_tuple(ExternFT, ExternFT, ExpectLinkedDeclChain),
+            std::make_tuple(ExternFT, StaticFT, ExpectUnlinkedDeclChain),
+            std::make_tuple(ExternFT, AnonFT, ExpectUnlinkedDeclChain),
+            std::make_tuple(StaticFT, ExternFT, ExpectUnlinkedDeclChain),
+            std::make_tuple(StaticFT, StaticFT, ExpectUnlinkedDeclChain),
+            std::make_tuple(StaticFT, AnonFT, ExpectUnlinkedDeclChain),
+            std::make_tuple(AnonFT, ExternFT, ExpectUnlinkedDeclChain),
+            std::make_tuple(AnonFT, StaticFT, ExpectUnlinkedDeclChain),
+            std::make_tuple(AnonFT, AnonFT, ExpectUnlinkedDeclChain))), );
 
 } // end namespace ast_matchers
 } // end namespace clang