Reinstate r281429, reverted in r281452, with a fix for its mishandling of

compiles without -fmodules-local-submodule-visibility. Original commit message:

[modules] When merging one definition into another, propagate the list of
re-exporting modules from the discarded definition to the retained definition.

llvm-svn: 284176

clang/include/clang/AST/ASTContext.h

@@ -308,10 +308,18 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// merged into.
   llvm::DenseMap<Decl*, Decl*> MergedDecls;
 
+  /// The modules into which a definition has been merged, or a map from a
+  /// merged definition to its canonical definition. This is really a union of
+  /// a NamedDecl* and a vector of Module*.
+  struct MergedModulesOrCanonicalDef {
+    llvm::TinyPtrVector<Module*> MergedModules;
+    NamedDecl *CanonicalDef = nullptr;
+  };
+
   /// \brief A mapping from a defining declaration to a list of modules (other
   /// than the owning module of the declaration) that contain merged
   /// definitions of that entity.
-  llvm::DenseMap<NamedDecl*, llvm::TinyPtrVector<Module*>> MergedDefModules;
+  llvm::DenseMap<NamedDecl*, MergedModulesOrCanonicalDef> MergedDefModules;
 
   /// \brief Initializers for a module, in order. Each Decl will be either
   /// something that has a semantic effect on startup (such as a variable with
@@ -883,6 +891,7 @@ public:
   /// and should be visible whenever \p M is visible.
   void mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
                                  bool NotifyListeners = true);
+  void mergeDefinitionIntoModulesOf(NamedDecl *ND, NamedDecl *Other);
   /// \brief Clean up the merged definition list. Call this if you might have
   /// added duplicates into the list.
   void deduplicateMergedDefinitonsFor(NamedDecl *ND);
@@ -893,7 +902,9 @@ public:
     auto MergedIt = MergedDefModules.find(Def);
     if (MergedIt == MergedDefModules.end())
       return None;
-    return MergedIt->second;
+    if (auto *CanonDef = MergedIt->second.CanonicalDef)
+      return getModulesWithMergedDefinition(CanonDef);
+    return MergedIt->second.MergedModules;
   }
 
   /// Add a declaration to the list of declarations that are initialized

clang/lib/AST/ASTContext.cpp

@@ -888,18 +888,72 @@ void ASTContext::mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
     if (auto *Listener = getASTMutationListener())
       Listener->RedefinedHiddenDefinition(ND, M);
 
-  if (getLangOpts().ModulesLocalVisibility)
-    MergedDefModules[ND].push_back(M);
-  else
+  auto *Merged = &MergedDefModules[ND];
+  if (auto *CanonDef = Merged->CanonicalDef) {
+    ND = CanonDef;
+    Merged = &MergedDefModules[ND];
+  }
+  assert(!Merged->CanonicalDef && "canonical def not canonical");
+
+  Merged->MergedModules.push_back(M);
+
+  if (!getLangOpts().ModulesLocalVisibility)
     ND->setHidden(false);
 }
 
+void ASTContext::mergeDefinitionIntoModulesOf(NamedDecl *Def,
+                                              NamedDecl *Other) {
+  // We need to know the owning module of the merge source.
+  assert(Other->isFromASTFile() && "merge of non-imported decl not supported");
+  assert(Def != Other && "merging definition into itself");
+
+  if (!getLangOpts().ModulesLocalVisibility && !Other->isHidden())
+    Def->setHidden(false);
+  else
+    assert(Other->getImportedOwningModule() &&
+           "hidden, imported declaration has no owning module");
+
+  // Mark Def as the canonical definition of merged definition Other.
+  {
+    auto &OtherMerged = MergedDefModules[Other];
+    assert((!OtherMerged.CanonicalDef || OtherMerged.CanonicalDef == Def) &&
+           "mismatched canonical definitions for declaration");
+    OtherMerged.CanonicalDef = Def;
+  }
+
+  auto &Merged = MergedDefModules[Def];
+  // Grab this again, we potentially just invalidated our reference.
+  auto &OtherMerged = MergedDefModules[Other];
+
+  if (Module *M = Other->getImportedOwningModule())
+    Merged.MergedModules.push_back(M);
+
+  // If this definition had any others merged into it, they're now merged into
+  // the canonical definition instead.
+  if (!OtherMerged.MergedModules.empty()) {
+    assert(!Merged.CanonicalDef && "canonical definition not canonical");
+    if (Merged.MergedModules.empty())
+      Merged.MergedModules = std::move(OtherMerged.MergedModules);
+    else
+      Merged.MergedModules.insert(Merged.MergedModules.end(),
+                                  OtherMerged.MergedModules.begin(),
+                                  OtherMerged.MergedModules.end());
+    OtherMerged.MergedModules.clear();
+  }
+}
+
 void ASTContext::deduplicateMergedDefinitonsFor(NamedDecl *ND) {
   auto It = MergedDefModules.find(ND);
   if (It == MergedDefModules.end())
     return;
 
-  auto &Merged = It->second;
+  if (auto *CanonDef = It->second.CanonicalDef) {
+    It = MergedDefModules.find(CanonDef);
+    if (It == MergedDefModules.end())
+      return;
+  }
+
+  auto &Merged = It->second.MergedModules;
   llvm::DenseSet<Module*> Found;
   for (Module *&M : Merged)
     if (!Found.insert(M).second)

clang/lib/Serialization/ASTReader.cpp

@@ -3481,23 +3481,10 @@ void ASTReader::makeModuleVisible(Module *Mod,
 /// visible.
 void ASTReader::mergeDefinitionVisibility(NamedDecl *Def,
                                           NamedDecl *MergedDef) {
-  // FIXME: This doesn't correctly handle the case where MergedDef is visible
-  // in modules other than its owning module. We should instead give the
-  // ASTContext a list of merged definitions for Def.
   if (Def->isHidden()) {
     // If MergedDef is visible or becomes visible, make the definition visible.
-    if (!MergedDef->isHidden())
-      Def->Hidden = false;
-    else if (getContext().getLangOpts().ModulesLocalVisibility) {
-      getContext().mergeDefinitionIntoModule(
-          Def, MergedDef->getImportedOwningModule(),
-          /*NotifyListeners*/ false);
-      PendingMergedDefinitionsToDeduplicate.insert(Def);
-    } else {
-      auto SubmoduleID = MergedDef->getOwningModuleID();
-      assert(SubmoduleID && "hidden definition in no module");
-      HiddenNamesMap[getSubmodule(SubmoduleID)].push_back(Def);
-    }
+    getContext().mergeDefinitionIntoModulesOf(Def, MergedDef);
+    PendingMergedDefinitionsToDeduplicate.insert(Def);
   }
 }
 
@@ -8632,7 +8619,7 @@ void ASTReader::finishPendingActions() {
       const FunctionDecl *Defn = nullptr;
       if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn))
         FD->setLazyBody(PB->second);
-      else
+      else if (FD != Defn)
         mergeDefinitionVisibility(const_cast<FunctionDecl*>(Defn), FD);
       continue;
     }

