[PATCH] D13430: [VFS] Add an in-memory file system implementation.

[Benjamin Kramer via cfe-commits]

bkramer marked 10 inline comments as done.

================
Comment at: include/clang/Basic/VirtualFileSystem.h:286-288
@@ +285,5 @@
+  }
+  std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+    WorkingDirectory = Path.str();
+    return std::error_code();
+  }
----------------
klimek wrote:
> Return errc::success?
There is no errc::success, the preferred way is to just use error_code(). Fixed the comment.

================
Comment at: lib/Basic/VirtualFileSystem.cpp:80
@@ -79,1 +79,3 @@
 
+// FIXME: This is copypasted from LLVM's file system implementation.
+std::error_code FileSystem::make_absolute(SmallVectorImpl<char> &path) const {
----------------
klimek wrote:
> What's the proposed fix? :)
Moving it to LLVM. That's done now :)

================
Comment at: lib/Basic/VirtualFileSystem.cpp:312-316
@@ -229,4 +311,7 @@
 
 void OverlayFileSystem::pushOverlay(IntrusiveRefCntPtr<FileSystem> FS) {
   FSList.push_back(FS);
+  // Synchronize added file systems by duplicating the working directory from
+  // the first one in the list.
+  FS->setCurrentWorkingDirectory(getCurrentWorkingDirectory().get());
 }
----------------
klimek wrote:
> If I read this correctly, it gets and sets the pwd from FS for the first one? That seems superfluous.
> (perhaps change the constructor to not use pushOverlay)
Good catch!

================
Comment at: lib/Basic/VirtualFileSystem.cpp:476
@@ +475,3 @@
+  std::error_code close() override { return std::error_code(); }
+  void setName(StringRef Name) override { Parent.getStatus().setName(Name); }
+};
----------------
klimek wrote:
> Is this used in the File interface? It seems - strange... is this really a "mv" implementation?
Gone it is.

================
Comment at: lib/Basic/VirtualFileSystem.cpp:546-550
@@ +545,7 @@
+        // End of the path, create a new file.
+        Status Stat(Path, "", getNextVirtualUniqueID(),
+                    llvm::sys::TimeValue(ModificationTime), 0, 0,
+                    Buffer->getBufferSize(),
+                    llvm::sys::fs::file_type::regular_file,
+                    llvm::sys::fs::all_all);
+        Dir->addChild(Name, llvm::make_unique<detail::InMemoryFile>(
----------------
klimek wrote:
> I think we'll want to get some of these into the interface. But that's fine in a follow-up patch. Add a FIXME though.
FIXME added.

================
Comment at: lib/Basic/VirtualFileSystem.cpp:800
@@ -455,1 +799,3 @@
 
+  std::string WorkingDirectory;
+
----------------
klimek wrote:
> Doesn't the YAML FS work on the same pwd as the current process?
Changed the methods to forward to the inner FS of the yaml overlay.


http://reviews.llvm.org/D13430