[clang][lex] Remove HeaderFileInfo::Framework

This reduces the size of `HeaderFileInfo` from 32 to 16 bytes.

Author: jansvoboda11

clang-tools-extra/clangd/index/SymbolCollector.cpp

@@ -335,9 +335,10 @@ class SymbolCollector::HeaderFileURICache {
   }
 
   struct FrameworkHeaderPath {
-    // Path to the framework directory containing the Headers/PrivateHeaders
-    // directories  e.g. /Frameworks/Foundation.framework/
-    llvm::StringRef HeadersParentDir;
+    // Path to the frameworks directory containing the .framework directory.
+    llvm::StringRef FrameworkParentDir;
+    // Name of the framework.
+    llvm::StringRef FrameworkName;
     // Subpath relative to the Headers or PrivateHeaders dir, e.g. NSObject.h
     // Note: This is NOT relative to the `HeadersParentDir`.
     llvm::StringRef HeaderSubpath;
@@ -351,19 +352,17 @@ class SymbolCollector::HeaderFileURICache {
     path::reverse_iterator I = path::rbegin(Path);
     path::reverse_iterator Prev = I;
     path::reverse_iterator E = path::rend(Path);
+    FrameworkHeaderPath HeaderPath;
     while (I != E) {
-      if (*I == "Headers") {
-        FrameworkHeaderPath HeaderPath;
-        HeaderPath.HeadersParentDir = Path.substr(0, I - E);
+      if (*I == "Headers" || *I == "PrivateHeaders") {
         HeaderPath.HeaderSubpath = Path.substr(Prev - E);
-        HeaderPath.IsPrivateHeader = false;
-        return HeaderPath;
-      }
-      if (*I == "PrivateHeaders") {
-        FrameworkHeaderPath HeaderPath;
-        HeaderPath.HeadersParentDir = Path.substr(0, I - E);
-        HeaderPath.HeaderSubpath = Path.substr(Prev - E);
-        HeaderPath.IsPrivateHeader = true;
+        HeaderPath.IsPrivateHeader = *I == "PrivateHeaders";
+        if (++I == E)
+          break;
+        HeaderPath.FrameworkName = *I;
+        if (!HeaderPath.FrameworkName.consume_back(".framework"))
+          break;
+        HeaderPath.FrameworkParentDir = Path.substr(0, I - E);
         return HeaderPath;
       }
       Prev = I;
@@ -379,26 +378,27 @@ class SymbolCollector::HeaderFileURICache {
   // <Foundation/NSObject_Private.h> which should be used instead of directly
   // importing the header.
   std::optional<std::string>
-  getFrameworkUmbrellaSpelling(llvm::StringRef Framework,
-                               const HeaderSearch &HS,
+  getFrameworkUmbrellaSpelling(const HeaderSearch &HS,
                                FrameworkHeaderPath &HeaderPath) {
+    StringRef Framework = HeaderPath.FrameworkName;
     auto Res = CacheFrameworkToUmbrellaHeaderSpelling.try_emplace(Framework);
     auto *CachedSpelling = &Res.first->second;
     if (!Res.second) {
       return HeaderPath.IsPrivateHeader ? CachedSpelling->PrivateHeader
                                         : CachedSpelling->PublicHeader;
     }
-    SmallString<256> UmbrellaPath(HeaderPath.HeadersParentDir);
-    llvm::sys::path::append(UmbrellaPath, "Headers", Framework + ".h");
+    SmallString<256> UmbrellaPath(HeaderPath.FrameworkParentDir);
+    llvm::sys::path::append(UmbrellaPath, Framework + ".framework", "Headers",
+                            Framework + ".h");
 
     llvm::vfs::Status Status;
     auto StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status);
     if (!StatErr)
       CachedSpelling->PublicHeader = llvm::formatv("<{0}/{0}.h>", Framework);
 
-    UmbrellaPath = HeaderPath.HeadersParentDir;
-    llvm::sys::path::append(UmbrellaPath, "PrivateHeaders",
-                            Framework + "_Private.h");
+    UmbrellaPath = HeaderPath.FrameworkParentDir;
+    llvm::sys::path::append(UmbrellaPath, Framework + ".framework",
+                            "PrivateHeaders", Framework + "_Private.h");
 
     StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status);
     if (!StatErr)
@@ -414,8 +414,7 @@ class SymbolCollector::HeaderFileURICache {
   // give <Foundation/Foundation.h> if the umbrella header exists, otherwise
   // <Foundation/NSObject.h>.
   std::optional<llvm::StringRef>
-  getFrameworkHeaderIncludeSpelling(FileEntryRef FE, llvm::StringRef Framework,
-                                    HeaderSearch &HS) {
+  getFrameworkHeaderIncludeSpelling(FileEntryRef FE, HeaderSearch &HS) {
     auto Res = CachePathToFrameworkSpelling.try_emplace(FE.getName());
     auto *CachedHeaderSpelling = &Res.first->second;
     if (!Res.second)
@@ -429,13 +428,15 @@ class SymbolCollector::HeaderFileURICache {
       return std::nullopt;
     }
     if (auto UmbrellaSpelling =
-            getFrameworkUmbrellaSpelling(Framework, HS, *HeaderPath)) {
+            getFrameworkUmbrellaSpelling(HS, *HeaderPath)) {
       *CachedHeaderSpelling = *UmbrellaSpelling;
       return llvm::StringRef(*CachedHeaderSpelling);
     }
 
     *CachedHeaderSpelling =
-        llvm::formatv("<{0}/{1}>", Framework, HeaderPath->HeaderSubpath).str();
+        llvm::formatv("<{0}/{1}>", HeaderPath->FrameworkName,
+                      HeaderPath->HeaderSubpath)
+            .str();
     return llvm::StringRef(*CachedHeaderSpelling);
   }
 
@@ -454,11 +455,8 @@ class SymbolCollector::HeaderFileURICache {
     // Framework headers are spelled as <FrameworkName/Foo.h>, not
     // "path/FrameworkName.framework/Headers/Foo.h".
     auto &HS = PP->getHeaderSearchInfo();
-    if (const auto *HFI = HS.getExistingFileInfo(*FE))
-      if (!HFI->Framework.empty())
-        if (auto Spelling =
-                getFrameworkHeaderIncludeSpelling(*FE, HFI->Framework, HS))
-          return *Spelling;
+    if (auto Spelling = getFrameworkHeaderIncludeSpelling(*FE, HS))
+      return *Spelling;
 
     if (!tooling::isSelfContainedHeader(*FE, PP->getSourceManager(),
                                         PP->getHeaderSearchInfo())) {

clang/include/clang/Lex/HeaderSearch.h

@@ -122,10 +122,6 @@ struct HeaderFileInfo {
   /// external storage.
   LazyIdentifierInfoPtr LazyControllingMacro;
 
-  /// If this header came from a framework include, this is the name
-  /// of the framework.
-  StringRef Framework;
-
   HeaderFileInfo()
       : IsLocallyIncluded(false), isImport(false), isPragmaOnce(false),
         DirInfo(SrcMgr::C_User), External(false), isModuleHeader(false),
@@ -144,6 +140,8 @@ struct HeaderFileInfo {
   void mergeModuleMembership(ModuleMap::ModuleHeaderRole Role);
 };
 
+static_assert(sizeof(HeaderFileInfo) <= 16);
+
 /// An external source of header file information, which may supply
 /// information about header files already included.
 class ExternalHeaderFileInfoSource {

clang/lib/Lex/HeaderSearch.cpp

@@ -974,11 +974,9 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
         const HeaderFileInfo *FromHFI = getExistingFileInfo(*Includer);
         assert(FromHFI && "includer without file info");
         unsigned DirInfo = FromHFI->DirInfo;
-        StringRef Framework = FromHFI->Framework;
 
         HeaderFileInfo &ToHFI = getFileInfo(*FE);
         ToHFI.DirInfo = DirInfo;
-        ToHFI.Framework = Framework;
 
         if (SearchPath) {
           StringRef SearchPathRef(IncluderAndDir.second.getName());
@@ -1120,16 +1118,6 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
       }
     }
 
-    // Set the `Framework` info if this file is in a header map with framework
-    // style include spelling or found in a framework dir. The header map case
-    // is possible when building frameworks which use header maps.
-    if ((CurDir->isHeaderMap() && isAngled) || CurDir->isFramework()) {
-      size_t SlashPos = Filename.find('/');
-      if (SlashPos != StringRef::npos)
-        HFI.Framework =
-            getUniqueFrameworkName(StringRef(Filename.begin(), SlashPos));
-    }
-
     if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(), IncludeLoc)) {
       if (SuggestedModule)
         *SuggestedModule = MSSuggestedModule;
@@ -1314,9 +1302,6 @@ static void mergeHeaderFileInfo(HeaderFileInfo &HFI,
   HFI.DirInfo = OtherHFI.DirInfo;
   HFI.External = (!HFI.IsValid || HFI.External);
   HFI.IsValid = true;
-
-  if (HFI.Framework.empty())
-    HFI.Framework = OtherHFI.Framework;
 }
 
 HeaderFileInfo &HeaderSearch::getFileInfo(FileEntryRef FE) {

clang/lib/Serialization/ASTReader.cpp

@@ -2125,13 +2125,9 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
   HFI.DirInfo = (Flags >> 1) & 0x07;
   HFI.LazyControllingMacro = Reader.getGlobalIdentifierID(
       M, endian::readNext<IdentifierID, llvm::endianness::little>(d));
-  if (unsigned FrameworkOffset =
-          endian::readNext<uint32_t, llvm::endianness::little>(d)) {
-    // The framework offset is 1 greater than the actual offset,
-    // since 0 is used as an indicator for "no framework name".
-    StringRef FrameworkName(FrameworkStrings + FrameworkOffset - 1);
-    HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
-  }
+  auto Zero = endian::readNext<uint32_t, llvm::endianness::little>(d);
+  assert(Zero == 0);
+  (void)Zero;
 
   assert((End - d) % 4 == 0 &&
          "Wrong data length in HeaderFileInfo deserialization");
@@ -3893,13 +3889,10 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
       F.HeaderFileInfoTableData = Blob.data();
       F.LocalNumHeaderFileInfos = Record[1];
       if (Record[0]) {
-        F.HeaderFileInfoTable
-          = HeaderFileInfoLookupTable::Create(
-                   (const unsigned char *)F.HeaderFileInfoTableData + Record[0],
-                   (const unsigned char *)F.HeaderFileInfoTableData,
-                   HeaderFileInfoTrait(*this, F,
-                                       &PP.getHeaderSearchInfo(),
-                                       Blob.data() + Record[2]));
+        F.HeaderFileInfoTable = HeaderFileInfoLookupTable::Create(
+            (const unsigned char *)F.HeaderFileInfoTableData + Record[0],
+            (const unsigned char *)F.HeaderFileInfoTableData,
+            HeaderFileInfoTrait(*this, F));
 
         PP.getHeaderSearchInfo().SetExternalSource(this);
         if (!PP.getHeaderSearchInfo().getExternalLookup())

clang/lib/Serialization/ASTReaderInternals.h

@@ -243,8 +243,6 @@ using ASTSelectorLookupTable =
 class HeaderFileInfoTrait {
   ASTReader &Reader;
   ModuleFile &M;
-  HeaderSearch *HS;
-  const char *FrameworkStrings;
 
 public:
   using external_key_type = FileEntryRef;
@@ -262,9 +260,8 @@ class HeaderFileInfoTrait {
   using hash_value_type = unsigned;
   using offset_type = unsigned;
 
-  HeaderFileInfoTrait(ASTReader &Reader, ModuleFile &M, HeaderSearch *HS,
-                      const char *FrameworkStrings)
-      : Reader(Reader), M(M), HS(HS), FrameworkStrings(FrameworkStrings) {}
+  HeaderFileInfoTrait(ASTReader &Reader, ModuleFile &M)
+      : Reader(Reader), M(M) {}
 
   static hash_value_type ComputeHash(internal_key_ref ikey);
   internal_key_type GetInternalKey(external_key_type ekey);

clang/lib/Serialization/ASTWriter.cpp

@@ -1958,10 +1958,6 @@ namespace {
   class HeaderFileInfoTrait {
     ASTWriter &Writer;
 
-    // Keep track of the framework names we've used during serialization.
-    SmallString<128> FrameworkStringData;
-    llvm::StringMap<unsigned> FrameworkNameOffset;
-
   public:
     HeaderFileInfoTrait(ASTWriter &Writer) : Writer(Writer) {}
 
@@ -2045,21 +2041,7 @@ namespace {
         LE.write<IdentifierID>(
             Writer.getIdentifierRef(Data.HFI.LazyControllingMacro.getPtr()));
 
-      unsigned Offset = 0;
-      if (!Data.HFI.Framework.empty()) {
-        // If this header refers into a framework, save the framework name.
-        llvm::StringMap<unsigned>::iterator Pos
-          = FrameworkNameOffset.find(Data.HFI.Framework);
-        if (Pos == FrameworkNameOffset.end()) {
-          Offset = FrameworkStringData.size() + 1;
-          FrameworkStringData.append(Data.HFI.Framework);
-          FrameworkStringData.push_back(0);
-
-          FrameworkNameOffset[Data.HFI.Framework] = Offset;
-        } else
-          Offset = Pos->second;
-      }
-      LE.write<uint32_t>(Offset);
+      LE.write<uint32_t>(0);
 
       auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
         if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
@@ -2076,9 +2058,6 @@ namespace {
 
       assert(Out.tell() - Start == DataLen && "Wrong data length");
     }
-
-    const char *strings_begin() const { return FrameworkStringData.begin(); }
-    const char *strings_end() const { return FrameworkStringData.end(); }
   };
 
 } // namespace
@@ -2213,7 +2192,6 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
   // Write the header search table
   RecordData::value_type Record[] = {HEADER_SEARCH_TABLE, BucketOffset,
                                      NumHeaderSearchEntries, TableData.size()};
-  TableData.append(GeneratorTrait.strings_begin(),GeneratorTrait.strings_end());
   Stream.EmitRecordWithBlob(TableAbbrev, Record, TableData);
 
   // Free all of the strings we had to duplicate.

clang/unittests/Lex/HeaderSearchTest.cpp

@@ -250,7 +250,6 @@ TEST_F(HeaderSearchTest, HeaderFrameworkLookup) {
   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");
 }
 
@@ -320,7 +319,6 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
   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");
 }