cameron314 added inline comments.
================ Comment at: lib/Frontend/ASTUnit.cpp:1014 +/// with another virtual file system. +class PCHOverlayFileSystem : public vfs::FileSystem +{ ---------------- ilya-biryukov wrote: > Maybe create a combination of `InMemoryFileSystem` and `OverlayFileSystem` > instead of custom filtering implementation? > We really need to read only a single file given that `ASTUnit` never creates > directory PCHs. > I bet it would make the code simpler and less error-prone. I hadn't thought of that. Yes, that makes sense and is more concise. ================ Comment at: lib/Frontend/ASTUnit.cpp:1090 + IntrusiveRefCntPtr<vfs::FileSystem> RealFS = vfs::getRealFileSystem(); + if (OverrideMainBuffer && VFS && RealFS && VFS != RealFS) { + // We have a slight inconsistency here -- we're using the VFS to ---------------- ilya-biryukov wrote: > Maybe create a PCH overlay only when `!VFS->exists(/*PreamblePath...*/)`? > This seems like a good enough indication that `RealFS` is underneath the > `vfs::FileSystem` instance, even if pointer equality does not hold (i.e. in > case of overlays over `RealFS`). Makes sense. ================ Comment at: unittests/Frontend/CMakeLists.txt:9 CodeGenActionTest.cpp + PchPreambleTest.cpp ) ---------------- ilya-biryukov wrote: > Maybe rename to `PCHPreambleTest`? > LLVM code generally capitalizes all letters in abbreviations. Oops, overlooked this one. Will do! https://reviews.llvm.org/D37474 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits