[clang] Track how headers get included generally during lookup time

[cishida]

Summary

tapi & clang-extractapi both attempt to construct then check against
how a header was included to determine api information when working
against multiple search paths, headermap, and vfsoverlay mechanisms.
Validating this against what the preprocessor sees during lookup time
makes this check more reliable.

[zixuw]

LGTM!

[jansvoboda11]

Can you describe how come the check is not reliable without this patch? It might be worth fixing the underlying reason for the unreliability first.

clang/include/clang/Lex/HeaderSearch.h

121

I think HeaderFileInfo is supposed to be very small, hence the bit packing. Adding SmallString here might negatively impact performance. See https://reviews.llvm.org/D104344#inline-1039503

Note that HeaderFileInfo used to track aliases, but does not anymore: https://reviews.llvm.org/D123885

[tschuett]

clang/lib/Lex/HeaderSearch.cpp

1040

Why did the comment move?

[cishida]

In D124638#3481932, @jansvoboda11 wrote:

Can you describe how come the check is not reliable without this patch? It might be worth fixing the underlying reason for the unreliability first.

Different search paths alter where a header path may be resolved, in these cases we attempt to match the header based on how it was included <foo/bar.h>. We determine how the header was included by assembling from HeaderFileInfo::Framework and the filename associated with the FileEntry. This becomes inaccurate if a framework header is found without a header map and theres multiple locations to find it. This happens in an Xcode configuration when you have a Framework target, so theres potentially the same named headers in

• DeviredData/.../foo.framework/Headers/bar.h
• SDKROOT/foo.framework/Headers/bar.h
• SRCROOT/foo/bar.h

and search paths for all of them (depending on order).
Given all the potential cases to cover, I think it would make sense to properly track what the include name was during lookup time as a reference-able source of truth.

Thank you for pointing out the potential performance cost. I could hold this information as apart of a container in HeaderSearch itself instead. This approach looks similar to HeaderSearch:: IncludeAliases which is only used for pragma include_alias.

[cishida]

clang/lib/Lex/HeaderSearch.cpp

1040

It refers to setting the DirInfo field, so I moved it to keep the comment near that assignment.

[cishida]

Hold include name in HeaderSearch itself, keeping HeaderFileInfo lightweight.

[jansvoboda11]

LGTM, thanks!

clang/include/clang/Lex/HeaderSearch.h

 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/DirectoryLookup.h"
 #include "clang/Lex/HeaderMap.h"
 #include "clang/Lex/ModuleMap.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Allocator.h"
 #include <cassert>
 #include <cstddef>
 #include <memory>
 #include <string>
struct HeaderFileInfo {
   /// external storage.
   const IdentifierInfo *ControllingMacro = nullptr;
 
   /// If this header came from a framework include, this is the name
   /// of the framework.
   StringRef Framework;
 
   HeaderFileInfo()
       : isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
         External(false), isModuleHeader(false), isCompilingModuleHeader(false),
         Resolved(false), IndexHeaderMapHeader(false), IsValid(false)  {}
 
   /// Retrieve the controlling macro for this header file, if
   /// any.
   const IdentifierInfo *
   getControllingMacro(ExternalPreprocessorSource *External);
 };
class HeaderSearch {
 
   /// Describes whether a given directory has a module map in it.
   llvm::DenseMap<const DirectoryEntry *, bool> DirectoryHasModuleMap;
 
   /// Set of module map files we've already loaded, and a flag indicating
   /// whether they were valid or not.
   llvm::DenseMap<const FileEntry *, bool> LoadedModuleMaps;
 
+  // A map of discovered headers with their associated include file name.
+  llvm::DenseMap<const FileEntry *, llvm::SmallString<64>> IncludeNames;
+
   /// Uniqued set of framework names, which is used to track which
   /// headers were included as framework headers.
   llvm::StringSet<llvm::BumpPtrAllocator> FrameworkNames;
 
