IncludeNames reverse mapping misses some lookups
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
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> Includers,
+ SmallVectorImpl *SearchPath, SmallVectorImpl *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 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> Includers,
+ SmallVectorImpl *SearchPath, SmallVectorImpl *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!
Contributor guide
Research direction
Read clang/include/clang/Lex/HeaderSearch.h and clang/lib/Lex/HeaderSearch.cpp, starting with HeaderSearch::LookupFile and HeaderSearchInfo::getIncludeNameForHeader. Trace the successful lookup exits, especially the adjacent-header case, and compare them with the proposed experiment. Done means successful FileEntry lookups consistently provide the include name needed by IWYU.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100