| Issue |
174482
|
| Summary |
IncludeNames reverse mapping misses some lookups
|
| Labels |
new issue
|
| Assignees |
|
| Reporter |
kimgr
|
b6c67c3c67893d532fe741c508dfa2ac40fae1ad added support for recording from which include-name a `FileEntry*` was last resolved in `HeaderSearch::LookupFile`.
But it only records the reverse mapping for one of seven successful exits from the function (e.g. special casing for absolute paths, adjacent header search, `-I` header search, MSVC compat header search).
`HeaderSearchInfo::getIncludeNameForHeader` is only used [in a single place inside Clang](https://github.com/llvm/llvm-project/blob/5de2c7519c5d151bffe51359b179753d7f479523/clang/lib/Lex/HeaderSearch.cpp#L1615), but I've found it really useful in IWYU to resolve include names as they were actually used earlier, where I don't have more context available to reconstruct the include names myself.
I've now stumbled on a few scenarios where I know a file has been included, but it's not available from `HeaderSearchInfo::getIncludeNameForHeader`. For the issue I'm looking at, it appears to be in the adjacent-header case.
I've applied the following patch to force-remember all successful lookups:
```diff
Subject: [PATCH] Experiment: always add resolved file lookups to IncludeNames
---
clang/include/clang/Lex/HeaderSearch.h | 9 +++++++++
clang/lib/Lex/HeaderSearch.cpp | 21 +++++++++++++++++++--
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index 5369c872ac1c..0f1f49be4bec 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -520,6 +520,15 @@ public:
bool BuildSystemModule = false, bool OpenFile = true,
bool CacheFailures = true);
+ OptionalFileEntryRef LookupFileImpl(
+ StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
+ ConstSearchDirIterator FromDir, ConstSearchDirIterator *CurDir,
+ ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers,
+ SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
+ Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
+ bool *IsMapped, bool *IsFrameworkFound, bool SkipCache,
+ bool BuildSystemModule, bool OpenFile, bool CacheFailures);
+
/// Look up a subframework for the specified \#include file.
///
/// For example, if \#include'ing <HIToolbox/HIToolbox.h> from
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index b2ed24f765da..a22f51019532 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -954,6 +954,25 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
bool *IsMapped, bool *IsFrameworkFound, bool SkipCache,
bool BuildSystemModule, bool OpenFile, bool CacheFailures) {
+ OptionalFileEntryRef FE = LookupFileImpl(
+ Filename, IncludeLoc, isAngled, FromDir, CurDirArg, Includers, SearchPath,
+ RelativePath, RequestingModule, SuggestedModule, IsMapped,
+ IsFrameworkFound, SkipCache, BuildSystemModule, OpenFile, CacheFailures);
+ if (FE) {
+ IncludeNames[*FE] = Filename;
+ }
+ return FE;
+}
+
+/// See LookupFile.
+OptionalFileEntryRef HeaderSearch::LookupFileImpl(
+ StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
+ ConstSearchDirIterator FromDir, ConstSearchDirIterator *CurDirArg,
+ ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers,
+ SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
+ Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
+ bool *IsMapped, bool *IsFrameworkFound, bool SkipCache,
+ bool BuildSystemModule, bool OpenFile, bool CacheFailures) {
ConstSearchDirIterator CurDirLocal = nullptr;
ConstSearchDirIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
@@ -1166,8 +1185,6 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
CurDir = It;
- IncludeNames[*File] = Filename;
-
// This file is a system header or C++ unfriendly if the dir is.
HeaderFileInfo &HFI = getFileInfo(*File);
HFI.DirInfo = CurDir->getDirCharacteristic();
--
2.43.0
```
... and my IWYU use cases now work as expected. Not saying this is the right patch, but it proves the point without making too many changes to the complex `LookupFile` function.
@cyndyishida @zixu-w @jansvoboda11 You were involved in the original patch linked above -- do you have any more context on what might be a good way to address this? Thanks!
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
