[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files
Cameron via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 8 10:33:14 PDT 2017
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
More information about the cfe-commits
mailing list