   /// Entity used to resolve the identifier IDs of controlling
   /// macros into IdentifierInfo pointers, and keep the identifire up to date,
   /// as needed.
   ExternalPreprocessorSource *ExternalLookup = nullptr;
public:
   }
 
   /// Get the index of the given search directory.
   unsigned searchDirIdx(const DirectoryLookup &DL) const;
 
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
+  /// Retrieve the include name for the header.
+  ///
+  /// \param File The entry for a given header.
+  /// \returns The name of how the file was included when the header's location
+  /// was resolved.
+  StringRef getIncludeNameForHeader(const FileEntry *File) const;
+
   /// Suggest a path by which the specified file could be found, for use in
   /// diagnostics to suggest a #include. Returned path will only contain forward
   /// slashes as separators. MainFile is the absolute path of the file that we
   /// are generating the diagnostics for. It will try to shorten the path using
   /// MainFile location, if none of the include search directories were prefix
   /// of File.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested

clang/lib/Lex/HeaderSearch.cpp

if (IsFrameworkFound)
       // lookups, ignore IsFrameworkFoundInDir after the first remapping and not
       // just for remapping in a current search directory.
       *IsFrameworkFound |= (IsFrameworkFoundInDir && !CacheLookup.MappedName);
     if (!File)
       continue;
 
     CurDir = It;
 
+    const auto FE = &File->getFileEntry();
+    IncludeNames[FE] = Filename;
+
     // This file is a system header or C++ unfriendly if the dir is.
-    HeaderFileInfo &HFI = getFileInfo(&File->getFileEntry());
+    HeaderFileInfo &HFI = getFileInfo(FE);
     HFI.DirInfo = CurDir->getDirCharacteristic();
 
     // If the directory characteristic is User but this framework was
     // user-specified to be treated as a system framework, promote the
     // characteristic.
     if (HFI.DirInfo == SrcMgr::C_User && InUserSpecifiedSystemFramework)
       HFI.DirInfo = SrcMgr::C_System;
 
     // If the filename matches a known system header prefix, override
     // whether the file is a system header.
     for (unsigned j = SystemHeaderPrefixes.size(); j; --j) {

 unsigned HeaderSearch::searchDirIdx(const DirectoryLookup &DL) const {
   return &DL - &*SearchDirs.begin();
 }
 
 StringRef HeaderSearch::getUniqueFrameworkName(StringRef Framework) {
   return FrameworkNames.insert(Framework).first->first();
 }
 
+StringRef HeaderSearch::getIncludeNameForHeader(const FileEntry *File) const {
+  auto It = IncludeNames.find(File);
+  if (It == IncludeNames.end())
+    return {};
+  return It->second;
+}
+
 bool HeaderSearch::hasModuleMap(StringRef FileName,
                                 const DirectoryEntry *Root,
                                 bool IsSystem) {
   if (!HSOpts->ImplicitModuleMaps)
     return false;
 
   SmallVector<const DirectoryEntry *, 2> FixUpDirectories;
 

clang/unittests/Lex/HeaderSearchTest.cpp

TEST_F(HeaderSearchTest, HeaderFrameworkLookup) {
 
   EXPECT_TRUE(FoundFile.hasValue());
   EXPECT_TRUE(IsFrameworkFound);
   auto &FE = FoundFile.getValue();
   auto FI = Search.getExistingFileInfo(FE);
   EXPECT_TRUE(FI);
   EXPECT_TRUE(FI->IsValid);
   EXPECT_EQ(FI->Framework.str(), "Foo");
+  EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
 }
 
 // Helper struct with null terminator character to make MemoryBuffer happy.
 template <class FileTy, class PaddingTy>
 struct NullTerminatedFile : public FileTy {
   PaddingTy Padding = 0;
 };
 
TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
 
   EXPECT_TRUE(FoundFile.hasValue());
   EXPECT_TRUE(IsMapped);
   auto &FE = FoundFile.getValue();
   auto FI = Search.getExistingFileInfo(FE);
   EXPECT_TRUE(FI);
   EXPECT_TRUE(FI->IsValid);
   EXPECT_EQ(FI->Framework.str(), "Foo");
+  EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
 }
 
 } // namespace
 } // namespace clang