[clang] [clang][modules] Adopt `FileEntryRef` in the `HeaderFileInfo` block in PCM files (PR #67383)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 25 16:45:04 PDT 2023
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_svoboda at 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;
More information about the cfe-commits
mailing list