<div dir="ltr">Hello Simon,<br><br>This commit added broken tests to one of our builders:<br><a href="http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/18386/steps/test/logs/stdio">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/18386/steps/test/logs/stdio</a><br>. . . <br>Failing Tests (4):<br>    Clang :: PCH/case-insensitive-include.c<br>    Clang-Unit :: Basic/./BasicTests.exe/InMemoryFileSystemTest.DirectoryIteration<br>    Clang-Unit :: Basic/./BasicTests.exe/InMemoryFileSystemTest.StatusName<br>    LLVM :: MC/ELF/debug-prefix-map.s<br><br>Please have a look?<br>The builder was already red and did not send notifications.<br><br>Thanks<br><br>Galina<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 11, 2018 at 7:08 AM, Simon Marchi via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: simark<br>
Date: Wed Jul 11 07:08:17 2018<br>
New Revision: 336807<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=336807&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=336807&view=rev</a><br>
Log:<br>
[VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name<br>
<br>
Summary:<br>
InMemoryFileSystem::status behaves differently than<br>
RealFileSystem::status.  The Name contained in the Status returned by<br>
RealFileSystem::status will be the path as requested by the caller,<br>
whereas InMemoryFileSystem::status returns the normalized path.<br>
<br>
For example, when requested the status for "../src/first.h",<br>
RealFileSystem returns a Status with "../src/first.h" as the Name.<br>
InMemoryFileSystem returns "/absolute/path/to/src/first.<wbr>h".<br>
<br>
The reason for this change is that I want to make a unit test in the<br>
clangd testsuite (where we use an InMemoryFileSystem) to reproduce a<br>
bug I get with the clangd program (where a RealFileSystem is used).<br>
This difference in behavior "hides" the bug in the unit test version.<br>
<br>
In general, I guess it's good if InMemoryFileSystem works as much as<br>
possible like RealFileSystem.<br>
<br>
Doing so made the FileEntry::RealPathName value (assigned in<br>
FileManager::getFile) wrong when using the InMemoryFileSystem.  That's<br>
because it assumes that vfs::File::getName will always return the real<br>
path.  I changed to to use FileSystem::getRealPath instead.<br>
<br>
Subscribers: ilya-biryukov, ioeric, cfe-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D48903" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D48903</a><br>
<br>
Modified:<br>
    cfe/trunk/lib/Basic/<wbr>FileManager.cpp<br>
    cfe/trunk/lib/Basic/<wbr>VirtualFileSystem.cpp<br>
    cfe/trunk/unittests/Basic/<wbr>VirtualFileSystemTest.cpp<br>
    cfe/trunk/unittests/Driver/<wbr>ToolChainTest.cpp<br>
<br>
Modified: cfe/trunk/lib/Basic/<wbr>FileManager.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=336807&r1=336806&r2=336807&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Basic/<wbr>FileManager.cpp?rev=336807&r1=<wbr>336806&r2=336807&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Basic/<wbr>FileManager.cpp (original)<br>
+++ cfe/trunk/lib/Basic/<wbr>FileManager.cpp Wed Jul 11 07:08:17 2018<br>
@@ -315,9 +315,11 @@ const FileEntry *FileManager::getFile(St<br>
   UFE.InPCH = Data.InPCH;<br>
   UFE.File = std::move(F);<br>
   UFE.IsValid = true;<br>
-  if (UFE.File)<br>
-    if (auto RealPathName = UFE.File->getName())<br>
-      UFE.RealPathName = *RealPathName;<br>
+<br>
+  SmallString<128> RealPathName;<br>
+  if (!FS->getRealPath(<wbr>InterndFileName, RealPathName))<br>
+    UFE.RealPathName = RealPathName.str();<br>
+<br>
   return &UFE;<br>
 }<br>
<br>
<br>
Modified: cfe/trunk/lib/Basic/<wbr>VirtualFileSystem.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=336807&r1=336806&r2=336807&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Basic/<wbr>VirtualFileSystem.cpp?rev=<wbr>336807&r1=336806&r2=336807&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Basic/<wbr>VirtualFileSystem.cpp (original)<br>
+++ cfe/trunk/lib/Basic/<wbr>VirtualFileSystem.cpp Wed Jul 11 07:08:17 2018<br>
@@ -471,15 +471,33 @@ enum InMemoryNodeKind { IME_File, IME_Di<br>
 /// The in memory file system is a tree of Nodes. Every node can either be a<br>
 /// file or a directory.<br>
 class InMemoryNode {<br>
-  Status Stat;<br>
   InMemoryNodeKind Kind;<br>
+  Status Stat;<br>
+<br>
+protected:<br>
+  /// Return Stat.  This should only be used for internal/debugging use.  When<br>
+  /// clients wants the Status of this node, they should use<br>
+  /// \p getStatus(StringRef).<br>
+  const Status& getStatus() const {<br>
+    return Stat;<br>
+  }<br>
<br>
 public:<br>
   InMemoryNode(Status Stat, InMemoryNodeKind Kind)<br>
-      : Stat(std::move(Stat)), Kind(Kind) {}<br>
+      : Kind(Kind), Stat(std::move(Stat)) {}<br>
   virtual ~InMemoryNode() = default;<br>
<br>
-  const Status &getStatus() const { return Stat; }<br>
+  /// Return the \p Status for this node. \p RequestedName should be the name<br>
+  /// through which the caller referred to this node. It will override<br>
+  /// \p Status::Name in the return value, to mimic the behavior of \p RealFile.<br>
+  Status getStatus(StringRef RequestedName) const {<br>
+    return Status::copyWithNewName(Stat, RequestedName);<br>
+  }<br>
+<br>
+  /// Get the filename of this node (the name without the directory part).<br>
+  StringRef getFileName() const {<br>
+    return llvm::sys::path::filename(<wbr>Stat.getName());<br>
+  }<br>
   InMemoryNodeKind getKind() const { return Kind; }<br>
   virtual std::string toString(unsigned Indent) const = 0;<br>
 };<br>
@@ -504,14 +522,22 @@ public:<br>
   }<br>
 };<br>
