[PATCH] D111468: [clang] Capture Framework when HeaderSearch is resolved via headermap

2021-10-15 Thread Cyndy Ishida via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG395e1fe30574: [clang] Capture Framework when HeaderSearch is 
resolved via headermap (authored by cishida).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111468/new/

https://reviews.llvm.org/D111468

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -18,6 +18,7 @@
 #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 {
@@ -47,7 +48,8 @@
   }
 
   void addHeaderMap(llvm::StringRef Filename,
-std::unique_ptr Buf) {
+std::unique_ptr 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);
@@ -58,7 +60,7 @@
 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 VFS;
@@ -179,5 +181,48 @@
 "d.h");
 }
 
+TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
+  typedef NullTerminatedFile, 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 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, ,
+  /*IsFrameworkFound=*/nullptr);
+
+  EXPECT_TRUE(FoundFile.hasValue());
+  EXPECT_TRUE(IsMapped);
+  auto  = FoundFile.getValue();
+  auto FI = Search.getExistingFileInfo(FE);
+  EXPECT_TRUE(FI);
+  EXPECT_TRUE(FI->IsValid);
+  EXPECT_EQ(FI->Framework.str(), "Foo");
+}
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1005,13 +1005,13 @@
 
 // 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 ? >getFileEntry() : nullptr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111468: [clang] Capture Framework when HeaderSearch is resolved via headermap

2021-10-15 Thread Cyndy Ishida via Phabricator via cfe-commits 
cishida added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111468/new/

https://reviews.llvm.org/D111468

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111468: [clang] Capture Framework when HeaderSearch is resolved via headermap

2021-10-15 Thread Jan Svoboda via Phabricator via cfe-commits 
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111468/new/

https://reviews.llvm.org/D111468

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111468: [clang] Capture Framework when HeaderSearch is resolved via headermap

2021-10-14 Thread Cyndy Ishida via Phabricator via cfe-commits 
cishida updated this revision to Diff 379721.
cishida added a comment.

Fix windows build.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111468/new/

https://reviews.llvm.org/D111468

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -18,6 +18,7 @@
 #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 {
@@ -47,7 +48,8 @@
   }
 
   void addHeaderMap(llvm::StringRef Filename,
-std::unique_ptr Buf) {
+std::unique_ptr 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);
@@ -58,7 +60,7 @@
 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 VFS;
@@ -179,5 +181,48 @@
 "d.h");
 }
 
+TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
+  typedef NullTerminatedFile, 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 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, ,
+  /*IsFrameworkFound=*/nullptr);
+
+  EXPECT_TRUE(FoundFile.hasValue());
+  EXPECT_TRUE(IsMapped);
+  auto  = FoundFile.getValue();
+  auto FI = Search.getExistingFileInfo(FE);
+  EXPECT_TRUE(FI);
+  EXPECT_TRUE(FI->IsValid);
+  EXPECT_EQ(FI->Framework.str(), "Foo");
+}
+
 } // namespace
 } // namespace clang
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1005,13 +1005,13 @@
 
 // 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 ? >getFileEntry() : nullptr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111468: [clang] Capture Framework when HeaderSearch is resolved via headermap

2021-10-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith added a reviewer: jansvoboda11.
dexonsmith added a subscriber: jansvoboda11.
dexonsmith added a comment.

@jansvoboda11, can you help to review this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111468/new/

https://reviews.llvm.org/D111468

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111468: [clang] Capture Framework when HeaderSearch is resolved via headermap

2021-10-08 Thread Cyndy Ishida via Phabricator via cfe-commits 
cishida created this revision.
cishida added reviewers: dexonsmith, ributzka.
cishida requested review of this revision.
Herald added a project: clang.

When building frameworks, headermaps responsible for mapping angle-included 
headers to their source file location are passed via
`-I` and not `-index-header-map`. However, `-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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111468

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -18,6 +18,7 @@
 #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 {
@@ -47,7 +48,8 @@
   }
 
   void addHeaderMap(llvm::StringRef Filename,
-std::unique_ptr Buf) {
+std::unique_ptr 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);
@@ -56,9 +58,10 @@
 // Test class supports only one HMap at a time.
 assert(!HMap);
 HMap = HeaderMap::Create(*FE, FileMgr);
+HMap->dump();
 auto DL =
 DirectoryLookup(HMap.get(), SrcMgr::C_User, /*isFramework=*/false);
-Search.AddSearchPath(DL, /*isAngled=*/false);
+Search.AddSearchPath(DL, isAngled);
   }
 
   IntrusiveRefCntPtr VFS;
@@ -179,5 +182,38 @@
 "d.h");
 }
 
+TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
+  typedef NullTerminatedFile, char> FileTy;
+  FileTy File;
+  File.init();
+
+  test::HMapFileMockMaker Maker(File);
+  auto a = Maker.addString("Foo/Foo.h");
+  auto b = Maker.addString("/tmp/Sources/Foo/Headers/");
+  auto c = Maker.addString("Foo.h");
+  Maker.addBucket("Foo/Foo.h", a, b, c);
+  addHeaderMap("product-headers.hmap", File.getBuffer(), /*isAngled=*/true);
+
+  StringRef HeaderName = "/tmp/Sources/Foo/Headers/Foo.h";
+  VFS->addFile(
+  HeaderName, 0, llvm::MemoryBuffer::getMemBufferCopy("", HeaderName),
+  /*User=*/None, /*Group=*/None, llvm::sys::fs::file_type::regular_file);
+
+  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=*/nullptr,
+  /*IsFrameworkFound=*/nullptr);
+
+  EXPECT_TRUE(FoundFile.hasValue());
+  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
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -964,13 +964,13 @@
 
 // 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 ? >getFileEntry() : nullptr,


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -18,6 +18,7 @@
 #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 {
@@ -47,7 +48,8 @@
   }
 
   void addHeaderMap(llvm::StringRef Filename,
-std::unique_ptr Buf) {
+std::unique_ptr Buf,
+bool isAngled = false) {
 VFS->addFile(Filename, 0,