[clang] b2a7f1c - Remove a few effectively-unused FileEntry APIs. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 7 07:45:54 PDT 2022
Author: Sam McCall
Date: 2022-04-07T16:45:47+02:00
New Revision: b2a7f1c3904ede781565c6ed772f155167aa8325
URL: https://github.com/llvm/llvm-project/commit/b2a7f1c3904ede781565c6ed772f155167aa8325
DIFF: https://github.com/llvm/llvm-project/commit/b2a7f1c3904ede781565c6ed772f155167aa8325.diff
LOG: Remove a few effectively-unused FileEntry APIs. NFC
- isValid: FileManager only ever returns valid FileEntries (see next point)
- construction from outside FileManager (both FileEntry and DirectoryEntry).
It's not possible to create a useful FileEntry this way, there are no setters.
This was only used in FileEntryTest, added a friend to enable this.
A real constructor is cleaner but requires larger changes to FileManager.
Differential Revision: https://reviews.llvm.org/D123197
Added:
Modified:
clang/include/clang/Basic/DirectoryEntry.h
clang/include/clang/Basic/FileEntry.h
clang/lib/Basic/FileManager.cpp
clang/lib/Frontend/LogDiagnosticPrinter.cpp
clang/lib/Frontend/TextDiagnostic.cpp
clang/unittests/Basic/FileEntryTest.cpp
clang/unittests/Basic/FileManagerTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DirectoryEntry.h b/clang/include/clang/Basic/DirectoryEntry.h
index 21565621b80fc..fe18b12769546 100644
--- a/clang/include/clang/Basic/DirectoryEntry.h
+++ b/clang/include/clang/Basic/DirectoryEntry.h
@@ -32,7 +32,11 @@ template <class RefTy> class MapEntryOptionalStorage;
/// Cached information about one directory (either on disk or in
/// the virtual file system).
class DirectoryEntry {
+ DirectoryEntry() = default;
+ DirectoryEntry(const DirectoryEntry &) = delete;
+ DirectoryEntry &operator=(const DirectoryEntry &) = delete;
friend class FileManager;
+ friend class FileEntryTestHelper;
// FIXME: We should not be storing a directory entry name here.
StringRef Name; // Name of the directory.
diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h
index a0a041c7ab3b9..92a67150ba453 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -65,7 +65,6 @@ class FileEntryRef {
}
DirectoryEntryRef getDir() const { return *ME->second->Dir; }
- inline bool isValid() const;
inline off_t getSize() const;
inline unsigned getUID() const;
inline const llvm::sys::fs::UniqueID &getUniqueID() const;
@@ -330,6 +329,10 @@ static_assert(
/// descriptor for the file.
class FileEntry {
friend class FileManager;
+ friend class FileEntryTestHelper;
+ FileEntry();
+ FileEntry(const FileEntry &) = delete;
+ FileEntry &operator=(const FileEntry &) = delete;
std::string RealPathName; // Real path to the file; could be empty.
off_t Size = 0; // File size in bytes.
@@ -338,7 +341,6 @@ class FileEntry {
llvm::sys::fs::UniqueID UniqueID;
unsigned UID = 0; // A unique (small) ID for the file.
bool IsNamedPipe = false;
- bool IsValid = false; // Is this \c FileEntry initialized and valid?
/// The open file, if it is owned by the \p FileEntry.
mutable std::unique_ptr<llvm::vfs::File> File;
@@ -355,17 +357,11 @@ class FileEntry {
Optional<FileEntryRef> LastRef;
public:
- FileEntry();
~FileEntry();
-
- FileEntry(const FileEntry &) = delete;
- FileEntry &operator=(const FileEntry &) = delete;
-
StringRef getName() const { return LastRef->getName(); }
FileEntryRef getLastRef() const { return *LastRef; }
StringRef tryGetRealPathName() const { return RealPathName; }
- bool isValid() const { return IsValid; }
off_t getSize() const { return Size; }
unsigned getUID() const { return UID; }
const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
@@ -381,8 +377,6 @@ class FileEntry {
void closeFile() const;
};
-bool FileEntryRef::isValid() const { return getFileEntry().isValid(); }
-
off_t FileEntryRef::getSize() const { return getFileEntry().getSize(); }
unsigned FileEntryRef::getUID() const { return getFileEntry().getUID(); }
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index b955e02f1711e..baabb322482c3 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -342,7 +342,6 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
UFE->UniqueID = Status.getUniqueID();
UFE->IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
UFE->File = std::move(F);
- UFE->IsValid = true;
if (UFE->File) {
if (auto PathName = UFE->File->getName())
@@ -453,7 +452,6 @@ FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size,
UFE->ModTime = ModificationTime;
UFE->Dir = &DirInfo->getDirEntry();
UFE->UID = NextFileUID++;
- UFE->IsValid = true;
UFE->File.reset();
return FileEntryRef(NamedFileEnt);
}
@@ -483,7 +481,6 @@ llvm::Optional<FileEntryRef> FileManager::getBypassFile(FileEntryRef VF) {
BFE->Dir = VF.getFileEntry().Dir;
BFE->ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
BFE->UID = NextFileUID++;
- BFE->IsValid = true;
// Save the entry in the bypass table and return.
return FileEntryRef(*Insertion.first);
diff --git a/clang/lib/Frontend/LogDiagnosticPrinter.cpp b/clang/lib/Frontend/LogDiagnosticPrinter.cpp
index df8b23691a7d3..d810b37ca57f2 100644
--- a/clang/lib/Frontend/LogDiagnosticPrinter.cpp
+++ b/clang/lib/Frontend/LogDiagnosticPrinter.cpp
@@ -118,8 +118,7 @@ void LogDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level,
const SourceManager &SM = Info.getSourceManager();
FileID FID = SM.getMainFileID();
if (FID.isValid()) {
- const FileEntry *FE = SM.getFileEntryForID(FID);
- if (FE && FE->isValid())
+ if (const FileEntry *FE = SM.getFileEntryForID(FID))
MainFilename = std::string(FE->getName());
}
}
@@ -148,8 +147,7 @@ void LogDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level,
// At least print the file name if available:
FileID FID = SM.getFileID(Info.getLocation());
if (FID.isValid()) {
- const FileEntry *FE = SM.getFileEntryForID(FID);
- if (FE && FE->isValid())
+ if (const FileEntry *FE = SM.getFileEntryForID(FID))
DE.Filename = std::string(FE->getName());
}
} else {
diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp
index 1c4a76e689534..6c0ea0cde3589 100644
--- a/clang/lib/Frontend/TextDiagnostic.cpp
+++ b/clang/lib/Frontend/TextDiagnostic.cpp
@@ -798,8 +798,7 @@ void TextDiagnostic::emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
// At least print the file name if available:
FileID FID = Loc.getFileID();
if (FID.isValid()) {
- const FileEntry *FE = Loc.getFileEntry();
- if (FE && FE->isValid()) {
+ if (const FileEntry *FE = Loc.getFileEntry()) {
emitFilename(FE->getName(), Loc.getManager());
OS << ": ";
}
diff --git a/clang/unittests/Basic/FileEntryTest.cpp b/clang/unittests/Basic/FileEntryTest.cpp
index 16c8e57d9a177..6c070fb2469df 100644
--- a/clang/unittests/Basic/FileEntryTest.cpp
+++ b/clang/unittests/Basic/FileEntryTest.cpp
@@ -12,25 +12,22 @@
#include "gtest/gtest.h"
using namespace llvm;
-using namespace clang;
-namespace {
-
-using FileMap = StringMap<llvm::ErrorOr<FileEntryRef::MapValue>>;
-using DirMap = StringMap<llvm::ErrorOr<DirectoryEntry &>>;
+namespace clang {
-struct RefMaps {
- FileMap Files;
- DirMap Dirs;
+class FileEntryTestHelper {
+ StringMap<llvm::ErrorOr<FileEntryRef::MapValue>> Files;
+ StringMap<llvm::ErrorOr<DirectoryEntry &>> Dirs;
SmallVector<std::unique_ptr<FileEntry>, 5> FEs;
SmallVector<std::unique_ptr<DirectoryEntry>, 5> DEs;
DirectoryEntryRef DR;
- RefMaps() : DR(addDirectory("dir")) {}
+public:
+ FileEntryTestHelper() : DR(addDirectory("dir")) {}
DirectoryEntryRef addDirectory(StringRef Name) {
- DEs.push_back(std::make_unique<DirectoryEntry>());
+ DEs.emplace_back(new DirectoryEntry());
return DirectoryEntryRef(*Dirs.insert({Name, *DEs.back()}).first);
}
DirectoryEntryRef addDirectoryAlias(StringRef Name, DirectoryEntryRef Base) {
@@ -40,7 +37,7 @@ struct RefMaps {
}
FileEntryRef addFile(StringRef Name) {
- FEs.push_back(std::make_unique<FileEntry>());
+ FEs.emplace_back(new FileEntry());
return FileEntryRef(
*Files.insert({Name, FileEntryRef::MapValue(*FEs.back().get(), DR)})
.first);
@@ -55,19 +52,9 @@ struct RefMaps {
}
};
-TEST(FileEntryTest, Constructor) {
- FileEntry FE;
- EXPECT_EQ(0, FE.getSize());
- EXPECT_EQ(0, FE.getModificationTime());
- EXPECT_EQ(nullptr, FE.getDir());
- EXPECT_EQ(0U, FE.getUniqueID().getDevice());
- EXPECT_EQ(0U, FE.getUniqueID().getFile());
- EXPECT_EQ(false, FE.isNamedPipe());
- EXPECT_EQ(false, FE.isValid());
-}
-
+namespace {
TEST(FileEntryTest, FileEntryRef) {
- RefMaps Refs;
+ FileEntryTestHelper Refs;
FileEntryRef R1 = Refs.addFile("1");
FileEntryRef R2 = Refs.addFile("2");
FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
@@ -84,7 +71,7 @@ TEST(FileEntryTest, FileEntryRef) {
}
TEST(FileEntryTest, OptionalFileEntryRefDegradesToFileEntryPtr) {
- RefMaps Refs;
+ FileEntryTestHelper Refs;
OptionalFileEntryRefDegradesToFileEntryPtr M0;
OptionalFileEntryRefDegradesToFileEntryPtr M1 = Refs.addFile("1");
OptionalFileEntryRefDegradesToFileEntryPtr M2 = Refs.addFile("2");
@@ -102,7 +89,7 @@ TEST(FileEntryTest, OptionalFileEntryRefDegradesToFileEntryPtr) {
}
TEST(FileEntryTest, equals) {
- RefMaps Refs;
+ FileEntryTestHelper Refs;
FileEntryRef R1 = Refs.addFile("1");
FileEntryRef R2 = Refs.addFile("2");
FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
@@ -123,7 +110,7 @@ TEST(FileEntryTest, equals) {
}
TEST(FileEntryTest, isSameRef) {
- RefMaps Refs;
+ FileEntryTestHelper Refs;
FileEntryRef R1 = Refs.addFile("1");
FileEntryRef R2 = Refs.addFile("2");
FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
@@ -135,7 +122,7 @@ TEST(FileEntryTest, isSameRef) {
}
TEST(FileEntryTest, DenseMapInfo) {
- RefMaps Refs;
+ FileEntryTestHelper Refs;
FileEntryRef R1 = Refs.addFile("1");
FileEntryRef R2 = Refs.addFile("2");
FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
@@ -164,7 +151,7 @@ TEST(FileEntryTest, DenseMapInfo) {
}
TEST(DirectoryEntryTest, isSameRef) {
- RefMaps Refs;
+ FileEntryTestHelper Refs;
DirectoryEntryRef R1 = Refs.addDirectory("1");
DirectoryEntryRef R2 = Refs.addDirectory("2");
DirectoryEntryRef R1Also = Refs.addDirectoryAlias("1-also", R1);
@@ -176,7 +163,7 @@ TEST(DirectoryEntryTest, isSameRef) {
}
TEST(DirectoryEntryTest, DenseMapInfo) {
- RefMaps Refs;
+ FileEntryTestHelper Refs;
DirectoryEntryRef R1 = Refs.addDirectory("1");
DirectoryEntryRef R2 = Refs.addDirectory("2");
DirectoryEntryRef R1Also = Refs.addDirectoryAlias("1-also", R1);
@@ -205,3 +192,4 @@ TEST(DirectoryEntryTest, DenseMapInfo) {
}
} // end namespace
+} // namespace clang
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index 3c058a07f23ed..31cb2bbac3cf2 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -165,7 +165,6 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingRealFile) {
auto file = manager.getFile("/tmp/test");
ASSERT_TRUE(file);
- ASSERT_TRUE((*file)->isValid());
EXPECT_EQ("/tmp/test", (*file)->getName());
const DirectoryEntry *dir = (*file)->getDir();
@@ -190,7 +189,6 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) {
manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
auto file = manager.getFile("virtual/dir/bar.h");
ASSERT_TRUE(file);
- ASSERT_TRUE((*file)->isValid());
EXPECT_EQ("virtual/dir/bar.h", (*file)->getName());
const DirectoryEntry *dir = (*file)->getDir();
@@ -212,9 +210,7 @@ TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) {
auto fileFoo = manager.getFile("foo.cpp");
auto fileBar = manager.getFile("bar.cpp");
ASSERT_TRUE(fileFoo);
- ASSERT_TRUE((*fileFoo)->isValid());
ASSERT_TRUE(fileBar);
- ASSERT_TRUE((*fileBar)->isValid());
EXPECT_NE(*fileFoo, *fileBar);
}
@@ -341,9 +337,6 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
statCache->InjectFile("abc/bar.cpp", 42);
manager.setStatCache(std::move(statCache));
- ASSERT_TRUE(manager.getVirtualFile("abc/foo.cpp", 100, 0)->isValid());
- ASSERT_TRUE(manager.getVirtualFile("abc/bar.cpp", 200, 0)->isValid());
-
auto f1 = manager.getFile("abc/foo.cpp");
auto f2 = manager.getFile("abc/bar.cpp");
@@ -418,14 +411,12 @@ TEST_F(FileManagerTest, getVirtualFileWithDifferentName) {
// Inject the virtual file:
const FileEntry *file1 = manager.getVirtualFile("c:\\tmp\\test", 123, 1);
ASSERT_TRUE(file1 != nullptr);
- ASSERT_TRUE(file1->isValid());
EXPECT_EQ(43U, file1->getUniqueID().getFile());
EXPECT_EQ(123, file1->getSize());
// Lookup the virtual file with a
diff erent name:
auto file2 = manager.getFile("c:/tmp/test", 100, 1);
ASSERT_TRUE(file2);
- ASSERT_TRUE((*file2)->isValid());
// Check that it's the same UFE:
EXPECT_EQ(file1, *file2);
EXPECT_EQ(43U, (*file2)->getUniqueID().getFile());
@@ -487,7 +478,6 @@ TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
// Check for real path.
const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123, 1);
ASSERT_TRUE(file != nullptr);
- ASSERT_TRUE(file->isValid());
SmallString<64> ExpectedResult = CustomWorkingDir;
llvm::sys::path::append(ExpectedResult, "tmp", "test");
@@ -515,7 +505,6 @@ TEST_F(FileManagerTest, getFileDontOpenRealPath) {
// Check for real path.
auto file = Manager.getFile("/tmp/test", /*OpenFile=*/false);
ASSERT_TRUE(file);
- ASSERT_TRUE((*file)->isValid());
SmallString<64> ExpectedResult = CustomWorkingDir;
llvm::sys::path::append(ExpectedResult, "tmp", "test");
@@ -548,7 +537,6 @@ TEST_F(FileManagerTest, getBypassFile) {
const FileEntry *File = Manager.getVirtualFile("/tmp/test", /*Size=*/10, 0);
ASSERT_TRUE(File);
const FileEntry &FE = *File;
- EXPECT_TRUE(FE.isValid());
EXPECT_EQ(FE.getSize(), 10);
// Calling a second time should not affect the UID or size.
@@ -564,7 +552,6 @@ TEST_F(FileManagerTest, getBypassFile) {
llvm::Optional<FileEntryRef> BypassRef =
Manager.getBypassFile(File->getLastRef());
ASSERT_TRUE(BypassRef);
- EXPECT_TRUE(BypassRef->isValid());
EXPECT_EQ("/tmp/test", BypassRef->getName());
// Check that it's
diff erent in the right ways.
More information about the cfe-commits
mailing list