r337834 - [VFS] Cleanups to VFS interfaces.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 24 09:00:55 PDT 2018
Author: sammccall
Date: Tue Jul 24 09:00:55 2018
New Revision: 337834
URL: http://llvm.org/viewvc/llvm-project?rev=337834&view=rev
Log:
[VFS] Cleanups to VFS interfaces.
Summary:
- add comments clarifying semantics
- Status::copyWithNewName(Status, Name) --> instance method
- Status::copyWithNewName(fs::file_status, Name) --> constructor (it's not a copy)
- File::getName() -> getRealPath(), reflecting its actual behavior/function
and stop returning status().getName() in the base class (callers can do this
fallback if they want to, it complicates the contracts).
This is mostly NFC, but the behavior of File::getName() affects FileManager's
FileEntry::tryGetRealPathName(), which now fails in more cases:
- non-real file cases
- real-file cases where the underlying vfs::File was opened in a way that
doesn't call realpath().
(In these cases we don't know a distinct real name, so in principle it seems OK)
Reviewers: klimek
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D49724
Modified:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=337834&r1=337833&r2=337834&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Tue Jul 24 09:00:55 2018
@@ -45,7 +45,8 @@ class MemoryBuffer;
namespace clang {
namespace vfs {
-/// The result of a \p status operation.
+/// File information from a \p File::status() operation.
+/// This is roughly equivalent to a `struct stat` plus a file path.
class Status {
std::string Name;
llvm::sys::fs::UniqueID UID;
@@ -66,13 +67,14 @@ public:
llvm::sys::TimePoint<> MTime, uint32_t User, uint32_t Group,
uint64_t Size, llvm::sys::fs::file_type Type,
llvm::sys::fs::perms Perms);
+ Status(const llvm::sys::fs::file_status &FSStatus, StringRef Name);
- /// Get a copy of a Status with a different name.
- static Status copyWithNewName(const Status &In, StringRef NewName);
- static Status copyWithNewName(const llvm::sys::fs::file_status &In,
- StringRef NewName);
+ /// Get a copy of a this Status with a different name.
+ Status copyWithNewName(StringRef NewName);
/// Returns the name that should be used for this file or directory.
+ /// This is usually the path that the file was opened as, without resolving
+ /// relative paths or symlinks.
StringRef getName() const { return Name; }
/// @name Status interface from llvm::sys::fs
@@ -107,15 +109,16 @@ public:
virtual ~File();
/// Get the status of the file.
+ /// This may access the filesystem (e.g. `stat()`), or return a cached value.
virtual llvm::ErrorOr<Status> status() = 0;
- /// Get the name of the file
- virtual llvm::ErrorOr<std::string> getName() {
- if (auto Status = status())
- return Status->getName().str();
- else
- return Status.getError();
- }
+ /// Get the "real name" of the file, if available.
+ /// This should be absolute, and typically has symlinks resolved.
+ ///
+ /// Only some VFS implementations provide this, and only sometimes.
+ /// FIXME: these maybe-available semantics are not very useful. It would be
+ /// nice if this was more consistent with FileSystem::getRealPath().
+ virtual llvm::Optional<std::string> getRealPath() { return llvm::None; }
/// Get the contents of the file as a \p MemoryBuffer.
virtual llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=337834&r1=337833&r2=337834&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Tue Jul 24 09:00:55 2018
@@ -316,7 +316,7 @@ const FileEntry *FileManager::getFile(St
UFE.File = std::move(F);
UFE.IsValid = true;
if (UFE.File)
- if (auto RealPathName = UFE.File->getName())
+ if (auto RealPathName = UFE.File->getRealPath())
UFE.RealPathName = *RealPathName;
return &UFE;
}
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=337834&r1=337833&r2=337834&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue Jul 24 09:00:55 2018
@@ -73,16 +73,14 @@ Status::Status(StringRef Name, UniqueID
: Name(Name), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size),
Type(Type), Perms(Perms) {}
-Status Status::copyWithNewName(const Status &In, StringRef NewName) {
- return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
- In.getUser(), In.getGroup(), In.getSize(), In.getType(),
- In.getPermissions());
-}
-
-Status Status::copyWithNewName(const file_status &In, StringRef NewName) {
- return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
- In.getUser(), In.getGroup(), In.getSize(), In.type(),
- In.permissions());
+Status::Status(const file_status &In, StringRef NewName)
+ : Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
+ In.getUser(), In.getGroup(), In.getSize(), In.type(),
+ In.permissions()) {}
+
+Status Status::copyWithNewName(StringRef NewName) {
+ return Status(NewName, getUniqueID(), getLastModificationTime(), getUser(),
+ getGroup(), getSize(), getType(), getPermissions());
}
bool Status::equivalent(const Status &Other) const {
@@ -178,6 +176,7 @@ class RealFile : public File {
Status S;
std::string RealName;
+ // NewRealPathName is used only if non-empty.
RealFile(int FD, StringRef NewName, StringRef NewRealPathName)
: FD(FD), S(NewName, {}, {}, {}, {}, {},
llvm::sys::fs::file_type::status_error, {}),
@@ -189,7 +188,7 @@ public:
~RealFile() override;
ErrorOr<Status> status() override;
- ErrorOr<std::string> getName() override;
+ Optional<std::string> getRealPath() override;
ErrorOr<std::unique_ptr<MemoryBuffer>> getBuffer(const Twine &Name,
int64_t FileSize,
bool RequiresNullTerminator,
@@ -207,13 +206,15 @@ ErrorOr<Status> RealFile::status() {
file_status RealStatus;
if (std::error_code EC = sys::fs::status(FD, RealStatus))
return EC;
- S = Status::copyWithNewName(RealStatus, S.getName());
+ S = Status(RealStatus, S.getName());
}
return S;
}
-ErrorOr<std::string> RealFile::getName() {
- return RealName.empty() ? S.getName().str() : RealName;
+Optional<std::string> RealFile::getRealPath() {
+ if (RealName.empty())
+ return llvm::None;
+ return RealName;
}
ErrorOr<std::unique_ptr<MemoryBuffer>>
@@ -251,7 +252,7 @@ ErrorOr<Status> RealFileSystem::status(c
sys::fs::file_status RealStatus;
if (std::error_code EC = sys::fs::status(Path, RealStatus))
return EC;
- return Status::copyWithNewName(RealStatus, Path.str());
+ return Status(RealStatus, Path.str());
}
ErrorOr<std::unique_ptr<File>>
@@ -303,7 +304,7 @@ public:
if (Iter != llvm::sys::fs::directory_iterator()) {
llvm::sys::fs::file_status S;
std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
- CurrentEntry = Status::copyWithNewName(S, Iter->path());
+ CurrentEntry = Status(S, Iter->path());
if (!EC)
EC = ErrorCode;
}
@@ -317,7 +318,7 @@ public:
} else {
llvm::sys::fs::file_status S;
std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
- CurrentEntry = Status::copyWithNewName(S, Iter->path());
+ CurrentEntry = Status(S, Iter->path());
if (!EC)
EC = ErrorCode;
}
@@ -1641,7 +1642,7 @@ static Status getRedirectedFileStatus(co
Status ExternalStatus) {
Status S = ExternalStatus;
if (!UseExternalNames)
- S = Status::copyWithNewName(S, Path.str());
+ S = S.copyWithNewName(Path.str());
S.IsVFSMapped = true;
return S;
}
@@ -1657,7 +1658,7 @@ ErrorOr<Status> RedirectingFileSystem::s
return S;
} else { // directory
auto *DE = cast<RedirectingDirectoryEntry>(E);
- return Status::copyWithNewName(DE->getStatus(), Path.str());
+ return DE->getStatus().copyWithNewName(Path.str());
}
}
More information about the cfe-commits
mailing list