ilya-biryukov added a comment.

In https://reviews.llvm.org/D37474#863335, @cameron314 wrote:

> Looking at the way remapped buffers are handled, I just remembered that they 
> must exist on the file system (at the very least, in a directory that exists) 
> or the remapping is not taken into account. So that pretty much rules out the 
> other approach, I think.


You are right, thanks for pointing this out. Fiddling with `VFS` seems like an 
only option.



================
Comment at: lib/Frontend/ASTUnit.cpp:1014
+/// with another virtual file system.
+class PCHOverlayFileSystem : public vfs::FileSystem
+{
----------------
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.


================
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
----------------
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`).


================
Comment at: unittests/Frontend/CMakeLists.txt:9
   CodeGenActionTest.cpp
+  PchPreambleTest.cpp
   )
----------------
Maybe rename to `PCHPreambleTest`?
LLVM code generally capitalizes all letters in abbreviations.


https://reviews.llvm.org/D37474



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to