[ASTImporter] Added visibility context check for EnumDecl.

Summary:
ASTImporter makes now difference between enums with same name in different translation
units if these are not visible outside.
("Scoped enums" are not handled yet.)

Reviewers: martong, a.sidorin, shafik, a_sidorin

Reviewed By: a_sidorin

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

Tags: #clang

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

llvm-svn: 365464

2 files changed, 67 insertions, 0 deletions

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 55d414e4e57..863a1cb0af8 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2476,6 +2476,8 @@ ExpectedDecl ASTNodeImporter::VisitEnumDecl(EnumDecl *D) {
       }
 
       if (auto *FoundEnum = dyn_cast<EnumDecl>(FoundDecl)) {
+        if (!hasSameVisibilityContext(FoundEnum, D))
+          continue;
         if (IsStructuralMatch(D, FoundEnum))
           return Importer.MapImported(D, FoundEnum);
       }
diff --git a/clang/unittests/AST/ASTImporterVisibilityTest.cpp b/clang/unittests/AST/ASTImporterVisibilityTest.cpp
index 95b7c4c920a..66c967e8460 100644
--- a/clang/unittests/AST/ASTImporterVisibilityTest.cpp
+++ b/clang/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -35,6 +35,10 @@ struct GetClassPattern {
   using DeclTy = CXXRecordDecl;
   BindableMatcher<Decl> operator()() { return cxxRecordDecl(hasName("X")); }
 };
+struct GetEnumPattern {
+  using DeclTy = EnumDecl;
+  BindableMatcher<Decl> operator()() { return enumDecl(hasName("E")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -48,6 +52,9 @@ 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 {}; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -183,10 +190,53 @@ protected:
     else
       EXPECT_FALSE(ToD1->getPreviousDecl());
   }
+
+  void TypedTest_ImportAfterWithMerge() {
+    TranslationUnitDecl *ToTu = getToTuDecl(getCode0(), Lang_CXX);
+    TranslationUnitDecl *FromTu = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
+
+    auto *ToF0 = FirstDeclMatcher<DeclTy>().match(ToTu, getPattern());
+    auto *FromF1 = FirstDeclMatcher<DeclTy>().match(FromTu, getPattern());
+
+    auto *ToF1 = Import(FromF1, Lang_CXX);
+
+    ASSERT_TRUE(ToF0);
+    ASSERT_TRUE(ToF1);
+
+    if (shouldBeLinked())
+      EXPECT_EQ(ToF0, ToF1);
+    else
+      EXPECT_NE(ToF0, ToF1);
+
+    // We expect no (ODR) warning during the import.
+    EXPECT_EQ(0u, ToTu->getASTContext().getDiagnostics().getNumWarnings());
+  }
+
+  void TypedTest_ImportAfterImportWithMerge() {
+    TranslationUnitDecl *FromTu0 = getTuDecl(getCode0(), Lang_CXX, "input0.cc");
+    TranslationUnitDecl *FromTu1 = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
+    auto *FromF0 = FirstDeclMatcher<DeclTy>().match(FromTu0, getPattern());
+    auto *FromF1 = FirstDeclMatcher<DeclTy>().match(FromTu1, getPattern());
+    auto *ToF0 = Import(FromF0, Lang_CXX);
+    auto *ToF1 = Import(FromF1, Lang_CXX);
+    ASSERT_TRUE(ToF0);
+    ASSERT_TRUE(ToF1);
+    if (shouldBeLinked())
+      EXPECT_EQ(ToF0, ToF1);
+    else
+      EXPECT_NE(ToF0, ToF1);
+
+    // We expect no (ODR) warning during the import.
+    EXPECT_EQ(0u, ToF0->getTranslationUnitDecl()
+                      ->getASTContext()
+                      .getDiagnostics()
+                      .getNumWarnings());
+  }
 };
 using ImportFunctionsVisibility = ImportVisibility<GetFunPattern>;
 using ImportVariablesVisibility = ImportVisibility<GetVarPattern>;
 using ImportClassesVisibility = ImportVisibility<GetClassPattern>;
+using ImportEnumsVisibility = ImportVisibility<GetEnumPattern>;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -209,6 +259,13 @@ TEST_P(ImportClassesVisibility, ImportAfter) {
 TEST_P(ImportClassesVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImport();
 }
+// EnumDecl.
+TEST_P(ImportEnumsVisibility, ImportAfter) {
+  TypedTest_ImportAfterWithMerge();
+}
+TEST_P(ImportEnumsVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImportWithMerge();
+}
 
 const bool ExpectLink = true;
 const bool ExpectNotLink = false;
@@ -247,6 +304,14 @@ INSTANTIATE_TEST_CASE_P(
                           std::make_tuple(ExternC, AnonC, ExpectNotLink),
                           std::make_tuple(AnonC, ExternC, ExpectNotLink),
                           std::make_tuple(AnonC, AnonC, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportEnumsVisibility,
+    ::testing::Combine(
+        DefaultTestValuesForRunOptions,
+        ::testing::Values(std::make_tuple(ExternE, ExternE, ExpectLink),
+                          std::make_tuple(ExternE, AnonE, ExpectNotLink),
+                          std::make_tuple(AnonE, ExternE, ExpectNotLink),
+                          std::make_tuple(AnonE, AnonE, ExpectNotLink))), );
 
 } // end namespace ast_matchers
 } // end namespace clang