Thanks Jordan! On Wed, Jul 25, 2018 at 6:01 PM Jordan Rupprecht <ruppre...@google.com> wrote:
> On Wed, Jul 25, 2018 at 1:40 AM Eric Liu <ioe...@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@file:///clangd-test/foo.h > Actual: {} > > > >> >> On Tue, Jul 24, 2018 at 10:28 PM Jordan Rupprecht via cfe-commits < >> cfe-commits@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@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
