rsmith created this revision.
rsmith added a reviewer: jansvoboda11.
Herald added a project: All.
rsmith requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Different requesting modules can have different lookup results, so don't
cache results across modules.
Fixes a regression introduced in reviews.llvm.org/D132779
<https://reviews.llvm.org/D132779>.
Test case based on one provided by Jan Svoboda.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D156000
Files:
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/lookup-file-cache.cpp
Index: clang/test/Modules/lookup-file-cache.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1008,7 +1008,8 @@
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 @@
// 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 @@
It = search_dir_nth(Iter->second);
}
}
- } else
- CacheLookup.reset(/*NewStartIt=*/NextIt);
+ } else {
+ CacheLookup.reset(RequestingModule, /*NewStartIt=*/NextIt);
+ }
SmallString<64> MappedName;
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -277,6 +277,9 @@
/// 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 @@
/// 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;
}
Index: clang/test/Modules/lookup-file-cache.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1008,7 +1008,8 @@
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 @@
// 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 @@
It = search_dir_nth(Iter->second);
}
}
- } else
- CacheLookup.reset(/*NewStartIt=*/NextIt);
+ } else {
+ CacheLookup.reset(RequestingModule, /*NewStartIt=*/NextIt);
+ }
SmallString<64> MappedName;
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -277,6 +277,9 @@
/// 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 @@
/// 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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
