r337850 - Revert "[VFS] Cleanups to VFS interfaces."
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 25 09:02:58 PDT 2018
Thanks Jordan!
On Wed, Jul 25, 2018 at 6:01 PM Jordan Rupprecht <rupprecht at google.com>
wrote:
> On Wed, Jul 25, 2018 at 1:40 AM Eric Liu <ioeric at google.com> wrote:
>
>> Please also include a link to the test failure in the commit message or
>> this email thread.
>>
> Sorry, I left the failure on the review thread (
> https://reviews.llvm.org/D49724) but forgot to include it in the commit
> message. I'll make sure to do that next time!
>
> Build bot logs:
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/14441/steps/ninja%20check%201
> Sample snippet:
>
> [ RUN ] GoToInclude.All
> /home/buildslave/buildslave/clang-cmake-aarch64-quick/llvm/tools/clang/tools/extra/unittests/clangd/XRefsTests.cpp:953:
> Failure
> Value of: *Locations
> Expected: has 1 element that is equal to 0:0-0:0 at file:///clangd-test/foo.h
> Actual: {}
>
>
>
>>
>> On Tue, Jul 24, 2018 at 10:28 PM Jordan Rupprecht via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: rupprecht
>>> Date: Tue Jul 24 13:28:07 2018
>>> New Revision: 337850
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=337850&view=rev
>>> Log:
>>> Revert "[VFS] Cleanups to VFS interfaces."
>>>
>>> This reverts commit r337834 due to test failures.
>>>
>>> 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=337850&r1=337849&r2=337850&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
>>> +++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Tue Jul 24
>>> 13:28:07 2018
>>> @@ -45,8 +45,7 @@ class MemoryBuffer;
>>> namespace clang {
>>> namespace vfs {
>>>
>>> -/// File information from a \p File::status() operation.
>>> -/// This is roughly equivalent to a `struct stat` plus a file path.
>>> +/// The result of a \p status operation.
>>> class Status {
>>> std::string Name;
>>> llvm::sys::fs::UniqueID UID;
>>> @@ -67,14 +66,13 @@ 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 this Status with a different name.
>>> - Status copyWithNewName(StringRef NewName);
>>> + /// 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);
>>>
>>> /// 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
>>> @@ -109,16 +107,15 @@ 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 "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 name of the file
>>> + virtual llvm::ErrorOr<std::string> getName() {
>>> + if (auto Status = status())
>>> + return Status->getName().str();
>>> + else
>>> + return Status.getError();
>>> + }
>>>
>>> /// 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=337850&r1=337849&r2=337850&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
>>> +++ cfe/trunk/lib/Basic/FileManager.cpp Tue Jul 24 13:28:07 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->getRealPath())
>>> + if (auto RealPathName = UFE.File->getName())
>>> 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=337850&r1=337849&r2=337850&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
>>> +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue Jul 24 13:28:07 2018
>>> @@ -73,14 +73,16 @@ Status::Status(StringRef Name, UniqueID
>>> : Name(Name), UID(UID), MTime(MTime), User(User), Group(Group),
>>> Size(Size),
>>> Type(Type), Perms(Perms) {}
>>>
>>> -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());
>>> +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());
>>> }
>>>
>>> bool Status::equivalent(const Status &Other) const {
>>> @@ -176,7 +178,6 @@ 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, {}),
>>> @@ -188,7 +189,7 @@ public:
>>> ~RealFile() override;
>>>
>>> ErrorOr<Status> status() override;
>>> - Optional<std::string> getRealPath() override;
>>> + ErrorOr<std::string> getName() override;
>>> ErrorOr<std::unique_ptr<MemoryBuffer>> getBuffer(const Twine &Name,
>>> int64_t FileSize,
>>> bool
>>> RequiresNullTerminator,
>>> @@ -206,15 +207,13 @@ ErrorOr<Status> RealFile::status() {
>>> file_status RealStatus;
>>> if (std::error_code EC = sys::fs::status(FD, RealStatus))
>>> return EC;
>>> - S = Status(RealStatus, S.getName());
>>> + S = Status::copyWithNewName(RealStatus, S.getName());
>>> }
>>> return S;
>>> }
>>>
>>> -Optional<std::string> RealFile::getRealPath() {
>>> - if (RealName.empty())
>>> - return llvm::None;
>>> - return RealName;
>>> +ErrorOr<std::string> RealFile::getName() {
>>> + return RealName.empty() ? S.getName().str() : RealName;
>>> }
>>>
>>> ErrorOr<std::unique_ptr<MemoryBuffer>>
>>> @@ -252,7 +251,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(RealStatus, Path.str());
>>> + return Status::copyWithNewName(RealStatus, Path.str());
>>> }
>>>
>>> ErrorOr<std::unique_ptr<File>>
>>> @@ -304,7 +303,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(S, Iter->path());
>>> + CurrentEntry = Status::copyWithNewName(S, Iter->path());
>>> if (!EC)
>>> EC = ErrorCode;
>>> }
>>> @@ -318,7 +317,7 @@ public:
>>> } else {
>>> llvm::sys::fs::file_status S;
>>> std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(),
>>> S, true);
>>> - CurrentEntry = Status(S, Iter->path());
>>> + CurrentEntry = Status::copyWithNewName(S, Iter->path());
>>> if (!EC)
>>> EC = ErrorCode;
>>> }
>>> @@ -1642,7 +1641,7 @@ static Status getRedirectedFileStatus(co
>>> Status ExternalStatus) {
>>> Status S = ExternalStatus;
>>> if (!UseExternalNames)
>>> - S = S.copyWithNewName(Path.str());
>>> + S = Status::copyWithNewName(S, Path.str());
>>> S.IsVFSMapped = true;
>>> return S;
>>> }
>>> @@ -1658,7 +1657,7 @@ ErrorOr<Status> RedirectingFileSystem::s
>>> return S;
>>> } else { // directory
>>> auto *DE = cast<RedirectingDirectoryEntry>(E);
>>> - return DE->getStatus().copyWithNewName(Path.str());
>>> + return Status::copyWithNewName(DE->getStatus(), Path.str());
>>> }
>>> }
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180725/30b9c221/attachment.html>
More information about the cfe-commits
mailing list