<div dir="ltr">Thanks Jordan!</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 25, 2018 at 6:01 PM Jordan Rupprecht <<a href="mailto:rupprecht@google.com">rupprecht@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Jul 25, 2018 at 1:40 AM Eric Liu <<a href="mailto:ioeric@google.com" target="_blank">ioeric@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Please also include a link to the test failure in the commit message or this email thread.</div></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div>Sorry, I left the failure on the review thread (<a href="https://reviews.llvm.org/D49724" target="_blank">https://reviews.llvm.org/D49724</a>) but forgot to include it in the commit message. I'll make sure to do that next time!</div><div><br></div><div><div>Build bot logs: <a href="http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/14441/steps/ninja%20check%201" target="_blank">http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/14441/steps/ninja%20check%201</a></div><div>Sample snippet:</div></div></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div class="gmail_quote"><div><div>[ RUN ] GoToInclude.All</div></div></div><div class="gmail_quote"><div><div>/home/buildslave/buildslave/clang-cmake-aarch64-quick/llvm/tools/clang/tools/extra/unittests/clangd/XRefsTests.cpp:953: Failure</div></div></div><div class="gmail_quote"><div><div>Value of: *Locations</div></div></div><div class="gmail_quote"><div><div>Expected: has 1 element that is equal to 0:0-0:0@file:///clangd-test/foo.h</div></div></div><div class="gmail_quote"><div><div> Actual: {}</div></div></div></blockquote></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 24, 2018 at 10:28 PM Jordan Rupprecht via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: rupprecht<br>
Date: Tue Jul 24 13:28:07 2018<br>
New Revision: 337850<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=337850&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=337850&view=rev</a><br>
Log:<br>
Revert "[VFS] Cleanups to VFS interfaces."<br>
<br>
This reverts commit r337834 due to test failures.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/VirtualFileSystem.h<br>
cfe/trunk/lib/Basic/FileManager.cpp<br>
cfe/trunk/lib/Basic/VirtualFileSystem.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=337850&r1=337849&r2=337850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=337850&r1=337849&r2=337850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)<br>
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Tue Jul 24 13:28:07 2018<br>
@@ -45,8 +45,7 @@ class MemoryBuffer;<br>
namespace clang {<br>
namespace vfs {<br>
<br>
-/// File information from a \p File::status() operation.<br>
-/// This is roughly equivalent to a `struct stat` plus a file path.<br>
+/// The result of a \p status operation.<br>
class Status {<br>
std::string Name;<br>
llvm::sys::fs::UniqueID UID;<br>
@@ -67,14 +66,13 @@ public:<br>
llvm::sys::TimePoint<> MTime, uint32_t User, uint32_t Group,<br>
uint64_t Size, llvm::sys::fs::file_type Type,<br>
llvm::sys::fs::perms Perms);<br>
- Status(const llvm::sys::fs::file_status &FSStatus, StringRef Name);<br>
<br>
- /// Get a copy of a this Status with a different name.<br>
- Status copyWithNewName(StringRef NewName);<br>
+ /// Get a copy of a Status with a different name.<br>
+ static Status copyWithNewName(const Status &In, StringRef NewName);<br>
+ static Status copyWithNewName(const llvm::sys::fs::file_status &In,<br>
+ StringRef NewName);<br>
<br>
/// Returns the name that should be used for this file or directory.<br>
- /// This is usually the path that the file was opened as, without resolving<br>
- /// relative paths or symlinks.<br>
StringRef getName() const { return Name; }<br>
<br>
/// @name Status interface from llvm::sys::fs<br>
@@ -109,16 +107,15 @@ public:<br>
virtual ~File();<br>
<br>
/// Get the status of the file.<br>
- /// This may access the filesystem (e.g. `stat()`), or return a cached value.<br>
virtual llvm::ErrorOr<Status> status() = 0;<br>
<br>
- /// Get the "real name" of the file, if available.<br>
- /// This should be absolute, and typically has symlinks resolved.<br>
- ///<br>
- /// Only some VFS implementations provide this, and only sometimes.<br>
- /// FIXME: these maybe-available semantics are not very useful. It would be<br>
- /// nice if this was more consistent with FileSystem::getRealPath().<br>
- virtual llvm::Optional<std::string> getRealPath() { return llvm::None; }<br>
+ /// Get the name of the file<br>
+ virtual llvm::ErrorOr<std::string> getName() {<br>
+ if (auto Status = status())<br>
+ return Status->getName().str();<br>
+ else<br>
+ return Status.getError();<br>
+ }<br>
<br>
/// Get the contents of the file as a \p MemoryBuffer.<br>
virtual llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>><br>
<br>
Modified: cfe/trunk/lib/Basic/FileManager.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=337850&r1=337849&r2=337850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=337850&r1=337849&r2=337850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/FileManager.cpp (original)<br>
+++ cfe/trunk/lib/Basic/FileManager.cpp Tue Jul 24 13:28:07 2018<br>
@@ -316,7 +316,7 @@ const FileEntry *FileManager::getFile(St<br>
UFE.File = std::move(F);<br>
UFE.IsValid = true;<br>
if (UFE.File)<br>
- if (auto RealPathName = UFE.File->getRealPath())<br>
+ if (auto RealPathName = UFE.File->getName())<br>
UFE.RealPathName = *RealPathName;<br>
return &UFE;<br>
}<br>
<br>
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=337850&r1=337849&r2=337850&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=337850&r1=337849&r2=337850&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)<br>
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue Jul 24 13:28:07 2018<br>
@@ -73,14 +73,16 @@ Status::Status(StringRef Name, UniqueID<br>
: Name(Name), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size),<br>
Type(Type), Perms(Perms) {}<br>
<br>
-Status::Status(const file_status &In, StringRef NewName)<br>
- : Status(NewName, In.getUniqueID(), In.getLastModificationTime(),<br>
- In.getUser(), In.getGroup(), In.getSize(), In.type(),<br>
- In.permissions()) {}<br>
-<br>
-Status Status::copyWithNewName(StringRef NewName) {<br>
- return Status(NewName, getUniqueID(), getLastModificationTime(), getUser(),<br>
- getGroup(), getSize(), getType(), getPermissions());<br>
+Status Status::copyWithNewName(const Status &In, StringRef NewName) {<br>
+ return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),<br>
+ In.getUser(), In.getGroup(), In.getSize(), In.getType(),<br>
+ In.getPermissions());<br>
+}<br>
+<br>
+Status Status::copyWithNewName(const file_status &In, StringRef NewName) {<br>
+ return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),<br>
+ In.getUser(), In.getGroup(), In.getSize(), In.type(),<br>
+ In.permissions());<br>
}<br>
<br>
bool Status::equivalent(const Status &Other) const {<br>
@@ -176,7 +178,6 @@ class RealFile : public File {<br>
Status S;<br>
std::string RealName;<br>
<br>
- // NewRealPathName is used only if non-empty.<br>
RealFile(int FD, StringRef NewName, StringRef NewRealPathName)<br>
: FD(FD), S(NewName, {}, {}, {}, {}, {},<br>
llvm::sys::fs::file_type::status_error, {}),<br>
@@ -188,7 +189,7 @@ public:<br>
~RealFile() override;<br>
<br>
ErrorOr<Status> status() override;<br>
- Optional<std::string> getRealPath() override;<br>
+ ErrorOr<std::string> getName() override;<br>
ErrorOr<std::unique_ptr<MemoryBuffer>> getBuffer(const Twine &Name,<br>
int64_t FileSize,<br>
bool RequiresNullTerminator,<br>
@@ -206,15 +207,13 @@ ErrorOr<Status> RealFile::status() {<br>
file_status RealStatus;<br>
if (std::error_code EC = sys::fs::status(FD, RealStatus))<br>
return EC;<br>
- S = Status(RealStatus, S.getName());<br>
+ S = Status::copyWithNewName(RealStatus, S.getName());<br>
}<br>
return S;<br>
}<br>
<br>
-Optional<std::string> RealFile::getRealPath() {<br>
- if (RealName.empty())<br>
- return llvm::None;<br>
- return RealName;<br>
+ErrorOr<std::string> RealFile::getName() {<br>
+ return RealName.empty() ? S.getName().str() : RealName;<br>
}<br>
<br>
ErrorOr<std::unique_ptr<MemoryBuffer>><br>
@@ -252,7 +251,7 @@ ErrorOr<Status> RealFileSystem::status(c<br>
sys::fs::file_status RealStatus;<br>
if (std::error_code EC = sys::fs::status(Path, RealStatus))<br>
return EC;<br>
- return Status(RealStatus, Path.str());<br>
+ return Status::copyWithNewName(RealStatus, Path.str());<br>
}<br>
<br>
ErrorOr<std::unique_ptr<File>><br>
@@ -304,7 +303,7 @@ public:<br>
if (Iter != llvm::sys::fs::directory_iterator()) {<br>
llvm::sys::fs::file_status S;<br>
std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);<br>
- CurrentEntry = Status(S, Iter->path());<br>
+ CurrentEntry = Status::copyWithNewName(S, Iter->path());<br>
if (!EC)<br>
EC = ErrorCode;<br>
}<br>
@@ -318,7 +317,7 @@ public:<br>
} else {<br>
llvm::sys::fs::file_status S;<br>
std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);<br>
- CurrentEntry = Status(S, Iter->path());<br>
+ CurrentEntry = Status::copyWithNewName(S, Iter->path());<br>
if (!EC)<br>
EC = ErrorCode;<br>
}<br>
@@ -1642,7 +1641,7 @@ static Status getRedirectedFileStatus(co<br>
Status ExternalStatus) {<br>
Status S = ExternalStatus;<br>
if (!UseExternalNames)<br>
- S = S.copyWithNewName(Path.str());<br>
+ S = Status::copyWithNewName(S, Path.str());<br>
S.IsVFSMapped = true;<br>
return S;<br>
}<br>
@@ -1658,7 +1657,7 @@ ErrorOr<Status> RedirectingFileSystem::s<br>
return S;<br>
} else { // directory<br>
auto *DE = cast<RedirectingDirectoryEntry>(E);<br>
- return DE->getStatus().copyWithNewName(Path.str());<br>
+ return Status::copyWithNewName(DE->getStatus(), Path.str());<br>
}<br>
}<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</blockquote></div></div></blockquote></div>