[clang] [llvm] [clang] NFC: Remove `{File, Directory}Entry::getName()` (PR #74910)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 8 18:55:33 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Jan Svoboda (jansvoboda11)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/74910.diff
7 Files Affected:
- (modified) clang/include/clang/Basic/DirectoryEntry.h (-7)
- (modified) clang/include/clang/Basic/FileEntry.h (-10)
- (modified) clang/lib/Basic/FileManager.cpp (-26)
- (modified) clang/unittests/Basic/FileManagerTest.cpp (-6)
- (modified) llvm/include/llvm/Support/VirtualFileSystem.h (-3)
- (modified) llvm/lib/Support/VirtualFileSystem.cpp (-1)
- (modified) llvm/unittests/Support/VirtualFileSystemTest.cpp (-18)
``````````diff
diff --git a/clang/include/clang/Basic/DirectoryEntry.h b/clang/include/clang/Basic/DirectoryEntry.h
index 906c2e9af23b31..35fe529ba79dff 100644
--- a/clang/include/clang/Basic/DirectoryEntry.h
+++ b/clang/include/clang/Basic/DirectoryEntry.h
@@ -41,13 +41,6 @@ class DirectoryEntry {
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.
-
-public:
- LLVM_DEPRECATED("Use DirectoryEntryRef::getName() instead.", "")
- StringRef getName() const { return Name; }
};
/// A reference to a \c DirectoryEntry that includes the name of the directory
diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h
index 35efa147950f06..68d4bf60930037 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -318,18 +318,8 @@ class FileEntry {
/// The file content, if it is owned by the \p FileEntry.
std::unique_ptr<llvm::MemoryBuffer> Content;
- // First access name for this FileEntry.
- //
- // This is Optional only to allow delayed construction (FileEntryRef has no
- // default constructor). It should always have a value in practice.
- //
- // TODO: remove this once everyone that needs a name uses FileEntryRef.
- OptionalFileEntryRef LastRef;
-
public:
~FileEntry();
- LLVM_DEPRECATED("Use FileEntryRef::getName() instead.", "")
- StringRef getName() const { return LastRef->getName(); }
StringRef tryGetRealPathName() const { return RealPathName; }
off_t getSize() const { return Size; }
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index d16626b1065213..e715e69f7bdbb0 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -107,7 +107,6 @@ void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
// Add the virtual directory to the cache.
auto *UDE = new (DirsAlloc.Allocate()) DirectoryEntry();
- UDE->Name = NamedDirEnt.first();
NamedDirEnt.second = *UDE;
VirtualDirectoryEntries.push_back(UDE);
@@ -179,7 +178,6 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
// We don't have this directory yet, add it. We use the string
// key from the SeenDirEntries map as the string.
UDE = new (DirsAlloc.Allocate()) DirectoryEntry();
- UDE->Name = InterndDirName;
}
NamedDirEnt.second = *UDE;
@@ -324,32 +322,10 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
FileEntryRef ReturnedRef(*NamedFileEnt);
if (ReusingEntry) { // Already have an entry with this inode, return it.
-
- // FIXME: This hack ensures that `getDir()` will use the path that was
- // used to lookup this file, even if we found a file by different path
- // first. This is required in order to find a module's structure when its
- // headers/module map are mapped in the VFS.
- //
- // See above for how this will eventually be removed. `IsVFSMapped`
- // *cannot* be narrowed to `ExposesExternalVFSPath` as crash reproducers
- // also depend on this logic and they have `use-external-paths: false`.
- if (&DirInfo.getDirEntry() != UFE->Dir && Status.IsVFSMapped)
- UFE->Dir = &DirInfo.getDirEntry();
-
- // Always update LastRef to the last name by which a file was accessed.
- // FIXME: Neither this nor always using the first reference is correct; we
- // want to switch towards a design where we return a FileName object that
- // encapsulates both the name by which the file was accessed and the
- // corresponding FileEntry.
- // FIXME: LastRef should be removed from FileEntry once all clients adopt
- // FileEntryRef.
- UFE->LastRef = ReturnedRef;
-
return ReturnedRef;
}
// Otherwise, we don't have this file yet, add it.
- UFE->LastRef = ReturnedRef;
UFE->Size = Status.getSize();
UFE->ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
UFE->Dir = &DirInfo.getDirEntry();
@@ -461,7 +437,6 @@ FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size,
}
NamedFileEnt.second = FileEntryRef::MapValue(*UFE, *DirInfo);
- UFE->LastRef = FileEntryRef(NamedFileEnt);
UFE->Size = Size;
UFE->ModTime = ModificationTime;
UFE->Dir = &DirInfo->getDirEntry();
@@ -490,7 +465,6 @@ OptionalFileEntryRef FileManager::getBypassFile(FileEntryRef VF) {
FileEntry *BFE = new (FilesAlloc.Allocate()) FileEntry();
BypassFileEntries.push_back(BFE);
Insertion.first->second = FileEntryRef::MapValue(*BFE, VF.getDir());
- BFE->LastRef = FileEntryRef(*Insertion.first);
BFE->Size = Status.getSize();
BFE->Dir = VF.getFileEntry().Dir;
BFE->ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index 43339676c4a6a2..d32036d975ce9c 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -284,9 +284,6 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) {
ASSERT_FALSE(!F1Alias);
ASSERT_FALSE(!F1Alias2);
EXPECT_EQ("dir/f1.cpp", F1->getName());
- LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
- EXPECT_EQ("dir/f1.cpp", F1->getFileEntry().getName());
- LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
EXPECT_EQ("dir/f1.cpp", F1Alias->getName());
EXPECT_EQ("dir/f1.cpp", F1Alias2->getName());
EXPECT_EQ(&F1->getFileEntry(), &F1Alias->getFileEntry());
@@ -305,9 +302,6 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) {
ASSERT_FALSE(!F2Alias);
ASSERT_FALSE(!F2Alias2);
EXPECT_EQ("dir/f2.cpp", F2->getName());
- LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_PUSH
- EXPECT_EQ("dir/f2.cpp", F2->getFileEntry().getName());
- LLVM_SUPPRESS_DEPRECATED_DECLARATIONS_POP
EXPECT_EQ("dir/f2.cpp", F2Alias->getName());
EXPECT_EQ("dir/f2.cpp", F2Alias2->getName());
EXPECT_EQ(&F2->getFileEntry(), &F2Alias->getFileEntry());
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index 44a56e54a0a09c..1c3d8e985592b6 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -55,9 +55,6 @@ class Status {
llvm::sys::fs::perms Perms;
public:
- // FIXME: remove when files support multiple names
- bool IsVFSMapped = false;
-
/// Whether this entity has an external path different from the virtual path,
/// and the external path is exposed by leaking it through the abstraction.
/// For example, a RedirectingFileSystem will set this for paths where
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 367e794d38f63a..9fed081f85a50a 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -2334,7 +2334,6 @@ static Status getRedirectedFileStatus(const Twine &OriginalPath,
S = Status::copyWithNewName(S, OriginalPath);
else
S.ExposesExternalVFSPath = true;
- S.IsVFSMapped = true;
return S;
}
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index bd4048f025c0de..87b794b05b48f7 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1563,13 +1563,11 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
ErrorOr<vfs::Status> S = O->status("//root/file1");
ASSERT_FALSE(S.getError());
EXPECT_EQ("//root/foo/bar/a", S->getName());
- EXPECT_TRUE(S->IsVFSMapped);
EXPECT_TRUE(S->ExposesExternalVFSPath);
ErrorOr<vfs::Status> SLower = O->status("//root/foo/bar/a");
EXPECT_EQ("//root/foo/bar/a", SLower->getName());
EXPECT_TRUE(S->equivalent(*SLower));
- EXPECT_FALSE(SLower->IsVFSMapped);
EXPECT_FALSE(SLower->ExposesExternalVFSPath);
// file after opening
@@ -1578,7 +1576,6 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
auto OpenedS = (*OpenedF)->status();
ASSERT_FALSE(OpenedS.getError());
EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
- EXPECT_TRUE(OpenedS->IsVFSMapped);
EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
// directory
@@ -1591,21 +1588,18 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
S = O->status("//root/mappeddir");
ASSERT_FALSE(S.getError());
EXPECT_TRUE(S->isDirectory());
- EXPECT_TRUE(S->IsVFSMapped);
EXPECT_TRUE(S->ExposesExternalVFSPath);
EXPECT_TRUE(S->equivalent(*O->status("//root/foo/bar")));
SLower = O->status("//root/foo/bar");
EXPECT_EQ("//root/foo/bar", SLower->getName());
EXPECT_TRUE(S->equivalent(*SLower));
- EXPECT_FALSE(SLower->IsVFSMapped);
EXPECT_FALSE(SLower->ExposesExternalVFSPath);
// file in remapped directory
S = O->status("//root/mappeddir/a");
ASSERT_FALSE(S.getError());
EXPECT_FALSE(S->isDirectory());
- EXPECT_TRUE(S->IsVFSMapped);
EXPECT_TRUE(S->ExposesExternalVFSPath);
EXPECT_EQ("//root/foo/bar/a", S->getName());
@@ -1613,7 +1607,6 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
S = O->status("//root/mappeddir2/a");
ASSERT_FALSE(S.getError());
EXPECT_FALSE(S->isDirectory());
- EXPECT_TRUE(S->IsVFSMapped);
EXPECT_FALSE(S->ExposesExternalVFSPath);
EXPECT_EQ("//root/mappeddir2/a", S->getName());
@@ -1623,7 +1616,6 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
OpenedS = (*OpenedF)->status();
ASSERT_FALSE(OpenedS.getError());
EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
- EXPECT_TRUE(OpenedS->IsVFSMapped);
EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
// file contents in remapped directory, with use-external-name=false
@@ -1632,7 +1624,6 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
OpenedS = (*OpenedF)->status();
ASSERT_FALSE(OpenedS.getError());
EXPECT_EQ("//root/mappeddir2/a", OpenedS->getName());
- EXPECT_TRUE(OpenedS->IsVFSMapped);
EXPECT_FALSE(OpenedS->ExposesExternalVFSPath);
// broken mapping
@@ -1665,13 +1656,11 @@ TEST_F(VFSFromYAMLTest, MappedRoot) {
ErrorOr<vfs::Status> S = O->status("//mappedroot/a");
ASSERT_FALSE(S.getError());
EXPECT_EQ("//root/foo/bar/a", S->getName());
- EXPECT_TRUE(S->IsVFSMapped);
EXPECT_TRUE(S->ExposesExternalVFSPath);
ErrorOr<vfs::Status> SLower = O->status("//root/foo/bar/a");
EXPECT_EQ("//root/foo/bar/a", SLower->getName());
EXPECT_TRUE(S->equivalent(*SLower));
- EXPECT_FALSE(SLower->IsVFSMapped);
EXPECT_FALSE(SLower->ExposesExternalVFSPath);
// file after opening
@@ -1680,7 +1669,6 @@ TEST_F(VFSFromYAMLTest, MappedRoot) {
auto OpenedS = (*OpenedF)->status();
ASSERT_FALSE(OpenedS.getError());
EXPECT_EQ("//root/foo/bar/a", OpenedS->getName());
- EXPECT_TRUE(OpenedS->IsVFSMapped);
EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
EXPECT_EQ(0, NumDiagnostics);
@@ -1829,13 +1817,11 @@ TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
auto OpenedS = (*OpenedF)->status();
ASSERT_FALSE(OpenedS.getError());
EXPECT_EQ("a", OpenedS->getName());
- EXPECT_FALSE(OpenedS->IsVFSMapped);
EXPECT_FALSE(OpenedS->ExposesExternalVFSPath);
auto DirectS = RemappedFS->status("a");
ASSERT_FALSE(DirectS.getError());
EXPECT_EQ("a", DirectS->getName());
- EXPECT_FALSE(DirectS->IsVFSMapped);
EXPECT_FALSE(DirectS->ExposesExternalVFSPath);
EXPECT_EQ(0, NumDiagnostics);
@@ -1871,13 +1857,11 @@ TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
auto OpenedS = (*OpenedF)->status();
ASSERT_FALSE(OpenedS.getError());
EXPECT_EQ("realname", OpenedS->getName());
- EXPECT_TRUE(OpenedS->IsVFSMapped);
EXPECT_TRUE(OpenedS->ExposesExternalVFSPath);
auto DirectS = FS->status("vfsname");
ASSERT_FALSE(DirectS.getError());
EXPECT_EQ("realname", DirectS->getName());
- EXPECT_TRUE(DirectS->IsVFSMapped);
EXPECT_TRUE(DirectS->ExposesExternalVFSPath);
EXPECT_EQ(0, NumDiagnostics);
@@ -1976,13 +1960,11 @@ TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
auto OpenedS = (*OpenedF)->status();
ASSERT_FALSE(OpenedS.getError());
EXPECT_EQ("vfsname", OpenedS->getName());
- EXPECT_TRUE(OpenedS->IsVFSMapped);
EXPECT_FALSE(OpenedS->ExposesExternalVFSPath);
auto DirectS = FS->status("vfsname");
ASSERT_FALSE(DirectS.getError());
EXPECT_EQ("vfsname", DirectS->getName());
- EXPECT_TRUE(DirectS->IsVFSMapped);
EXPECT_FALSE(DirectS->ExposesExternalVFSPath);
EXPECT_EQ(0, NumDiagnostics);
``````````
</details>
https://github.com/llvm/llvm-project/pull/74910
More information about the cfe-commits
mailing list