[clang] Make access to submodules via `iterator_range`

[Stoorx]

Summary

In file clang/lib/Basic/Module.cpp the Module class had submodule_begin() and submodule_end() functions to retrieve corresponding iterators for private vector of Modules. This commit removes mentioned functions, and replaces all of theirs usages with submodules() function and range-based for-loops.

[Stoorx]

Rebase

[phosek]

LGTM

[Stoorx]

Can you commit it? I have no commit access.

Stoorx
me[at]stoorx.one

[barannikov88]

clang/include/clang/Basic/Module.h

744–747

There was no need to delete these, they were useful (e.g. the change in Sema.cpp).
Well, I guess I'm late.

clang-tools-extra/modularize/CoverageChecker.cpp

if (!collectUmbrellaHeaders(UmbrellaDir->getName()))
       return false;
   }
 
   for (auto &HeaderKind : Mod.Headers)
     for (auto &Header : HeaderKind)
       ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(
         Header.Entry->getName()));
 
-  for (auto MI = Mod.submodule_begin(), MIEnd = Mod.submodule_end();
-       MI != MIEnd; ++MI)
-    collectModuleHeaders(**MI);
+  for (auto *Submodule : Mod.submodules())
+    collectModuleHeaders(*Submodule);
 
   return true;
 }
 
 // Collect headers from an umbrella directory.
 bool CoverageChecker::collectUmbrellaHeaders(StringRef UmbrellaDirName) {
   // Initialize directory name.
   SmallString<256> Directory(ModuleMapDirectory);

clang-tools-extra/modularize/ModularizeUtilities.cpp

bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
   // we can't know.
   if (Mod.IsExplicit)
     return true;
 
   // Treat headers in umbrella directory as dependencies.
   DependentsVector UmbrellaDependents;
 
   // Recursively do submodules.
-  for (auto MI = Mod.submodule_begin(), MIEnd = Mod.submodule_end();
-       MI != MIEnd; ++MI)
-    collectModuleHeaders(**MI);
+  for (auto *Submodule : Mod.submodules())
+    collectModuleHeaders(*Submodule);
 
   if (const FileEntry *UmbrellaHeader = Mod.getUmbrellaHeader().Entry) {
     std::string HeaderPath = getCanonicalPath(UmbrellaHeader->getName());
     // Collect umbrella header.
     HeaderFileNames.push_back(HeaderPath);
 
     // FUTURE: When needed, umbrella header header collection goes here.
   }

clang/include/clang/Basic/Module.h

bool isModuleVisible(const Module *M) const {
     return VisibleModulesCache.count(M);
   }
 
   unsigned getVisibilityID() const { return VisibilityID; }
 
   using submodule_iterator = std::vector<Module *>::iterator;
   using submodule_const_iterator = std::vector<Module *>::const_iterator;
 
-  submodule_iterator submodule_begin() { return SubModules.begin(); }
-  submodule_const_iterator submodule_begin() const {return SubModules.begin();}
-  submodule_iterator submodule_end()   { return SubModules.end(); }
-  submodule_const_iterator submodule_end() const { return SubModules.end(); }
-
   llvm::iterator_range<submodule_iterator> submodules() {
-    return llvm::make_range(submodule_begin(), submodule_end());
+    return llvm::make_range(SubModules.begin(), SubModules.end());
   }
   llvm::iterator_range<submodule_const_iterator> submodules() const {
-    return llvm::make_range(submodule_begin(), submodule_end());
+    return llvm::make_range(SubModules.begin(), SubModules.end());
   }
 
   /// Appends this module's list of exported modules to \p Exported.
   ///
   /// This provides a subset of immediately imported modules (the ones that are
   /// directly exported), not the complete set of exported modules.
   void getExportedModules(SmallVectorImpl<Module *> &Exported) const;
 

clang/lib/Basic/Module.cpp

if (Parent) {
     ModuleMapIsPrivate = Parent->ModuleMapIsPrivate;
 
     Parent->SubModuleIndex[Name] = Parent->SubModules.size();
     Parent->SubModules.push_back(this);
   }
 }
 
 Module::~Module() {
-  for (submodule_iterator I = submodule_begin(), IEnd = submodule_end();
-       I != IEnd; ++I) {
-    delete *I;
+  for (auto *Submodule : SubModules) {
+    delete Submodule;
   }
 }
 
 static bool isPlatformEnvironment(const TargetInfo &Target, StringRef Feature) {
   StringRef Platform = Target.getPlatformName();
   StringRef Env = Target.getTriple().getEnvironmentName();
 
   // Attempt to match platform and environment.
while (!Stack.empty()) {
     Module *Current = Stack.back();
     Stack.pop_back();
 
     if (!needUpdate(Current))
       continue;
 
     Current->IsAvailable = false;
     Current->IsUnimportable |= Unimportable;
-    for (submodule_iterator Sub = Current->submodule_begin(),
-                         SubEnd = Current->submodule_end();
-         Sub != SubEnd; ++Sub) {
-      if (needUpdate(*Sub))
-        Stack.push_back(*Sub);
+    for (auto *Submodule : Current->submodules()) {
+      if (needUpdate(Submodule))
+        Stack.push_back(Submodule);
     }
   }
 }
 
 Module *Module::findSubmodule(StringRef Name) const {
   llvm::StringMap<unsigned>::const_iterator Pos = SubModuleIndex.find(Name);
   if (Pos == SubModuleIndex.end())
     return nullptr;
for (auto *Unresolved : {&UnresolvedHeaders, &MissingHeaders}) {
     }
   }
 
   if (!ExportAsModule.empty()) {
     OS.indent(Indent + 2);
     OS << "export_as" << ExportAsModule << "\n";
   }
 
-  for (submodule_const_iterator MI = submodule_begin(), MIEnd = submodule_end();
-       MI != MIEnd; ++MI)
+  for (auto *Submodule : submodules())
     // Print inferred subframework modules so that we don't need to re-infer
     // them (requires expensive directory iteration + stat calls) when we build
     // the module. Regular inferred submodules are OK, as we need to look at all
     // those header files anyway.
-    if (!(*MI)->IsInferred || (*MI)->IsFramework)
-      (*MI)->print(OS, Indent + 2, Dump);
+    if (!Submodule->IsInferred || Submodule->IsFramework)
+      Submodule->print(OS, Indent + 2, Dump);
 
   for (unsigned I = 0, N = Exports.size(); I != N; ++I) {
     OS.indent(Indent + 2);
     OS << "export ";
     if (Module *Restriction = Exports[I].getPointer()) {
       OS << Restriction->getFullModuleName(true);
       if (Exports[I].getInt())
         OS << ".*";

clang/lib/CodeGen/CodeGenModule.cpp

while (!Stack.empty()) {
       clang::Module *Mod = Stack.pop_back_val();
       if (!EmittedModuleInitializers.insert(Mod).second)
         continue;
 
       for (auto *D : Context.getModuleInitializers(Mod))
         EmitTopLevelDecl(D);
 
       // Visit the submodules of this module.
-      for (clang::Module::submodule_iterator Sub = Mod->submodule_begin(),
-                                             SubEnd = Mod->submodule_end();
-           Sub != SubEnd; ++Sub) {
+      for (auto *Submodule : Mod->submodules()) {
         // Skip explicit children; they need to be explicitly imported to emit
         // the initializers.
-        if ((*Sub)->IsExplicit)
+        if (Submodule->IsExplicit)
           continue;
 
-        if (Visited.insert(*Sub).second)
-          Stack.push_back(*Sub);
+        if (Visited.insert(Submodule).second)
+          Stack.push_back(Submodule);
       }
     }
     break;
   }
 
   case Decl::Export:
     EmitDeclContext(cast<ExportDecl>(D));
     break;

clang/lib/Frontend/CompilerInstance.cpp

for (const std::string &LoadedModule : LoadedModules) {
         // Mark module as available if the only reason it was unavailable
         // was missing headers.
         SmallVector<Module *, 2> Stack;
         Stack.push_back(M);
         while (!Stack.empty()) {
           Module *Current = Stack.pop_back_val();
           if (Current->IsUnimportable) continue;
           Current->IsAvailable = true;
-          Stack.insert(Stack.end(),
-                       Current->submodule_begin(), Current->submodule_end());
+          auto SubmodulesRange = Current->submodules();
+          Stack.insert(Stack.end(), SubmodulesRange.begin(),
+                       SubmodulesRange.end());
         }
       }
     }
     LoadedModules.clear();
   }
 };
 } // namespace
 

