klimek added inline comments.
================
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();
+ }
----------------
Return errc::success?
================
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 {
----------------
What's the proposed fix? :)
================
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());
}
----------------
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)
================
Comment at: lib/Basic/VirtualFileSystem.cpp:437
@@ +436,3 @@
+ virtual ~InMemoryNode() {}
+ Status &getStatus() { return Stat; }
+ InMemoryNodeKind getKind() const { return Kind; }
----------------
Do we want to make this const?
================
Comment at: lib/Basic/VirtualFileSystem.cpp:439
@@ +438,3 @@
+ InMemoryNodeKind getKind() const { return Kind; }
+ virtual std::string toString(unsigned Indent) = 0;
+};
----------------
This probably wants to be const.
================
Comment at: lib/Basic/VirtualFileSystem.cpp:461
@@ +460,3 @@
+class InMemoryFileAdaptor : public File {
+ InMemoryFile &Parent;
+
----------------
I think parent is a misleading name here (parent implies a hierarchy, but this
is a simple adapter). Perhaps "Node".
================
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); }
+};
----------------
Is this used in the File interface? It seems - strange... is this really a "mv"
implementation?
================
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>(
----------------
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.
================
Comment at: lib/Basic/VirtualFileSystem.cpp:800
@@ -455,1 +799,3 @@
+ std::string WorkingDirectory;
+
----------------
Doesn't the YAML FS work on the same pwd as the current process?
================
Comment at: unittests/Tooling/RewriterTestContext.h:42-45
@@ +41,6 @@
+ DiagnosticPrinter(llvm::outs(), &*DiagOpts),
+ InMemoryFileSystem(new vfs::InMemoryFileSystem),
+ OverlayFileSystem(
+ new vfs::OverlayFileSystem(vfs::getRealFileSystem())),
+ Files(FileSystemOptions(), OverlayFileSystem),
+ Sources(Diagnostics, Files), Rewrite(Sources, Options) {
----------------
Perhaps add a FIXME: To make these tests truly in-memory, we need to overlay
the builtin headers.
http://reviews.llvm.org/D13430
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
