Modules: Enforce that ModuleManager::removeModules deletes the tail

ModuleManager::removeModules always deletes a tail of the
ModuleManager::Chain.  Change the API to enforce that so that we can
simplify the code inside.

There's no real functionality change, although there's a slight
performance hack to loop to the First deleted module instead of the
final module in the chain (skipping the about-to-be-deleted tail).

Also document something suspicious: we fail to clean deleted modules out
of ModuleFile::Imports.

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

diff --git a/include/clang/Serialization/ModuleManager.h b/include/clang/Serialization/ModuleManager.h
index 515222d2cba7c1fd1c7a4fe0c25a916b2260cc9f..0f4ec664864c3eae1c96893f359f28575faf6ff8 100644 (file)
--- a/include/clang/Serialization/ModuleManager.h
+++ b/include/clang/Serialization/ModuleManager.h
@@ -228,8 +228,8 @@ public:
                             ModuleFile *&Module,
                             std::string &ErrorStr);
 
-  /// \brief Remove the given set of modules.
-  void removeModules(ModuleIterator first, ModuleIterator last,
+  /// \brief Remove the modules starting from First (to the end).
+  void removeModules(ModuleIterator First,
                      llvm::SmallPtrSetImpl<ModuleFile *> &LoadedSuccessfully,
                      ModuleMap *modMap);
 

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 5cebe47052d80c35c176ae38da9ee3da0c440137..44cef7de312b9a9614078168f47ec23a42cd9378 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -3643,11 +3643,10 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
     for (const ImportedModule &IM : Loaded)
       LoadedSet.insert(IM.Mod);
 
-    ModuleMgr.removeModules(ModuleMgr.begin() + NumModules, ModuleMgr.end(),
-                            LoadedSet,
+    ModuleMgr.removeModules(ModuleMgr.begin() + NumModules, LoadedSet,
                             Context.getLangOpts().Modules
-                              ? &PP.getHeaderSearchInfo().getModuleMap()
-                              : nullptr);
+                                ? &PP.getHeaderSearchInfo().getModuleMap()
+                                : nullptr);
 
     // If we find that any modules are unusable, the global index is going
     // to be out-of-date. Just remove it.

diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp
index 5fb00883150fc572c9acad4f79e80ca3612bd78c..358dd0ec7f42b61fa62ad049f27a54e7329a50c0 100644 (file)
--- a/lib/Serialization/ModuleManager.cpp
+++ b/lib/Serialization/ModuleManager.cpp
@@ -184,32 +184,35 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
 }
 
 void ModuleManager::removeModules(
-    ModuleIterator first, ModuleIterator last,
+    ModuleIterator First,
     llvm::SmallPtrSetImpl<ModuleFile *> &LoadedSuccessfully,
     ModuleMap *modMap) {
-  if (first == last)
+  auto Last = end();
+  if (First == Last)
     return;
 
+
   // Explicitly clear VisitOrder since we might not notice it is stale.
   VisitOrder.clear();
 
   // Collect the set of module file pointers that we'll be removing.
   llvm::SmallPtrSet<ModuleFile *, 4> victimSet(
-      (llvm::pointer_iterator<ModuleIterator>(first)),
-      (llvm::pointer_iterator<ModuleIterator>(last)));
+      (llvm::pointer_iterator<ModuleIterator>(First)),
+      (llvm::pointer_iterator<ModuleIterator>(Last)));
 
   auto IsVictim = [&](ModuleFile *MF) {
     return victimSet.count(MF);
   };
   // Remove any references to the now-destroyed modules.
-  for (unsigned i = 0, n = Chain.size(); i != n; ++i) {
-    Chain[i]->ImportedBy.remove_if(IsVictim);
-  }
+  //
+  // FIXME: this should probably clean up Imports as well.
+  for (auto I = begin(); I != First; ++I)
+    I->ImportedBy.remove_if(IsVictim);
   Roots.erase(std::remove_if(Roots.begin(), Roots.end(), IsVictim),
               Roots.end());
 
   // Remove the modules from the PCH chain.
-  for (auto I = first; I != last; ++I) {
+  for (auto I = First; I != Last; ++I) {
     if (!I->isModule()) {
       PCHChain.erase(std::find(PCHChain.begin(), PCHChain.end(), &*I),
                      PCHChain.end());
@@ -218,7 +221,7 @@ void ModuleManager::removeModules(
   }
 
   // Delete the modules and erase them from the various structures.
-  for (ModuleIterator victim = first; victim != last; ++victim) {
+  for (ModuleIterator victim = First; victim != Last; ++victim) {
     Modules.erase(victim->File);
 
     if (modMap) {
@@ -236,8 +239,7 @@ void ModuleManager::removeModules(
   }
 
   // Delete the modules.
-  Chain.erase(Chain.begin() + (first - begin()),
-              Chain.begin() + (last - begin()));
+  Chain.erase(Chain.begin() + (First - begin()), Chain.end());
 }
 
 void