https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/67383
This patch adopts `FileEntryRef` in the `HeaderFileInfo`-writing part of `ASTWriter`. First, this patch removes the loop over `FileManager::VirtualFileEntries`. It's redundant, since all virtual file entries have a non-virtual/real counterpart that's already present in `UIDToFiles`. Second, since we now no longer rely on `FileEntry::getLastRef()`/`FileEntry::getName()`, this patch takes care to establish which path gets used for each UID by picking the `FileEntryRef` with the most "`<`" name (instead of just relying on the `StringMap` iteration order). Note that which `FileEntry`/`FileEntryRef` objects we pick for each UID for serialization into the `llvm::OnDiskChainedHashTable` doesn't really matter. The hash function only includes the file size and modification time. The file name only plays role during resolution of hash collisions, in which case it goes through `FileManager` and resolves to a `FileEntry` that gets pointer-compared with the queried `FileEntry`. (Reincarnation of [D143414](https://reviews.llvm.org/D143414) and [D142780](https://reviews.llvm.org/D142780).) >From e2201a10fe76be3de8efd0faca0f381f504b08d3 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Mon, 25 Sep 2023 16:37:49 -0700 Subject: [PATCH] [clang][modules] Adopt `FileEntryRef` in the `HeaderFileInfo` block in PCM files --- clang/include/clang/Basic/FileManager.h | 6 +++--- clang/lib/Basic/FileManager.cpp | 27 +++++++++++-------------- clang/lib/Serialization/ASTWriter.cpp | 20 +++++++++--------- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index 115558bfeee4ed5..56cb093dd8c376f 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -311,9 +311,9 @@ class FileManager : public RefCountedBase<FileManager> { bool makeAbsolutePath(SmallVectorImpl<char> &Path) const; /// Produce an array mapping from the unique IDs assigned to each - /// file to the corresponding FileEntry pointer. - void GetUniqueIDMapping( - SmallVectorImpl<const FileEntry *> &UIDToFiles) const; + /// file to the corresponding FileEntryRef. + void + GetUniqueIDMapping(SmallVectorImpl<OptionalFileEntryRef> &UIDToFiles) const; /// Retrieve the canonical name for a given directory. /// diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index c80fbfd7433f5be..3b834610dd55ef8 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -612,24 +612,21 @@ FileManager::getNoncachedStatValue(StringRef Path, } void FileManager::GetUniqueIDMapping( - SmallVectorImpl<const FileEntry *> &UIDToFiles) const { + SmallVectorImpl<OptionalFileEntryRef> &UIDToFiles) const { UIDToFiles.clear(); UIDToFiles.resize(NextFileUID); - // Map file entries - for (llvm::StringMap<llvm::ErrorOr<FileEntryRef::MapValue>, - llvm::BumpPtrAllocator>::const_iterator - FE = SeenFileEntries.begin(), - FEEnd = SeenFileEntries.end(); - FE != FEEnd; ++FE) - if (llvm::ErrorOr<FileEntryRef::MapValue> Entry = FE->getValue()) { - if (const auto *FE = Entry->V.dyn_cast<FileEntry *>()) - UIDToFiles[FE->getUID()] = FE; - } - - // Map virtual file entries - for (const auto &VFE : VirtualFileEntries) - UIDToFiles[VFE->getUID()] = VFE; + for (const auto &Entry : SeenFileEntries) { + // Only return existing non-virtual files. + if (!Entry.getValue() || !Entry.getValue()->V.is<FileEntry *>()) + continue; + FileEntryRef FE(Entry); + // Add this file if it's the first one with the UID, or if its name is + // better than the existing one. + OptionalFileEntryRef &ExistingFE = UIDToFiles[FE.getUID()]; + if (!ExistingFE || FE.getName() < ExistingFE->getName()) + ExistingFE = FE; + } } StringRef FileManager::getCanonicalName(DirectoryEntryRef Dir) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 216ca94111e156b..1d939a3f5e7a819 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -167,23 +167,23 @@ std::set<const FileEntry *> GetAffectingModuleMaps(const Preprocessor &PP, const HeaderSearch &HS = PP.getHeaderSearchInfo(); - SmallVector<const FileEntry *, 16> FilesByUID; + SmallVector<OptionalFileEntryRef, 16> FilesByUID; HS.getFileMgr().GetUniqueIDMapping(FilesByUID); if (FilesByUID.size() > HS.header_file_size()) FilesByUID.resize(HS.header_file_size()); for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) { - const FileEntry *File = FilesByUID[UID]; + OptionalFileEntryRef File = FilesByUID[UID]; if (!File) continue; const HeaderFileInfo *HFI = - HS.getExistingFileInfo(File, /*WantExternal*/ false); + HS.getExistingFileInfo(*File, /*WantExternal*/ false); if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)) continue; - for (const auto &KH : HS.findResolvedModulesForHeader(File)) { + for (const auto &KH : HS.findResolvedModulesForHeader(*File)) { if (!KH.getModule()) continue; ModulesToProcess.push_back(KH.getModule()); @@ -2003,14 +2003,14 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) { } } - SmallVector<const FileEntry *, 16> FilesByUID; + SmallVector<OptionalFileEntryRef, 16> FilesByUID; HS.getFileMgr().GetUniqueIDMapping(FilesByUID); if (FilesByUID.size() > HS.header_file_size()) FilesByUID.resize(HS.header_file_size()); for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) { - const FileEntry *File = FilesByUID[UID]; + OptionalFileEntryRef File = FilesByUID[UID]; if (!File) continue; @@ -2021,7 +2021,7 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) { // from a different module; in that case, we rely on the module(s) // containing the header to provide this information. const HeaderFileInfo *HFI = - HS.getExistingFileInfo(File, /*WantExternal*/!Chain); + HS.getExistingFileInfo(*File, /*WantExternal*/!Chain); if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)) continue; @@ -2035,13 +2035,13 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) { SavedStrings.push_back(Filename.data()); } - bool Included = PP->alreadyIncluded(File); + bool Included = PP->alreadyIncluded(*File); HeaderFileInfoTrait::key_type Key = { - Filename, File->getSize(), getTimestampForOutput(File) + Filename, File->getSize(), getTimestampForOutput(*File) }; HeaderFileInfoTrait::data_type Data = { - *HFI, Included, HS.getModuleMap().findResolvedModulesForHeader(File), {} + *HFI, Included, HS.getModuleMap().findResolvedModulesForHeader(*File), {} }; Generator.insert(Key, Data, GeneratorTrait); ++NumHeaderSearchEntries; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
