r337850 - Revert "[VFS] Cleanups to VFS interfaces."
Jordan Rupprecht via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 25 09:00:44 PDT 2018
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/fec930ab/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180725/fec930ab/attachment-0001.bin>
More information about the cfe-commits
mailing list