ilya-biryukov added inline comments.
================
Comment at: include/clang/Basic/VirtualFileSystem.h:313
+/// \brief Creates a \p vfs::OverlayFileSystem which overlays the given file
+/// system above the 'real' file system, as seen by the operating system.
+IntrusiveRefCntPtr<OverlayFileSystem>
----------------
I suggest leaving out the quotes around 'real'
================
Comment at: lib/Basic/VirtualFileSystem.cpp:328
void OverlayFileSystem::pushOverlay(IntrusiveRefCntPtr<FileSystem> FS) {
+ // FIXME: OverlayFS containing another one in its stack could be flattened.
FSList.push_back(FS);
----------------
I generally agree that it might be useful, but given that we can't use
`dynamic_cast` in LLVM code addressing this `FIXME` is probably not worth the
effort.
And this patch is probably not the right place to add this comment, since it
doesn't change `OverlayFileSystem` in any way.
================
Comment at: unittests/Tooling/ToolingTest.cpp:411
InMemoryFileSystem->addFile(
- "a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
+ "/a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
----------------
Using `'/'` in paths would probably break on Windows.
Why do we need to add it in the first place?
================
Comment at: unittests/Tooling/ToolingTest.cpp:435
+ newFrontendActionFactory<SyntaxOnlyAction>());
+ EXPECT_NE(0, Tool.run(Action.get()));
+}
----------------
It's not clear what this tests expects when looking at the code
A comment would be helpful.
Also consider matching on an actual error diagnostic (i.e. "<cstdio> not
found", right?)
https://reviews.llvm.org/D45094
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
