[modules] When merging one definition into another, propagate the list of

re-exporting modules from the discarded definition to the retained definition.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@281429 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h
index 0a184bc2e37e765e606b09b4b9bb48e1256c550f..0bc4aa4d80fdbeea0356f8844d69615d5ba8b867 100644 (file)
--- a/include/clang/AST/ASTContext.h
+++ b/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
@@ -894,6 +902,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);
@@ -904,7 +913,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

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index 9e801b5d7ae787afd9deb7bceea6eaa90a0f7e05..e5d7ef244fbd2ad76e23b0b0d2d564a6735dec86 100644 (file)
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -890,10 +890,59 @@ void ASTContext::mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
     if (auto *Listener = getASTMutationListener())
       Listener->RedefinedHiddenDefinition(ND, M);
 
-  if (getLangOpts().ModulesLocalVisibility)
-    MergedDefModules[ND].push_back(M);
-  else
+  if (getLangOpts().ModulesLocalVisibility) {
+    auto *Merged = &MergedDefModules[ND];
+    if (auto *CanonDef = Merged->CanonicalDef)
+      Merged = &MergedDefModules[CanonDef];
+    Merged->MergedModules.push_back(M);
+  } else {
+    auto MergedIt = MergedDefModules.find(ND);
+    if (MergedIt != MergedDefModules.end() && MergedIt->second.CanonicalDef)
+      ND = MergedIt->second.CanonicalDef;
     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);
+    return;
+  }
+  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) {
@@ -901,7 +950,13 @@ void ASTContext::deduplicateMergedDefinitonsFor(NamedDecl *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)

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 9b51f3dae5275b78faf451d06b8bbd7d574de5eb..a46d2d21d2f59c90040d53e81d2168c5994c90d4 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -3474,23 +3474,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);
   }
 }
 
@@ -8621,7 +8608,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;
     }

diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/a.h b/test/Modules/Inputs/merge-template-pattern-visibility/a.h
index e6592027611c2f91d7d09dfc6ca0d7dd1a3724d5..7225110a377f66b06a01d81486832e4555a76846 100644 (file)
--- a/test/Modules/Inputs/merge-template-pattern-visibility/a.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/a.h
@@ -4,3 +4,7 @@ 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) {}
+}

diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/b.h b/test/Modules/Inputs/merge-template-pattern-visibility/b.h
index 41b52d5e6abb5149eca5fc3368f96cb6bd90abc6..7301ca722dd888d295a3a3b7e2a0d05e1fbd6c92 100644 (file)
--- a/test/Modules/Inputs/merge-template-pattern-visibility/b.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/b.h
@@ -17,4 +17,6 @@ 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) {}
 }

diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/c.h b/test/Modules/Inputs/merge-template-pattern-visibility/c.h
index db80eda1ea6d97697b20a4379362a52657576bd7..68957dbad07190fe58370a074dadfdf251c13e08 100644 (file)
--- a/test/Modules/Inputs/merge-template-pattern-visibility/c.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/c.h
@@ -5,5 +5,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) {}
 }
 

diff --git a/test/Modules/merge-template-pattern-visibility.cpp b/test/Modules/merge-template-pattern-visibility.cpp
index ec5aa26c68d834c45e57a6e119b0354c04756035..4cfac176c05be9f80a2f483f1fa7ad60fb011ff7 100644 (file)
--- a/test/Modules/merge-template-pattern-visibility.cpp
+++ b/test/Modules/merge-template-pattern-visibility.cpp
@@ -7,6 +7,8 @@
 // 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
 
 #include "b.h"
 #include "d.h"
@@ -15,4 +17,5 @@
 void g() {
   CrossModuleMerge::B<int> bi;
   CrossModuleMerge::C(0);
+  CrossModuleMerge::D(0);
 }