From 75ea855fd7d4720ca07a0a52d10113593bff17db Mon Sep 17 00:00:00 2001
From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Wed, 17 Jun 2015 23:07:50 +0000
Subject: [PATCH] [modules] Ensure that if we merge the definitions of two
 enumerations, that making either of them visible makes the merged definition
 visible.

llvm-svn: 239969
---
 clang/lib/Sema/SemaDecl.cpp                  |  1 -
 clang/lib/Serialization/ASTReaderDecl.cpp    | 40 ++++++++++++++++------------
 clang/test/Modules/submodules-merge-defs.cpp |  9 +++++++
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 347d807..705f63c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13519,7 +13519,6 @@ Sema::SkipBodyInfo Sema::shouldSkipAnonEnumBody(Scope *S, IdentifierInfo *II,
       !hasVisibleDefinition(cast<NamedDecl>(PrevECD->getDeclContext()),
                             &Hidden)) {
     SkipBodyInfo Skip;
-    Skip.ShouldSkip = true;
     Skip.Previous = Hidden;
     return Skip;
   }
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 548ce9c..4d64c52 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -365,6 +365,27 @@ namespace clang {
     void VisitObjCPropertyDecl(ObjCPropertyDecl *D);
     void VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D);
     void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
+
+    /// We've merged the definition \p MergedDef into the existing definition
+    /// \p Def. Ensure that \p Def is made visible whenever \p MergedDef is made
+    /// visible.
+    void mergeDefinitionVisibility(NamedDecl *Def, NamedDecl *MergedDef) {
+      if (Def->isHidden()) {
+        // If MergedDef is visible or becomes visible, make the definition visible.
+        if (!MergedDef->isHidden())
+          Def->Hidden = false;
+        else if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
+          Reader.getContext().mergeDefinitionIntoModule(
+              Def, MergedDef->getImportedOwningModule(),
+              /*NotifyListeners*/ false);
+          Reader.PendingMergedDefinitionsToDeduplicate.insert(Def);
+        } else {
+          auto SubmoduleID = MergedDef->getOwningModuleID();
+          assert(SubmoduleID && "hidden definition in no module");
+          Reader.HiddenNamesMap[Reader.getSubmodule(SubmoduleID)].push_back(Def);
+        }
+      }
+    }
   };
 }
 
@@ -588,6 +609,7 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
     if (EnumDecl *&OldDef = Reader.EnumDefinitions[ED->getCanonicalDecl()]) {
       Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
       ED->IsCompleteDefinition = false;
+      mergeDefinitionVisibility(OldDef, ED);
     } else {
       OldDef = ED;
     }
@@ -1400,23 +1422,7 @@ void ASTDeclReader::MergeDefinitionData(
   if (DD.Definition != MergeDD.Definition) {
     Reader.MergedLookups[DD.Definition].push_back(MergeDD.Definition);
     DD.Definition->setHasExternalVisibleStorage();
-
-    if (DD.Definition->isHidden()) {
-      // If MergeDD is visible or becomes visible, make the definition visible.
-      if (!MergeDD.Definition->isHidden())
-        DD.Definition->Hidden = false;
-      else if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
-        Reader.getContext().mergeDefinitionIntoModule(
-            DD.Definition, MergeDD.Definition->getImportedOwningModule(),
-            /*NotifyListeners*/ false);
-        Reader.PendingMergedDefinitionsToDeduplicate.insert(DD.Definition);
-      } else {
-        auto SubmoduleID = MergeDD.Definition->getOwningModuleID();
-        assert(SubmoduleID && "hidden definition in no module");
-        Reader.HiddenNamesMap[Reader.getSubmodule(SubmoduleID)].push_back(
-            DD.Definition);
-      }
-    }
+    mergeDefinitionVisibility(DD.Definition, MergeDD.Definition);
   }
 
   auto PFDI = Reader.PendingFakeDefinitionData.find(&DD);
diff --git a/clang/test/Modules/submodules-merge-defs.cpp b/clang/test/Modules/submodules-merge-defs.cpp
index 38b3147..94c0fd3 100644
--- a/clang/test/Modules/submodules-merge-defs.cpp
+++ b/clang/test/Modules/submodules-merge-defs.cpp
@@ -48,6 +48,13 @@ int pre_ff = F<int>().f(); // expected-error +{{must be imported}}
 int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}}
 // expected-note@defs.h:26 +{{here}}
 
+G::A pre_ga // expected-error +{{must be imported}}
+  = G::a; // expected-error +{{must be imported}}
+// expected-note@defs.h:40 +{{here}}
+// expected-note@defs.h:41 +{{here}}
+decltype(G::h) pre_gh = G::h; // expected-error +{{must be imported}}
+// expected-note@defs.h:42 +{{here}}
+
 J<> pre_j; // expected-error {{declaration of 'J' must be imported}}
 #ifdef IMPORT_USE_2
 // expected-error-re@-2 {{default argument of 'J' must be imported from one of {{.*}}stuff.use{{.*}}stuff.use-2}}
@@ -78,6 +85,8 @@ int post_use_dx = use_dx(post_dx);
 int post_e = E(0);
 int post_ff = F<char>().f();
 int post_fg = F<char>().g<int>();
+G::A post_ga = G::a;
+decltype(G::h) post_gh = G::h;
 J<> post_j;
 template<typename T, int N, template<typename> class K> struct J;
 J<> post_j2;
-- 
2.7.4