[clang-tools-extra] [clang] NFCI: Use `FileEntryRef` in `SourceManager::FileInfos` (PR #67742)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 28 14:26:39 PDT 2023
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/67742
None
>From 242fa18a43ced2d738fe5cbe44912293767712f9 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Thu, 28 Sep 2023 14:24:56 -0700
Subject: [PATCH] [clang] NFCI: Use `FileEntryRef` in
`SourceManager::FileInfos`
---
.../ExpandModularHeadersPPCallbacks.cpp | 20 +++++-----
clang/include/clang/Basic/FileEntry.h | 12 ++++++
clang/include/clang/Basic/SourceManager.h | 6 +--
clang/lib/Basic/SourceManager.cpp | 5 +--
clang/lib/CodeGen/CGOpenMPRuntime.cpp | 4 +-
clang/unittests/Basic/FileEntryTest.cpp | 40 +++++++++++++++++++
6 files changed, 69 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
index 6130d11e07db950..52cc2e6569b0520 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -21,17 +21,17 @@ namespace clang::tooling {
class ExpandModularHeadersPPCallbacks::FileRecorder {
public:
/// Records that a given file entry is needed for replaying callbacks.
- void addNecessaryFile(const FileEntry *File) {
+ void addNecessaryFile(FileEntryRef File) {
// Don't record modulemap files because it breaks same file detection.
- if (!(File->getName().endswith("module.modulemap") ||
- File->getName().endswith("module.private.modulemap") ||
- File->getName().endswith("module.map") ||
- File->getName().endswith("module_private.map")))
+ if (!(File.getName().endswith("module.modulemap") ||
+ File.getName().endswith("module.private.modulemap") ||
+ File.getName().endswith("module.map") ||
+ File.getName().endswith("module_private.map")))
FilesToRecord.insert(File);
}
/// Records content for a file and adds it to the FileSystem.
- void recordFileContent(const FileEntry *File,
+ void recordFileContent(FileEntryRef File,
const SrcMgr::ContentCache &ContentCache,
llvm::vfs::InMemoryFileSystem &InMemoryFs) {
// Return if we are not interested in the contents of this file.
@@ -43,7 +43,7 @@ class ExpandModularHeadersPPCallbacks::FileRecorder {
if (!Data)
return;
- InMemoryFs.addFile(File->getName(), /*ModificationTime=*/0,
+ InMemoryFs.addFile(File.getName(), /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBufferCopy(*Data));
// Remove the file from the set of necessary files.
FilesToRecord.erase(File);
@@ -55,13 +55,13 @@ class ExpandModularHeadersPPCallbacks::FileRecorder {
LLVM_DEBUG({
for (auto FileEntry : FilesToRecord)
llvm::dbgs() << "Did not record contents for input file: "
- << FileEntry->getName() << "\n";
+ << FileEntry.getName() << "\n";
});
}
private:
/// A set of files whose contents are to be recorded.
- llvm::DenseSet<const FileEntry *> FilesToRecord;
+ llvm::DenseSet<FileEntryRef> FilesToRecord;
};
ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks(
@@ -125,7 +125,7 @@ void ExpandModularHeadersPPCallbacks::handleModuleFile(
Compiler.getASTReader()->visitInputFiles(
*MF, true, false,
[this](const serialization::InputFile &IF, bool /*IsSystem*/) {
- Recorder->addNecessaryFile(IF.getFile());
+ Recorder->addNecessaryFile(*IF.getFile());
});
// Recursively handle all transitively imported modules.
for (auto *Import : MF->Imports)
diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h
index 1fcca4c6dfc3da8..c377aba2c1d2ebd 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -292,6 +292,18 @@ template <> struct DenseMapInfo<clang::FileEntryRef> {
// It's safe to use operator==.
return LHS == RHS;
}
+
+ /// Support for finding `const FileEntry *` in a `DenseMap<FileEntryRef, T>`.
+ /// @{
+ static unsigned getHashValue(const clang::FileEntry *Val) {
+ return llvm::hash_value(Val);
+ }
+ static bool isEqual(const clang::FileEntry *LHS, clang::FileEntryRef RHS) {
+ if (RHS.isSpecialDenseMapKey())
+ return false;
+ return LHS == RHS;
+ }
+ /// @}
};
} // end namespace llvm
diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index 2f846502d6f3327..31f014478583026 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -649,7 +649,7 @@ class SourceManager : public RefCountedBase<SourceManager> {
/// This map allows us to merge ContentCache entries based
/// on their FileEntry*. All ContentCache objects will thus have unique,
/// non-null, FileEntry pointers.
- llvm::DenseMap<const FileEntry*, SrcMgr::ContentCache*> FileInfos;
+ llvm::DenseMap<FileEntryRef, SrcMgr::ContentCache*> FileInfos;
/// True if the ContentCache for files that are overridden by other
/// files, should report the original file name. Defaults to true.
@@ -1680,12 +1680,12 @@ class SourceManager : public RefCountedBase<SourceManager> {
// Iterators over FileInfos.
using fileinfo_iterator =
- llvm::DenseMap<const FileEntry*, SrcMgr::ContentCache*>::const_iterator;
+ llvm::DenseMap<FileEntryRef, SrcMgr::ContentCache *>::const_iterator;
fileinfo_iterator fileinfo_begin() const { return FileInfos.begin(); }
fileinfo_iterator fileinfo_end() const { return FileInfos.end(); }
bool hasFileInfo(const FileEntry *File) const {
- return FileInfos.contains(File);
+ return FileInfos.find_as(File) != FileInfos.end();
}
/// Print statistics to stderr.
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 757b7ea68bb14a4..74334819045d462 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -324,8 +324,7 @@ SourceManager::~SourceManager() {
ContentCacheAlloc.Deallocate(MemBufferInfos[i]);
}
}
- for (llvm::DenseMap<const FileEntry*, SrcMgr::ContentCache*>::iterator
- I = FileInfos.begin(), E = FileInfos.end(); I != E; ++I) {
+ for (auto I = FileInfos.begin(), E = FileInfos.end(); I != E; ++I) {
if (I->second) {
I->second->~ContentCache();
ContentCacheAlloc.Deallocate(I->second);
@@ -702,7 +701,7 @@ void SourceManager::overrideFileContents(const FileEntry *SourceFile,
assert(SourceFile->getSize() == NewFile.getSize() &&
"Different sizes, use the FileManager to create a virtual file with "
"the correct size");
- assert(FileInfos.count(SourceFile) == 0 &&
+ assert(FileInfos.find_as(SourceFile) == FileInfos.end() &&
"This function should be called at the initialization stage, before "
"any parsing occurs.");
// FileEntryRef is not default-constructible.
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 92b7c8d4aa546f0..b4e482ca5cba6ee 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -2913,8 +2913,8 @@ void CGOpenMPRuntime::createOffloadEntriesAndInfoMetadata() {
for (auto I = CGM.getContext().getSourceManager().fileinfo_begin(),
E = CGM.getContext().getSourceManager().fileinfo_end();
I != E; ++I) {
- if (I->getFirst()->getUniqueID().getDevice() == EntryInfo.DeviceID &&
- I->getFirst()->getUniqueID().getFile() == EntryInfo.FileID) {
+ if (I->getFirst().getUniqueID().getDevice() == EntryInfo.DeviceID &&
+ I->getFirst().getUniqueID().getFile() == EntryInfo.FileID) {
Loc = CGM.getContext().getSourceManager().translateFileLineCol(
I->getFirst(), EntryInfo.Line, 1);
break;
diff --git a/clang/unittests/Basic/FileEntryTest.cpp b/clang/unittests/Basic/FileEntryTest.cpp
index 0878063994708f0..80ce2731f6bf665 100644
--- a/clang/unittests/Basic/FileEntryTest.cpp
+++ b/clang/unittests/Basic/FileEntryTest.cpp
@@ -179,6 +179,46 @@ TEST(FileEntryTest, DenseMapInfo) {
EXPECT_TRUE(Set.find(R1)->isSameRef(R1));
EXPECT_TRUE(Set.find(R2)->isSameRef(R2));
}
+
+ // Insert R1Also second and confirm R1 "wins" when looked up as FileEntry.
+ {
+ SmallDenseSet<FileEntryRef, 8> Set;
+ Set.insert(R1Also);
+ Set.insert(R1);
+ Set.insert(R2);
+
+ auto R1AlsoIt = Set.find_as(&R1Also.getFileEntry());
+ ASSERT_TRUE(R1AlsoIt != Set.end());
+ EXPECT_TRUE(R1AlsoIt->isSameRef(R1Also));
+
+ auto R1It = Set.find_as(&R1.getFileEntry());
+ ASSERT_TRUE(R1It != Set.end());
+ EXPECT_TRUE(R1It->isSameRef(R1Also));
+
+ auto R2It = Set.find_as(&R2.getFileEntry());
+ ASSERT_TRUE(R2It != Set.end());
+ EXPECT_TRUE(R2It->isSameRef(R2));
+ }
+
+ // Insert R1Also second and confirm R1 "wins" when looked up as FileEntry.
+ {
+ SmallDenseSet<FileEntryRef, 8> Set;
+ Set.insert(R1);
+ Set.insert(R1Also);
+ Set.insert(R2);
+
+ auto R1AlsoIt = Set.find_as(&R1Also.getFileEntry());
+ ASSERT_TRUE(R1AlsoIt != Set.end());
+ EXPECT_TRUE(R1AlsoIt->isSameRef(R1));
+
+ auto R1It = Set.find_as(&R1.getFileEntry());
+ ASSERT_TRUE(R1It != Set.end());
+ EXPECT_TRUE(R1It->isSameRef(R1));
+
+ auto R2It = Set.find_as(&R2.getFileEntry());
+ ASSERT_TRUE(R2It != Set.end());
+ EXPECT_TRUE(R2It->isSameRef(R2));
+ }
}
TEST(DirectoryEntryTest, isSameRef) {
More information about the cfe-commits
mailing list