[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: ilya-biryukov.
ilya-biryukov added a comment.

Mimicing RealFS seems like the right thing to do here, so I would vouch for checking this change in.
I'm a little worried that there are tests/clients relying on the old behavior, have you run all the tests?

Also, could you please run `git-clang-format` to fix the formatting issues?



================
Comment at: lib/Basic/VirtualFileSystem.cpp:516
+  explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string RequestedName)
+  : Node(Node), RequestedName (std::move (RequestedName))
+  {}
----------------
NIT: The formatting is broken here.


================
Comment at: lib/Basic/VirtualFileSystem.cpp:520
+  llvm::ErrorOr<Status> status() override {
+    Status St = Node.getStatus();
+    return Status::copyWithNewName(St, RequestedName);
----------------
Maybe add a `RequestedName` parameter to the `InMemoryNode` instead to make sure it's not misused?
It looks like all the clients calling it have to change the name and some are not doing it now, e.g. the directory iterator will use statuses with full paths.


================
Comment at: lib/Basic/VirtualFileSystem.cpp:521
+    Status St = Node.getStatus();
+    return Status::copyWithNewName(St, RequestedName);
+  }
----------------
Maybe add a comment that this matches the real file system behavior?


================
Comment at: lib/Basic/VirtualFileSystem.cpp:722
   if (Node)
-    return (*Node)->getStatus();
+    {
+      Status St = (*Node)->getStatus();
----------------
NIT: The indent is incorrect here.


================
Comment at: lib/Basic/VirtualFileSystem.cpp:724
+      Status St = (*Node)->getStatus();
+      return Status::copyWithNewName(St, Path.str());
+    }
----------------
NIT: we don't need `str()` here


Repository:
  rC Clang

https://reviews.llvm.org/D48903