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.
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.
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 

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
-      "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,;
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?)

cfe-commits mailing list

Reply via email to