clang/lib/Frontend/FrontendAction.cpp

if (Module::Header UmbrellaHeader = Module->getUmbrellaHeader()) {
     for (auto &H : Headers) {
       // Include this header as part of the umbrella directory.
       Module->addTopHeader(H.second);
       addHeaderInclude(H.first, Includes, LangOpts, Module->IsExternC);
     }
   }
 
   // Recurse into submodules.
-  for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
-                                      SubEnd = Module->submodule_end();
-       Sub != SubEnd; ++Sub)
+  for (auto *Submodule : Module->submodules())
     if (std::error_code Err = collectModuleHeaderIncludes(
-            LangOpts, FileMgr, Diag, ModMap, *Sub, Includes))
+            LangOpts, FileMgr, Diag, ModMap, Submodule, Includes))
       return Err;
 
   return std::error_code();
 }
 
 static bool loadModuleMapForModuleBuild(CompilerInstance &CI, bool IsSystem,
                                         bool IsPreprocessed,
                                         std::string &PresumedModuleMapFile,

clang/lib/Sema/Sema.cpp

if (Module *CurrentModule = PP.getCurrentModule()) {
         // FIXME: Actually complain, once we figure out how to teach the
         // diagnostic client to deal with complaints in the module map at this
         // point.
         ModMap.resolveExports(Mod, /*Complain=*/false);
         ModMap.resolveUses(Mod, /*Complain=*/false);
         ModMap.resolveConflicts(Mod, /*Complain=*/false);
 
         // Queue the submodules, so their exports will also be resolved.
-        Stack.append(Mod->submodule_begin(), Mod->submodule_end());
+        auto SubmodulesRange = Mod->submodules();
+        Stack.append(SubmodulesRange.begin(), SubmodulesRange.end());
       }
     }
 
     // Warnings emitted in ActOnEndOfTranslationUnit() should be emitted for
     // modules when they are built, not every time they are used.
     emitAndClearUnusedLocalTypedefWarnings();
   }
 

