[ASTImporter] Added visibility context check for TypedefNameDecl.

Summary:
ASTImporter makes now difference between typedefs and type aliases
with same name in different translation units
if these are not visible outside.

Reviewers: martong, a.sidorin, shafik, a_sidorin

Reviewed By: martong, shafik

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

llvm-svn: 370903

clang/lib/AST/ASTImporter.cpp

@@ -932,6 +932,27 @@ Expected<LambdaCapture> ASTNodeImporter::import(const LambdaCapture &From) {
       EllipsisLoc);
 }
 
+template <typename T>
+bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
+  if (From->hasExternalFormalLinkage())
+    return Found->hasExternalFormalLinkage();
+  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
+    return false;
+  if (From->isInAnonymousNamespace())
+    return Found->isInAnonymousNamespace();
+  else
+    return !Found->isInAnonymousNamespace() &&
+           !Found->hasExternalFormalLinkage();
+}
+
+template <>
+bool ASTNodeImporter::hasSameVisibilityContext(TypedefNameDecl *Found,
+                                               TypedefNameDecl *From) {
+  if (From->isInAnonymousNamespace() && Found->isInAnonymousNamespace())
+    return Importer.GetFromTU(Found) == From->getTranslationUnitDecl();
+  return From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
+}
+
 } // namespace clang
 
 //----------------------------------------------------------------------------
@@ -2344,6 +2365,9 @@ ASTNodeImporter::VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias) {
   // If this typedef is not in block scope, determine whether we've
   // seen a typedef with the same name (that we can merge with) or any
   // other entity by that name (which name lookup could conflict with).
+  // Note: Repeated typedefs are not valid in C99:
+  // 'typedef int T; typedef int T;' is invalid
+  // We do not care about this now.
   if (!DC->isFunctionOrMethod()) {
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     unsigned IDNS = Decl::IDNS_Ordinary;
@@ -2352,6 +2376,9 @@ ASTNodeImporter::VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias) {
       if (!FoundDecl->isInIdentifierNamespace(IDNS))
         continue;
       if (auto *FoundTypedef = dyn_cast<TypedefNameDecl>(FoundDecl)) {
+        if (!hasSameVisibilityContext(FoundTypedef, D))
+          continue;
+
         QualType FromUT = D->getUnderlyingType();
         QualType FoundUT = FoundTypedef->getUnderlyingType();
         if (Importer.IsStructurallyEquivalent(FromUT, FoundUT)) {
@@ -3022,19 +3049,6 @@ Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD,
   return Error::success();
 }
 
-template <typename T>
-bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
-  if (From->hasExternalFormalLinkage())
-    return Found->hasExternalFormalLinkage();
-  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
-    return false;
-  if (From->isInAnonymousNamespace())
-    return Found->isInAnonymousNamespace();
-  else
-    return !Found->isInAnonymousNamespace() &&
-           !Found->hasExternalFormalLinkage();
-}
-
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);

clang/unittests/AST/ASTImporterVisibilityTest.cpp

@@ -39,6 +39,10 @@ struct GetEnumPattern {
   using DeclTy = EnumDecl;
   BindableMatcher<Decl> operator()() { return enumDecl(hasName("E")); }
 };
+struct GetTypedefNamePattern {
+  using DeclTy = TypedefNameDecl;
+  BindableMatcher<Decl> operator()() { return typedefNameDecl(hasName("T")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -55,6 +59,11 @@ 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; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -243,6 +252,7 @@ using ImportFunctionsVisibility = ImportVisibility<GetFunPattern>;
 using ImportVariablesVisibility = ImportVisibility<GetVarPattern>;
 using ImportClassesVisibility = ImportVisibility<GetClassPattern>;
 using ImportEnumsVisibility = ImportVisibility<GetEnumPattern>;
+using ImportTypedefNameVisibility = ImportVisibility<GetTypedefNamePattern>;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -272,6 +282,13 @@ TEST_P(ImportEnumsVisibility, ImportAfter) {
 TEST_P(ImportEnumsVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImportWithMerge();
 }
+// TypedefNameDecl.
+TEST_P(ImportTypedefNameVisibility, ImportAfter) {
+  TypedTest_ImportAfterWithMerge();
+}
+TEST_P(ImportTypedefNameVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImportWithMerge();
+}
 
 const bool ExpectLink = true;
 const bool ExpectNotLink = false;
@@ -318,6 +335,30 @@ INSTANTIATE_TEST_CASE_P(
                           std::make_tuple(ExternE, AnonE, ExpectNotLink),
                           std::make_tuple(AnonE, ExternE, ExpectNotLink),
                           std::make_tuple(AnonE, AnonE, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportTypedefNameVisibility,
+    ::testing::Combine(
+        DefaultTestValuesForRunOptions,
+        ::testing::Values(
+            std::make_tuple(ExternTypedef, ExternTypedef, ExpectLink),
+            std::make_tuple(ExternTypedef, AnonTypedef, ExpectNotLink),
+            std::make_tuple(AnonTypedef, ExternTypedef, ExpectNotLink),
+            std::make_tuple(AnonTypedef, AnonTypedef, ExpectNotLink),
+
+            std::make_tuple(ExternUsing, ExternUsing, ExpectLink),
+            std::make_tuple(ExternUsing, AnonUsing, ExpectNotLink),
+            std::make_tuple(AnonUsing, ExternUsing, ExpectNotLink),
+            std::make_tuple(AnonUsing, AnonUsing, ExpectNotLink),
+
+            std::make_tuple(ExternUsing, ExternTypedef, ExpectLink),
+            std::make_tuple(ExternUsing, AnonTypedef, ExpectNotLink),
+            std::make_tuple(AnonUsing, ExternTypedef, ExpectNotLink),
+            std::make_tuple(AnonUsing, AnonTypedef, ExpectNotLink),
+
+            std::make_tuple(ExternTypedef, ExternUsing, ExpectLink),
+            std::make_tuple(ExternTypedef, AnonUsing, ExpectNotLink),
+            std::make_tuple(AnonTypedef, ExternUsing, ExpectNotLink),
+            std::make_tuple(AnonTypedef, AnonUsing, ExpectNotLink))), );
 
 } // end namespace ast_matchers
 } // end namespace clang