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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to