clang/lib/Sema/SemaCodeComplete.cpp

if (Path.empty()) {
     }
   } else if (getLangOpts().Modules) {
     // Load the named module.
     Module *Mod =
         PP.getModuleLoader().loadModule(ImportLoc, Path, Module::AllVisible,
                                         /*IsInclusionDirective=*/false);
     // Enumerate submodules.
     if (Mod) {
-      for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-                                      SubEnd = Mod->submodule_end();
-           Sub != SubEnd; ++Sub) {
-
+      for (auto *Submodule : Mod->submodules()) {
         Builder.AddTypedTextChunk(
-            Builder.getAllocator().CopyString((*Sub)->Name));
+            Builder.getAllocator().CopyString(Submodule->Name));
         Results.AddResult(Result(
             Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-            (*Sub)->isAvailable() ? CXAvailability_Available
+            Submodule->isAvailable() ? CXAvailability_Available
                                      : CXAvailability_NotAvailable));
       }
     }
   }
   Results.ExitScope();
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size());
 }
 

clang/lib/Serialization/ASTWriter.cpp

while (!Worklist.empty()) {
         SmallString<128> Filename(M->Directory->getName());
         llvm::sys::path::append(Filename, U.FileName);
         PreparePathForOutput(Filename);
 
         StringRef FilenameDup = strdup(Filename.c_str());
         SavedStrings.push_back(FilenameDup.data());
 
         HeaderFileInfoTrait::key_type Key = {
-          FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-        };
+            FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
         HeaderFileInfoTrait::data_type Data = {
-          Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-        };
+            Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
         // FIXME: Deal with cases where there are multiple unresolved header
         // directives in different submodules for the same header.
         Generator.insert(Key, Data, GeneratorTrait);
         ++NumHeaderSearchEntries;
       }
-
-      Worklist.append(M->submodule_begin(), M->submodule_end());
+      auto SubmodulesRange = M->submodules();
+      Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
     }
   }
 
   SmallVector<const FileEntry *, 16> FilesByUID;
   HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
 
   if (FilesByUID.size() > HS.header_file_size())
     FilesByUID.resize(HS.header_file_size());
unsigned ASTWriter::getSubmoduleID(Module *Mod) {
   //        "asked for module ID for non-local, non-imported module");
   return ID;
 }
 
 /// Compute the number of modules within the given tree (including the
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-       Sub != SubEnd; ++Sub)
-    ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+    ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
 
 void ASTWriter::WriteSubmodules(Module *WritingModule) {
   // Enter the submodule description block.
   Stream.EnterSubblock(SUBMODULE_BLOCK_ID, /*bits for abbreviations*/5);
 

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   return MD.ID;
 }
 
 static void forEachSubmoduleSorted(const Module *M,
                                    llvm::function_ref<void(const Module *)> F) {
   // Submodule order depends on order of header includes for inferred submodules
   // we don't care about the exact order, so sort so that it's consistent across
   // TUs to improve sharing.
-  SmallVector<const Module *> Submodules(M->submodule_begin(),
-                                         M->submodule_end());
+  SmallVector<const Module *> Submodules(M->submodules());
   llvm::stable_sort(Submodules, [](const Module *A, const Module *B) {
     return A->Name < B->Name;
   });
   for (const Module *SubM : Submodules)
     F(SubM);
 }
 
 void ModuleDepCollectorPP::addAllSubmodulePrebuiltDeps(