From: Sam McCall Date: Tue, 24 Jul 2018 16:00:55 +0000 (+0000) Subject: [VFS] Cleanups to VFS interfaces. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9e67652533017e8c81d6a9c89b9cb3c69c1b23fc;p=clang [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@337834 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/VirtualFileSystem.h b/include/clang/Basic/VirtualFileSystem.h index 2480b91123..bd752c8dad 100644 --- a/include/clang/Basic/VirtualFileSystem.h +++ b/include/clang/Basic/VirtualFileSystem.h @@ -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() = 0; - /// Get the name of the file - virtual llvm::ErrorOr 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 getRealPath() { return llvm::None; } /// Get the contents of the file as a \p MemoryBuffer. virtual llvm::ErrorOr> diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp index 7e2d01c498..13a3d8f95c 100644 --- a/lib/Basic/FileManager.cpp +++ b/lib/Basic/FileManager.cpp @@ -316,7 +316,7 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile, 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; } diff --git a/lib/Basic/VirtualFileSystem.cpp b/lib/Basic/VirtualFileSystem.cpp index bcfcbdbb90..b269a9fcc0 100644 --- a/lib/Basic/VirtualFileSystem.cpp +++ b/lib/Basic/VirtualFileSystem.cpp @@ -73,16 +73,14 @@ Status::Status(StringRef Name, UniqueID UID, sys::TimePoint<> MTime, : 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(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(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::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() override; - ErrorOr getName() override; + Optional getRealPath() override; ErrorOr> getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator, @@ -207,13 +206,15 @@ ErrorOr 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 RealFile::getName() { - return RealName.empty() ? S.getName().str() : RealName; +Optional RealFile::getRealPath() { + if (RealName.empty()) + return llvm::None; + return RealName; } ErrorOr> @@ -251,7 +252,7 @@ ErrorOr RealFileSystem::status(const Twine &Path) { 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> @@ -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(const Twine &Path, bool UseExternalNames, 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 RedirectingFileSystem::status(const Twine &Path, Entry *E) { return S; } else { // directory auto *DE = cast(E); - return Status::copyWithNewName(DE->getStatus(), Path.str()); + return DE->getStatus().copyWithNewName(Path.str()); } }