[clang][lex] Remove HeaderFileInfo::Framework (llvm#114460)

This PR removes the `HeaderFileInfo::Framework` member and reduces the
size of this data type from 32B to 16B. This should improve Clang's
memory usage in situations where it keeps track of lots of header files.
NFCI. Depends on llvm#114459.

(cherry picked from commit e494e26)

Author: jansvoboda11

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

@@ -290,9 +290,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;
@@ -306,19 +307,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;
@@ -334,26 +333,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)
@@ -369,8 +369,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)
@@ -384,13 +383,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);
   }
 
@@ -409,11 +410,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/include/clang/Serialization/ASTBitCodes.h

@@ -44,7 +44,7 @@ namespace serialization {
 /// Version 4 of AST files also requires that the version control branch and
 /// revision match exactly, since there is no backward compatibility of
 /// AST files at this time.
-const unsigned VERSION_MAJOR = 32;
+const unsigned VERSION_MAJOR = 33;
 
 /// AST file minor version number supported by this version of
 /// Clang.

clang/lib/Lex/HeaderSearch.cpp

@@ -973,11 +973,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());
@@ -1119,16 +1117,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;
@@ -1313,9 +1301,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

@@ -2111,13 +2111,6 @@ 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);
-  }
 
   assert((End - d) % 4 == 0 &&
          "Wrong data length in HeaderFileInfo deserialization");
@@ -3894,13 +3887,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

@@ -1988,10 +1988,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) {}
 
@@ -2035,7 +2031,7 @@ namespace {
     std::pair<unsigned, unsigned>
     EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
       unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
-      unsigned DataLen = 1 + sizeof(IdentifierID) + 4;
+      unsigned DataLen = 1 + sizeof(IdentifierID);
       for (auto ModInfo : Data.KnownHeaders)
         if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
           DataLen += 4;
@@ -2075,22 +2071,6 @@ 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);
-
       auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
         if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
           uint32_t Value = (ModID << 3) | (unsigned)Role;
@@ -2106,9 +2086,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
@@ -2243,7 +2220,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");
 }