r239969 - [modules] Ensure that if we merge the definitions of two enumerations, that

[Richard Smith]

Author: rsmith
Date: Wed Jun 17 18:07:50 2015
New Revision: 239969

URL: http://llvm.org/viewvc/llvm-project?rev=239969&view=rev
Log:
[modules] Ensure that if we merge the definitions of two enumerations, that
making either of them visible makes the merged definition visible.

Modified:
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/test/Modules/submodules-merge-defs.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=239969&r1=239968&r2=239969&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jun 17 18:07:50 2015
@@ -13519,7 +13519,6 @@ Sema::SkipBodyInfo Sema::shouldSkipAnonE
       !hasVisibleDefinition(cast<NamedDecl>(PrevECD->getDeclContext()),
                             &Hidden)) {
     SkipBodyInfo Skip;
-    Skip.ShouldSkip = true;
     Skip.Previous = Hidden;
     return Skip;
   }

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=239969&r1=239968&r2=239969&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Jun 17 18:07:50 2015
@@ -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(EnumDe
     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);

Modified: cfe/trunk/test/Modules/submodules-merge-defs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=239969&r1=239968&r2=239969&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original)
+++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Wed Jun 17 18:07:50 2015
@@ -48,6 +48,13 @@ int pre_ff = F<int>().f(); // expected-e
 int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}}
 // expected-note at defs.h:26 +{{here}}
 
+G::A pre_ga // expected-error +{{must be imported}}
+  = G::a; // expected-error +{{must be imported}}
+// expected-note at defs.h:40 +{{here}}
+// expected-note at defs.h:41 +{{here}}
+decltype(G::h) pre_gh = G::h; // expected-error +{{must be imported}}
+// expected-note at defs.h:42 +{{here}}
+
 J<> pre_j; // expected-error {{declaration of 'J' must be imported}}
 #ifdef IMPORT_USE_2
 // expected-error-re at -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;