clang/test/Modules/Inputs/merge-template-pattern-visibility/a.h

@@ -4,3 +4,8 @@ template<typename T> struct B;
 template<typename, typename> struct A {};
 template<typename T> struct B : A<T> {};
 template<typename T> inline auto C(T) {}
+
+namespace CrossModuleMerge {
+  template<typename T> inline auto D(T) {}
+  struct E {};
+}

clang/test/Modules/Inputs/merge-template-pattern-visibility/b.h

@@ -17,4 +17,7 @@ namespace CrossModuleMerge {
   template<typename, typename> struct A {};
   template<typename T> struct B : A<T> {};
   template<typename T> inline auto C(T) {}
+
+  template<typename T> inline auto D(T) {}
+  struct E;
 }

clang/test/Modules/Inputs/merge-template-pattern-visibility/c.h

@@ -5,5 +5,8 @@ namespace CrossModuleMerge {
   template<typename, typename> struct A {};
   template<typename T> struct B : A<T> {};
   template<typename T> inline auto C(T) {}
+
+  template<typename T> inline auto D(T) {}
+  struct E {};
 }
 

clang/test/Modules/Inputs/merge-template-pattern-visibility/d.h

@@ -1 +1,3 @@
-// d.h: empty
+namespace CrossModuleMerge {
+  struct E {};
+}

clang/test/Modules/Inputs/merge-var-template-def/a.h

@@ -0,0 +1,6 @@
+#ifndef A_H
+#define A_H
+template<typename T> struct A { static bool b; };
+template<typename T> bool A<T>::b = false;
+template<typename T> void *get() { return &(A<T>::b); }
+#endif

clang/test/Modules/Inputs/merge-var-template-def/b1.h

@@ -0,0 +1,7 @@
+#ifndef B1_H
+#define B1_H
+template<typename T> struct A { static bool b; };
+template<typename T> bool A<T>::b = false;
+template<typename T> void *get() { return &(A<T>::b); }
+#include "a.h"
+#endif

clang/test/Modules/Inputs/merge-var-template-def/b2.h

@@ -0,0 +1,6 @@
+#ifndef B2_H
+#define B2_H
+template<typename T> struct A { static bool b; };
+template<typename T> bool A<T>::b = false;
+template<typename T> void *get() { return &(A<T>::b); }
+#endif

clang/test/Modules/Inputs/merge-var-template-def/module.modulemap

@@ -0,0 +1,5 @@
+module a { header "a.h" export * }
+module b {
+  module b1 { header "b1.h" export * }
+  module b2 { header "b2.h" export * }
+}

clang/test/Modules/merge-template-pattern-visibility.cpp

@@ -7,6 +7,12 @@
 // RUN:            -fmodules-local-submodule-visibility -o %t/Y.pcm
 // RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 -fmodule-file=%t/X.pcm -fmodule-file=%t/Y.pcm \
 // RUN:            -fmodules-local-submodule-visibility -verify %s -I%S/Inputs/merge-template-pattern-visibility
+// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 -fmodule-file=%t/Y.pcm -fmodule-file=%t/X.pcm \
+// RUN:            -fmodules-local-submodule-visibility -verify %s -I%S/Inputs/merge-template-pattern-visibility
+// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 -fmodule-file=%t/X.pcm -fmodule-file=%t/Y.pcm \
+// RUN:            -verify %s -I%S/Inputs/merge-template-pattern-visibility
+// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 -fmodule-file=%t/Y.pcm -fmodule-file=%t/X.pcm \
+// RUN:            -verify %s -I%S/Inputs/merge-template-pattern-visibility
 
 #include "b.h"
 #include "d.h"
@@ -15,4 +21,6 @@
 void g() {
   CrossModuleMerge::B<int> bi;
   CrossModuleMerge::C(0);
+  CrossModuleMerge::D(0);
+  CrossModuleMerge::E e;
 }

clang/test/Modules/merge-var-template-def.cpp

@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -I%S/Inputs/merge-var-template-def -verify -fmodules -Werror=undefined-internal -fmodules-local-submodule-visibility -fmodules-cache-path=%t -fimplicit-module-maps %s
+// expected-no-diagnostics
+
+#include "b2.h"
+namespace { struct X; }
+void *x = get<X>();