[clang] 7c5e4ef - Track the RequestingModule in the HeaderSearch LookupFile cache.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2023-07-21T15:49:47-07:00
New Revision: 7c5e4efb099e0badb4912467b7286938a4ed5011

URL: https://github.com/llvm/llvm-project/commit/7c5e4efb099e0badb4912467b7286938a4ed5011
DIFF: https://github.com/llvm/llvm-project/commit/7c5e4efb099e0badb4912467b7286938a4ed5011.diff

LOG: Track the RequestingModule in the HeaderSearch LookupFile cache.

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.

Reviewed By: jansvoboda11

Differential Revision: https://reviews.llvm.org/D156000

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

Modified: 
    clang/include/clang/Lex/HeaderSearch.h
    clang/lib/Lex/HeaderSearch.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index 4906fa94e8ab6e..1bd14283dc539d 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -277,6 +277,9 @@ class HeaderSearch {
 
   /// 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.
@@ -292,7 +295,9 @@ class HeaderSearch {
     /// 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;
     }

diff  --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 3200d3d1d0b6ab..a5e8b028b25ecf 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1008,7 +1008,8 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
   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;
@@ -1021,7 +1022,7 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
       // 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
@@ -1036,8 +1037,9 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
           It = search_dir_nth(Iter->second);
       }
     }
-  } else
-    CacheLookup.reset(/*NewStartIt=*/NextIt);
+  } else {
+    CacheLookup.reset(RequestingModule, /*NewStartIt=*/NextIt);
+  }
 
   SmallString<64> MappedName;
 

diff  --git a/clang/test/Modules/lookup-file-cache.cpp b/clang/test/Modules/lookup-file-cache.cpp
new file mode 100644
index 00000000000000..fada031e389aef
--- /dev/null
+++ b/clang/test/Modules/lookup-file-cache.cpp
@@ -0,0 +1,27 @@
+// 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