[clang] Improve laziness of resolving module map headers.

[adamcz]

Summary

clang has support for lazy headers in module maps - if size and/or
modtime and provided in the cppmap file, headers are only resolved when
an include directive for a file with that size/modtime is encoutered.

Before this change, the lazy resolution was all-or-nothing per module.
That means as soon as even one file in that module potentially matched
an include, all lazy files in that module were resolved. With this
change, only files with matching size/modtime will be resolved.

The goal is to avoid unnecessary stat() calls on non-included files,
which is especially valuable on networked file systems, with higher
latency.

[kadircet]

Thanks, this LGTM. but I am afraid all users of modules/modulemaps might not be aware of the fact that a module might be "partially" resolved (or still unresolved after a resolve operation, ASTWriter is a good example of an aware user).

For example there's Module::isAvailable that checks MissingHeaders in a module for unavailability. I am not sure if it's possible to get there without ever fully resolving a Module, but I couldn't prove otherwise either.
So this might cause surprising breakages or hide some diagnostics.

It would be great if you could test the change against LLVM or some other codebases with module enabled builds to see nothing crashes. Also we should probably be on the watch out after landing this for possible complaints from outside.

clang/include/clang/Lex/ModuleMap.h

public:
   findResolvedModulesForHeader(const FileEntry *File) const;
 
   /// Resolve all lazy header directives for the specified file.
   ///
   /// This ensures that the HeaderFileInfo on HeaderSearch is up to date. This
   /// is effectively internal, but is exposed so HeaderSearch can call it.
   void resolveHeaderDirectives(const FileEntry *File) const;
 
-  /// Resolve all lazy header directives for the specified module.
-  void resolveHeaderDirectives(Module *Mod) const;
+  /// Resolve lazy header directives for the specified module. If File is
+  /// provided, only headers with same size and modtime are resolved. If File
+  /// is not set, all headers are resolved.
+  void resolveHeaderDirectives(Module *Mod,
+                               llvm::Optional<const FileEntry *> File) const;
 
   /// Reports errors if a module must not include a specific file.
   ///
   /// \param RequestingModule The module including a file.
   ///
   /// \param RequestingModuleIsModuleInterface \c true if the inclusion is in
   ///        the interface of RequestingModule, \c false if it's in the
   ///        implementation of RequestingModule. Value is ignored and

clang/lib/Frontend/FrontendAction.cpp

 static std::error_code collectModuleHeaderIncludes(
     const LangOptions &LangOpts, FileManager &FileMgr, DiagnosticsEngine &Diag,
     ModuleMap &ModMap, clang::Module *Module, SmallVectorImpl<char> &Includes) {
   // Don't collect any headers for unavailable modules.
   if (!Module->isAvailable())
     return std::error_code();
 
   // Resolve all lazy header directives to header files.
-  ModMap.resolveHeaderDirectives(Module);
+  ModMap.resolveHeaderDirectives(Module, /*File=*/llvm::None);
 
   // If any headers are missing, we can't build this module. In most cases,
   // diagnostics for this should have already been produced; we only get here
   // if explicit stat information was provided.
   // FIXME: If the name resolves to a file with different stat information,
   // produce a better diagnostic.
   if (!Module->MissingHeaders.empty()) {
     auto &MissingHeader = Module->MissingHeaders.front();

clang/lib/Lex/ModuleMap.cpp

void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
                                         const FileEntry *File) {
   // No errors for indirect modules. This may be a bit of a problem for modules
   // with no source files.
   if (getTopLevelOrNull(RequestingModule) != getTopLevelOrNull(SourceModule))
     return;
 
   if (RequestingModule) {
     resolveUses(RequestingModule, /*Complain=*/false);
-    resolveHeaderDirectives(RequestingModule);
+    resolveHeaderDirectives(RequestingModule, /*File=*/llvm::None);
   }
 
   bool Excluded = false;
   Module *Private = nullptr;
   Module *NotUsed = nullptr;
 
   HeadersMap::iterator Known = findKnownHeader(File);
   if (Known != Headers.end()) {
void ModuleMap::addUnresolvedHeader(Module *Mod,
   // Perform the lookup now.
   resolveHeader(Mod, Header, NeedsFramework);
 }
 
 void ModuleMap::resolveHeaderDirectives(const FileEntry *File) const {
   auto BySize = LazyHeadersBySize.find(File->getSize());
   if (BySize != LazyHeadersBySize.end()) {
     for (auto *M : BySize->second)
-      resolveHeaderDirectives(M);
+      resolveHeaderDirectives(M, File);
     LazyHeadersBySize.erase(BySize);
   }
 
   auto ByModTime = LazyHeadersByModTime.find(File->getModificationTime());
   if (ByModTime != LazyHeadersByModTime.end()) {
     for (auto *M : ByModTime->second)
-      resolveHeaderDirectives(M);
+      resolveHeaderDirectives(M, File);
     LazyHeadersByModTime.erase(ByModTime);
   }
 }
 
-void ModuleMap::resolveHeaderDirectives(Module *Mod) const {
+void ModuleMap::resolveHeaderDirectives(
+    Module *Mod, llvm::Optional<const FileEntry *> File) const {
   bool NeedsFramework = false;
-  for (auto &Header : Mod->UnresolvedHeaders)
+  SmallVector<Module::UnresolvedHeaderDirective, 1> NewHeaders;
+  const auto Size = File ? File.getValue()->getSize() : 0;
+  const auto ModTime = File ? File.getValue()->getModificationTime() : 0;
+
+  for (auto &Header : Mod->UnresolvedHeaders) {
+    if (File && ((Header.ModTime && Header.ModTime != ModTime) ||
+                 (Header.Size && Header.Size != Size)))
+      NewHeaders.push_back(Header);
+    else
       // This operation is logically const; we're just changing how we represent
       // the header information for this file.
-    const_cast<ModuleMap*>(this)->resolveHeader(Mod, Header, NeedsFramework);
-  Mod->UnresolvedHeaders.clear();
+      const_cast<ModuleMap *>(this)->resolveHeader(Mod, Header, NeedsFramework);
+  }
+  Mod->UnresolvedHeaders.swap(NewHeaders);
 }
 
 void ModuleMap::addHeader(Module *Mod, Module::Header Header,
                           ModuleHeaderRole Role, bool Imported) {
   KnownHeader KH(Mod, Role);
 
   // Only add each header to the headers list once.
   // FIXME: Should we diagnose if a header is listed twice in the

clang/lib/Serialization/ASTWriter.cpp

while (!Worklist.empty()) {
       if (M->isUnimportable())
         continue;
 
       // Map to disk files where possible, to pick up any missing stat
       // information. This also means we don't need to check the unresolved
       // headers list when emitting resolved headers in the first loop below.
       // FIXME: It'd be preferable to avoid doing this if we were given
       // sufficient stat information in the module map.
-      HS.getModuleMap().resolveHeaderDirectives(M);
+      HS.getModuleMap().resolveHeaderDirectives(M, /*File=*/llvm::None);
 
       // If the file didn't exist, we can still create a module if we were given
       // enough information in the module map.
       for (auto U : M->MissingHeaders) {
         // Check that we were given enough information to build a module
         // without this file existing on disk.
         if (!U.Size || (!U.ModTime && IncludeTimestamps)) {
           PP->Diag(U.FileNameLoc, diag::err_module_no_size_mtime_for_header)