From 6c41f8d5a6018d25b91fa0420c4325b0d85f0743 Mon Sep 17 00:00:00 2001
From: Balazs Keri <1.int32@gmail.com>
Date: Mon, 27 May 2019 09:36:00 +0000
Subject: [PATCH] [ASTImporter] Added visibility context check for
 CXXRecordDecl.

Summary:
ASTImporter makes now difference between classes with same name in different
translation units if these are not visible outside. These classes are not linked
into one decl chain.

Reviewers: martong, a.sidorin, shafik

Reviewed By: shafik

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

Tags: #clang

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@361752 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/AST/ASTImporter.cpp                     |  3 ++
 unittests/AST/ASTImporterVisibilityTest.cpp | 37 +++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp
index 2b7470410f..2e4c304b3d 100644
--- a/lib/AST/ASTImporter.cpp
+++ b/lib/AST/ASTImporter.cpp
@@ -2559,6 +2559,9 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
           if (!IsStructuralMatch(D, FoundRecord, false))
             continue;
 
+        if (!hasSameVisibilityContext(FoundRecord, D))
+          continue;
+
         if (IsStructuralMatch(D, FoundRecord)) {
           RecordDecl *FoundDef = FoundRecord->getDefinition();
           if (D->isThisDeclarationADefinition() && FoundDef) {
diff --git a/unittests/AST/ASTImporterVisibilityTest.cpp b/unittests/AST/ASTImporterVisibilityTest.cpp
index a4d242d3bb..95b7c4c920 100644
--- a/unittests/AST/ASTImporterVisibilityTest.cpp
+++ b/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -31,6 +31,10 @@ struct GetVarPattern {
   using DeclTy = VarDecl;
   BindableMatcher<Decl> operator()() { return varDecl(hasName("v")); }
 };
+struct GetClassPattern {
+  using DeclTy = CXXRecordDecl;
+  BindableMatcher<Decl> operator()() { return cxxRecordDecl(hasName("X")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -41,6 +45,9 @@ const auto *AnonF = "namespace { void f(); }";
 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; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -84,14 +91,19 @@ protected:
 // Manual instantiation of the fixture with each type.
 using ImportFunctionsVisibilityChain = ImportVisibilityChain<GetFunPattern>;
 using ImportVariablesVisibilityChain = ImportVisibilityChain<GetVarPattern>;
-// Value-parameterized test for the first type.
+using ImportClassesVisibilityChain = ImportVisibilityChain<GetClassPattern>;
+// Value-parameterized test for functions.
 TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
-// Value-parameterized test for the second type.
+// Value-parameterized test for variables.
 TEST_P(ImportVariablesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
+// Value-parameterized test for classes.
+TEST_P(ImportClassesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
 
 // Automatic instantiation of the value-parameterized tests.
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
@@ -110,6 +122,11 @@ INSTANTIATE_TEST_CASE_P(
         // 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)), );
 
 // First value in tuple: Compile options.
 // Second value in tuple: Tuple with informations for the test.
@@ -169,6 +186,7 @@ protected:
 };
 using ImportFunctionsVisibility = ImportVisibility<GetFunPattern>;
 using ImportVariablesVisibility = ImportVisibility<GetVarPattern>;
+using ImportClassesVisibility = ImportVisibility<GetClassPattern>;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -184,6 +202,13 @@ TEST_P(ImportVariablesVisibility, ImportAfter) {
 TEST_P(ImportVariablesVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImport();
 }
+// CXXRecordDecl.
+TEST_P(ImportClassesVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportClassesVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
 
 const bool ExpectLink = true;
 const bool ExpectNotLink = false;
@@ -214,6 +239,14 @@ INSTANTIATE_TEST_CASE_P(
                           std::make_tuple(AnonV, ExternV, ExpectNotLink),
                           std::make_tuple(AnonV, StaticV, ExpectNotLink),
                           std::make_tuple(AnonV, AnonV, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportClassesVisibility,
+    ::testing::Combine(
+        DefaultTestValuesForRunOptions,
+        ::testing::Values(std::make_tuple(ExternC, ExternC, ExpectLink),
+                          std::make_tuple(ExternC, AnonC, ExpectNotLink),
+                          std::make_tuple(AnonC, ExternC, ExpectNotLink),
+                          std::make_tuple(AnonC, AnonC, ExpectNotLink))), );
 
 } // end namespace ast_matchers
 } // end namespace clang
-- 
2.40.0