<br>
-/// Adapt a InMemoryFile for VFS' File interface.<br>
+/// Adapt a InMemoryFile for VFS' File interface.  The goal is to make<br>
+/// \p InMemoryFileAdaptor mimic as much as possible the behavior of<br>
+/// \p RealFile.<br>
 class InMemoryFileAdaptor : public File {<br>
   InMemoryFile &Node;<br>
<br>
+  /// The name to use when returning a Status for this file.<br>
+  std::string RequestedName;<br>
+<br>
 public:<br>
-  explicit InMemoryFileAdaptor(<wbr>InMemoryFile &Node) : Node(Node) {}<br>
+  explicit InMemoryFileAdaptor(<wbr>InMemoryFile &Node, std::string RequestedName)<br>
+      : Node(Node), RequestedName(std::move(<wbr>RequestedName)) {}<br>
<br>
-  llvm::ErrorOr<Status> status() override { return Node.getStatus(); }<br>
+  llvm::ErrorOr<Status> status() override {<br>
+    return Node.getStatus(RequestedName);<br>
+  }<br>
<br>
   llvm::ErrorOr<std::unique_ptr<<wbr>llvm::MemoryBuffer>><br>
   getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,<br>
@@ -711,7 +737,7 @@ lookupInMemoryNode(const InMemoryFileSys<br>
 llvm::ErrorOr<Status> InMemoryFileSystem::status(<wbr>const Twine &Path) {<br>
   auto Node = lookupInMemoryNode(*this, Root.get(), Path);<br>
   if (Node)<br>
-    return (*Node)->getStatus();<br>
+    return (*Node)->getStatus(Path.str())<wbr>;<br>
   return Node.getError();<br>
 }<br>
<br>
@@ -724,7 +750,8 @@ InMemoryFileSystem::<wbr>openFileForRead(cons<br>
   // When we have a file provide a heap-allocated wrapper for the memory buffer<br>
   // to match the ownership semantics for File.<br>
   if (auto *F = dyn_cast<detail::InMemoryFile><wbr>(*Node))<br>
-    return std::unique_ptr<File>(new detail::InMemoryFileAdaptor(*<wbr>F));<br>
+    return std::unique_ptr<File>(<br>
+        new detail::InMemoryFileAdaptor(*<wbr>F, Path.str()));<br>
<br>
   // FIXME: errc::not_a_file?<br>
   return make_error_code(llvm::errc::<wbr>invalid_argument);<br>
@@ -736,21 +763,33 @@ namespace {<br>
 class InMemoryDirIterator : public clang::vfs::detail::<wbr>DirIterImpl {<br>
   detail::InMemoryDirectory::<wbr>const_iterator I;<br>
   detail::InMemoryDirectory::<wbr>const_iterator E;<br>
+  std::string RequestedDirName;<br>
+<br>
+  void setCurrentEntry() {<br>
+    if (I != E) {<br>
+      SmallString<256> Path(RequestedDirName);<br>
+      llvm::sys::path::append(Path, I->second->getFileName());<br>
+      CurrentEntry = I->second->getStatus(Path.str(<wbr>));<br>
+    } else {<br>
+      // When we're at the end, make CurrentEntry invalid and DirIterImpl will<br>
+      // do the rest.<br>
+      CurrentEntry = Status();<br>
+    }<br>
+  }<br>
<br>
 public:<br>
   InMemoryDirIterator() = default;<br>
<br>
-  explicit InMemoryDirIterator(detail::<wbr>InMemoryDirectory &Dir)<br>
-      : I(Dir.begin()), E(Dir.end()) {<br>
-    if (I != E)<br>
-      CurrentEntry = I->second->getStatus();<br>
+  explicit InMemoryDirIterator(detail::<wbr>InMemoryDirectory &Dir,<br>
+                               std::string RequestedDirName)<br>
+      : I(Dir.begin()), E(Dir.end()),<br>
+        RequestedDirName(std::move(<wbr>RequestedDirName)) {<br>
+    setCurrentEntry();<br>
   }<br>
<br>
   std::error_code increment() override {<br>
     ++I;<br>
-    // When we're at the end, make CurrentEntry invalid and DirIterImpl will do<br>
-    // the rest.<br>
-    CurrentEntry = I != E ? I->second->getStatus() : Status();<br>
+    setCurrentEntry();<br>
     return {};<br>
   }<br>
 };<br>
@@ -766,7 +805,8 @@ directory_iterator InMemoryFileSystem::d<br>
   }<br>
<br>
   if (auto *DirNode = dyn_cast<detail::<wbr>InMemoryDirectory>(*Node))<br>
-    return directory_iterator(std::make_<wbr>shared<InMemoryDirIterator>(*<wbr>DirNode));<br>
+    return directory_iterator(<br>
+        std::make_shared<<wbr>InMemoryDirIterator>(*DirNode, Dir.str()));<br>
<br>
   EC = make_error_code(llvm::errc::<wbr>not_a_directory);<br>
   return directory_iterator(std::make_<wbr>shared<InMemoryDirIterator>())<wbr>;<br>
<br>
Modified: cfe/trunk/unittests/Basic/<wbr>VirtualFileSystemTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=336807&r1=336806&r2=336807&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/unittests/<wbr>Basic/VirtualFileSystemTest.<wbr>cpp?rev=336807&r1=336806&r2=<wbr>336807&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/unittests/Basic/<wbr>VirtualFileSystemTest.cpp (original)<br>
+++ cfe/trunk/unittests/Basic/<wbr>VirtualFileSystemTest.cpp Wed Jul 11 07:08:17 2018<br>
@@ -794,7 +794,7 @@ TEST_F(InMemoryFileSystemTest, WorkingDi<br>
<br>
   auto Stat = FS.status("/b/c");<br>
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();<br>
-  ASSERT_EQ("c", Stat->getName());<br>
+  ASSERT_EQ("/b/c", Stat->getName());<br>
   ASSERT_EQ("/b", *FS.<wbr>getCurrentWorkingDirectory());<br>
<br>
   Stat = FS.status("c");<br>
@@ -919,6 +919,37 @@ TEST_F(InMemoryFileSystemTest, AddDirect<br>
   ASSERT_TRUE(Stat-><wbr>isRegularFile());<br>
 }<br>
<br>
+// Test that the name returned by status() is in the same form as the path that<br>
+// was requested (to match the behavior of RealFileSystem).<br>
+TEST_F(<wbr>InMemoryFileSystemTest, StatusName) {<br>
+  NormalizedFS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("<wbr>abc"),<br>
+                       /*User=*/None,<br>
+                       /*Group=*/None, sys::fs::file_type::regular_<wbr>file);<br>
+  NormalizedFS.<wbr>setCurrentWorkingDirectory("/<wbr>a/b");<br>
+<br>
+  // Access using InMemoryFileSystem::status.<br>
+  auto Stat = NormalizedFS.status("../b/c");<br>
+  ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"<br>
+                                << NormalizedFS.toString();<br>
+  ASSERT_TRUE(Stat-><wbr>isRegularFile());<br>
+  ASSERT_EQ("../b/c", Stat->getName());<br>
+<br>
+  // Access using InMemoryFileAdaptor::status.<br>
+  auto File = NormalizedFS.openFileForRead("<wbr>../b/c");<br>
+  ASSERT_FALSE(File.getError()) << File.getError() << "\n"<br>
+                                << NormalizedFS.toString();<br>
+  Stat = (*File)->status();<br>
+  ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"<br>
+                                << NormalizedFS.toString();<br>
+  ASSERT_TRUE(Stat-><wbr>isRegularFile());<br>
+  ASSERT_EQ("../b/c", Stat->getName());<br>
+<br>
+  // Access using a directory iterator.<br>
+  std::error_code EC;<br>
+  clang::vfs::directory_iterator It = NormalizedFS.dir_begin("../b", EC);<br>
+  ASSERT_EQ("../b/c", It->getName());<br>
+}<br>
+<br>
 // NOTE: in the tests below, we use '//root/' as our root directory, since it is<br>
 // a legal *absolute* path on Windows as well as *nix.<br>
 class VFSFromYAMLTest : public ::testing::Test {<br>
<br>
Modified: cfe/trunk/unittests/Driver/<wbr>ToolChainTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Driver/ToolChainTest.cpp?rev=336807&r1=336806&r2=336807&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/unittests/<wbr>Driver/ToolChainTest.cpp?rev=<wbr>336807&r1=336806&r2=336807&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/unittests/Driver/<wbr>ToolChainTest.cpp (original)<br>
+++ cfe/trunk/unittests/Driver/<wbr>ToolChainTest.cpp Wed Jul 11 07:08:17 2018<br>
@@ -113,7 +113,7 @@ TEST(ToolChainTest, VFSGCCInstallationRe<br>
   std::replace(S.begin(), S.end(), '\\', '/');<br>
 #endif<br>
   EXPECT_EQ("Found candidate GCC installation: "<br>
-            "/home/test/lib/gcc/arm-linux-<wbr>gnueabi/4.6.1\n"<br>
+            "/home/test/bin/../lib/gcc/<wbr>arm-linux-gnueabi/4.6.1\n"<br>
             "Selected GCC installation: "<br>
             "/home/test/bin/../lib/gcc/<wbr>arm-linux-gnueabi/4.6.1\n"<br>
             "Candidate multilib: .;@m32\n"<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">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/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>