[clang] Capture Framework when HeaderSearch is resolved via headermap

[cishida]

Summary

When building frameworks, headermaps responsible for mapping angle-included headers to their source file location are passed via
-I and not -index-header-map. Also, -index-header-map is only used for indexing purposes and not during most builds.
This patch holds on to the framework's name in HeaderFileInfo as this is retrieveable for cases outside of IndexHeaderMaps and
still represents the framework that is being built.

resolves: rdar://84046893

[dexonsmith]

@jansvoboda11, can you help to review this?

[cishida]

Fix windows build.

[jansvoboda11]

LGTM.

As a follow-up, do you think it would make sense to improve the documentation/comments around "index header maps"? Variable names refer to indexing while the documentation talks about building frameworks, which is confusing without referring back to the original Radar.

[cishida]

In D111468#3066820, @jansvoboda11 wrote:

LGTM.

As a follow-up, do you think it would make sense to improve the documentation/comments around "index header maps"? Variable names refer to indexing while the documentation talks about building frameworks, which is confusing without referring back to the original Radar.

Makes sense to me. If I understand correctly, the headermap kind was introduced to work around a task generation limitation in Xcode which no longer is an issue.

clang/lib/Lex/HeaderSearch.cpp

for (unsigned j = SystemHeaderPrefixes.size(); j; --j) {
         HFI.DirInfo = SystemHeaderPrefixes[j-1].second ? SrcMgr::C_System
                                                        : SrcMgr::C_User;
         break;
       }
     }
 
     // If this file is found in a header map and uses the framework style of
     // includes, then this header is part of a framework we're building.
-    if (CurDir->isIndexHeaderMap()) {
+    if (CurDir->isHeaderMap() && isAngled) {
       size_t SlashPos = Filename.find('/');
-      if (SlashPos != StringRef::npos) {
+      if (SlashPos != StringRef::npos)
+        HFI.Framework =
+            getUniqueFrameworkName(StringRef(Filename.begin(), SlashPos));
+      if (CurDir->isIndexHeaderMap())
         HFI.IndexHeaderMapHeader = 1;
-        HFI.Framework = getUniqueFrameworkName(StringRef(Filename.begin(),
-                                                         SlashPos));
-      }
     }
 
     if (checkMSVCHeaderSearch(Diags, MSFE ? &MSFE->getFileEntry() : nullptr,
                               &File->getFileEntry(), IncludeLoc)) {
       if (SuggestedModule)
         *SuggestedModule = MSSuggestedModule;
       return MSFE;
     }

clang/unittests/Lex/HeaderSearchTest.cpp

 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace {
 
 // The test fixture.
 class HeaderSearchTest : public ::testing::Test {
 protected:
VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
                  /*Group=*/None, llvm::sys::fs::file_type::directory_file);
     auto DE = FileMgr.getOptionalDirectoryRef(Dir);
     assert(DE);
     auto DL = DirectoryLookup(*DE, SrcMgr::C_User, /*isFramework=*/false);
     Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
   void addHeaderMap(llvm::StringRef Filename,
-                    std::unique_ptr<llvm::MemoryBuffer> Buf) {
+                    std::unique_ptr<llvm::MemoryBuffer> Buf,
+                    bool isAngled = false) {
     VFS->addFile(Filename, 0, std::move(Buf), /*User=*/None, /*Group=*/None,
                  llvm::sys::fs::file_type::regular_file);
     auto FE = FileMgr.getFile(Filename, true);
     assert(FE);
 
     // Test class supports only one HMap at a time.
     assert(!HMap);
     HMap = HeaderMap::Create(*FE, FileMgr);
     auto DL =
         DirectoryLookup(HMap.get(), SrcMgr::C_User, /*isFramework=*/false);
-    Search.AddSearchPath(DL, /*isAngled=*/false);
+    Search.AddSearchPath(DL, isAngled);
   }
 
   IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS;
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
   IntrusiveRefCntPtr<DiagnosticIDs> DiagID;
   DiagnosticsEngine Diags;
   SourceManager SourceMgr;
TEST_F(HeaderSearchTest, HeaderMapReverseLookup) {
   addSearchDir("/a");
 
   EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c.h",
                                                    /*WorkingDir=*/"",
                                                    /*MainFile=*/""),
             "d.h");
 }
 
+TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
+  typedef NullTerminatedFile<test::HMapFileMock<4, 128>, char> FileTy;
+  FileTy File;
+  File.init();
+
+  std::string HeaderDirName = "/tmp/Sources/Foo/Headers/";
+  std::string HeaderName = "Foo.h";
+#ifdef _WIN32
+  // Force header path to be absolute on windows.
+  // As headermap content should represent absolute locations.
+  HeaderDirName = "C:" + HeaderDirName;
+#endif /*_WIN32*/
+
+  test::HMapFileMockMaker<FileTy> Maker(File);
+  auto a = Maker.addString("Foo/Foo.h");
+  auto b = Maker.addString(HeaderDirName);
+  auto c = Maker.addString(HeaderName);
+  Maker.addBucket("Foo/Foo.h", a, b, c);
+  addHeaderMap("product-headers.hmap", File.getBuffer(), /*isAngled=*/true);
+
+  VFS->addFile(
+      HeaderDirName + HeaderName, 0,
+      llvm::MemoryBuffer::getMemBufferCopy("", HeaderDirName + HeaderName),
+      /*User=*/None, /*Group=*/None, llvm::sys::fs::file_type::regular_file);
+
+  bool IsMapped = false;
+  const DirectoryLookup *CurDir = nullptr;
+  auto FoundFile = Search.LookupFile(
+      "Foo/Foo.h", SourceLocation(), /*isAngled=*/true, /*FromDir=*/nullptr,
+      CurDir, /*Includers=*/{}, /*SearchPath=*/nullptr,
+      /*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
+      /*SuggestedModule=*/nullptr, &IsMapped,
+      /*IsFrameworkFound=*/nullptr);
+
+  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");
+}
+
 } // namespace
 } // namespace clang