Track the RequestingModule in the HeaderSearch LookupFile cache.

[rsmith]

Summary

Different requesting modules can have different lookup results, so don't
cache results across modules.

Fixes a regression introduced in reviews.llvm.org/D132779.

Test case based on one provided by Jan Svoboda.

[jansvoboda11]

LGTM, thanks for the quick turnaround!

clang/include/clang/Lex/HeaderSearch.h

class HeaderSearch {
   std::string ModuleCachePath;
 
   /// All of the preprocessor-specific data about files that are
   /// included, indexed by the FileEntry's UID.
   mutable std::vector<HeaderFileInfo> FileInfo;
 
   /// Keeps track of each lookup performed by LookupFile.
   struct LookupFileCacheInfo {
+    // The requesting module for the lookup we cached.
+    const Module *RequestingModule = nullptr;
+
     /// Starting search directory iterator that the cached search was performed
     /// from. If there is a hit and this value doesn't match the current query,
     /// the cache has to be ignored.
     ConstSearchDirIterator StartIt = nullptr;
 
     /// The search directory iterator that satisfied the query.
     ConstSearchDirIterator HitIt = nullptr;
 
     /// This is non-null if the original filename was mapped to a framework
     /// include via a headermap.
     const char *MappedName = nullptr;
 
     /// Default constructor -- Initialize all members with zero.
     LookupFileCacheInfo() = default;
 
-    void reset(ConstSearchDirIterator NewStartIt) {
+    void reset(const Module *NewRequestingModule,
+               ConstSearchDirIterator NewStartIt) {
+      RequestingModule = NewRequestingModule;
       StartIt = NewStartIt;
       MappedName = nullptr;
     }
   };
   llvm::StringMap<LookupFileCacheInfo, llvm::BumpPtrAllocator> LookupFileCache;
 
   /// Collection mapping a framework or subframework
   /// name like "Carbon" to the Carbon.framework directory.

clang/lib/Lex/HeaderSearch.cpp

OptionalFileEntryRef HeaderSearch::LookupFile(
   // multiply included, and the "pragma once" optimization prevents them from
   // being relex/pp'd, but they would still have to search through a
   // (potentially huge) series of SearchDirs to find it.
   LookupFileCacheInfo &CacheLookup = LookupFileCache[Filename];
 
   ConstSearchDirIterator NextIt = std::next(It);
 
   if (!SkipCache) {
-    if (CacheLookup.StartIt == NextIt) {
+    if (CacheLookup.StartIt == NextIt &&
+        CacheLookup.RequestingModule == RequestingModule) {
       // HIT: Skip querying potentially lots of directories for this lookup.
       if (CacheLookup.HitIt)
         It = CacheLookup.HitIt;
       if (CacheLookup.MappedName) {
         Filename = CacheLookup.MappedName;
         if (IsMapped)
           *IsMapped = true;
       }
     } else {
       // MISS: This is the first query, or the previous query didn't match
       // our search start.  We will fill in our found location below, so prime
       // the start point value.
-      CacheLookup.reset(/*NewStartIt=*/NextIt);
+      CacheLookup.reset(RequestingModule, /*NewStartIt=*/NextIt);
 
       if (It == search_dir_begin() && FirstNonHeaderMapSearchDirIdx > 0) {
         // Handle cold misses of user includes in the presence of many header
         // maps.  We avoid searching perhaps thousands of header maps by
         // jumping directly to the correct one or jumping beyond all of them.
         auto Iter = SearchDirHeaderMapIndex.find(Filename.lower());
         if (Iter == SearchDirHeaderMapIndex.end())
           // Not in index => Skip to first SearchDir after initial header maps
           It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
         else
           // In index => Start with a specific header map
           It = search_dir_nth(Iter->second);
       }
     }
-  } else
-    CacheLookup.reset(/*NewStartIt=*/NextIt);
+  } else {
+    CacheLookup.reset(RequestingModule, /*NewStartIt=*/NextIt);
+  }
 
   SmallString<64> MappedName;
 
   // Check each directory in sequence to see if it contains this file.
   for (; It != search_dir_end(); ++It) {
     bool InUserSpecifiedSystemFramework = false;
     bool IsInHeaderMap = false;
     bool IsFrameworkFoundInDir = false;

clang/test/Modules/lookup-file-cache.cpp

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -I %t/include -fmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t/cache -fsyntax-only %t/tu.c
+
+//--- include/module.modulemap
+module A [no_undeclared_includes] { textual header "A.h" }
+module B { header "B.h" }
+
+//--- include/A.h
+#if __has_include(<B.h>)
+#error B.h should not be available from A.h.
+#endif
+
+//--- include/B.h
+// This file intentionally left blank.
+
+//--- tu.c
+#if !__has_include(<B.h>)
+#error B.h should be available from tu.c.
+#endif
+
+#include "A.h"
+
+#if !__has_include(<B.h>)
+#error B.h should still be available from tu.c.
+#endif