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

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 14 03:05:10 PDT 2016


Author: ioeric
Date: Wed Sep 14 05:05:10 2016
New Revision: 281452

URL: http://llvm.org/viewvc/llvm-project?rev=281452&view=rev
Log:
Revert "[modules] When merging one definition into another, propagate the list of re-exporting modules from the discarded definition to the retained definition."

This reverts commit r281429.

Modified:
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
    cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
    cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h
    cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=281452&r1=281451&r2=281452&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Sep 14 05:05:10 2016
@@ -308,18 +308,10 @@ class ASTContext : public RefCountedBase
   /// 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*, MergedModulesOrCanonicalDef> MergedDefModules;
+  llvm::DenseMap<NamedDecl*, llvm::TinyPtrVector<Module*>> 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
@@ -902,7 +894,6 @@ 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);
@@ -913,9 +904,7 @@ public:
     auto MergedIt = MergedDefModules.find(Def);
     if (MergedIt == MergedDefModules.end())
       return None;
-    if (auto *CanonDef = MergedIt->second.CanonicalDef)
-      return getModulesWithMergedDefinition(CanonDef);
-    return MergedIt->second.MergedModules;
+    return MergedIt->second;
   }
 
   /// Add a declaration to the list of declarations that are initialized

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=281452&r1=281451&r2=281452&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep 14 05:05:10 2016
@@ -890,59 +890,10 @@ void ASTContext::mergeDefinitionIntoModu
     if (auto *Listener = getASTMutationListener())
       Listener->RedefinedHiddenDefinition(ND, M);
 
-  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;
+  if (getLangOpts().ModulesLocalVisibility)
+    MergedDefModules[ND].push_back(M);
+  else
     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) {
@@ -950,13 +901,7 @@ void ASTContext::deduplicateMergedDefini
   if (It == MergedDefModules.end())
     return;
 
-  if (auto *CanonDef = It->second.CanonicalDef) {
-    It = MergedDefModules.find(CanonDef);
-    if (It == MergedDefModules.end())
-      return;
-  }
-
-  auto &Merged = It->second.MergedModules;
+  auto &Merged = It->second;
   llvm::DenseSet<Module*> Found;
   for (Module *&M : Merged)
     if (!Found.insert(M).second)

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=281452&r1=281451&r2=281452&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Sep 14 05:05:10 2016
@@ -3474,10 +3474,23 @@ void ASTReader::makeModuleVisible(Module
 /// 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.
-    getContext().mergeDefinitionIntoModulesOf(Def, MergedDef);
-    PendingMergedDefinitionsToDeduplicate.insert(Def);
+    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);
+    }
   }
 }
 
@@ -8608,7 +8621,7 @@ void ASTReader::finishPendingActions() {
       const FunctionDecl *Defn = nullptr;
       if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn))
         FD->setLazyBody(PB->second);
-      else if (FD != Defn)
+      else
         mergeDefinitionVisibility(const_cast<FunctionDecl*>(Defn), FD);
       continue;
     }

Modified: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h?rev=281452&r1=281451&r2=281452&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h Wed Sep 14 05:05:10 2016
@@ -4,7 +4,3 @@ 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) {}
-}

Modified: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h?rev=281452&r1=281451&r2=281452&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h Wed Sep 14 05:05:10 2016
@@ -17,6 +17,4 @@ 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) {}
 }

Modified: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h?rev=281452&r1=281451&r2=281452&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h (original)
+++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h Wed Sep 14 05:05:10 2016
@@ -5,7 +5,5 @@ 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) {}
 }
 

Modified: cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp?rev=281452&r1=281451&r2=281452&view=diff
==============================================================================
--- cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp (original)
+++ cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp Wed Sep 14 05:05:10 2016
@@ -7,8 +7,6 @@
 // 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"
@@ -17,5 +15,4 @@
 void g() {
   CrossModuleMerge::B<int> bi;
   CrossModuleMerge::C(0);
-  CrossModuleMerge::D(0);
 }




More information about the cfe-commits mailing list