Skip to content

[clang] NFC: Remove {File,Directory}Entry::getName() #74910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 24, 2024
Merged

[clang] NFC: Remove {File,Directory}Entry::getName() #74910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions clang/include/clang/Basic/DirectoryEntry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions clang/include/clang/Basic/FileEntry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
26 changes: 0 additions & 26 deletions clang/lib/Basic/FileManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down
6 changes: 0 additions & 6 deletions clang/unittests/Basic/FileManagerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down
3 changes: 0 additions & 3 deletions llvm/include/llvm/Support/VirtualFileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Support/VirtualFileSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2334,7 +2334,6 @@ static Status getRedirectedFileStatus(const Twine &OriginalPath,
S = Status::copyWithNewName(S, OriginalPath);
else
S.ExposesExternalVFSPath = true;
S.IsVFSMapped = true;
return S;
}

Expand Down
18 changes: 0 additions & 18 deletions llvm/unittests/Support/VirtualFileSystemTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -1591,29 +1588,25 @@ 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());

// file in remapped directory, with use-external-name=false
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());

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down