[PATCH] D37474: [PCH] Allow VFS to be used for tests that generate PCH files

[Cameron via Phabricator via cfe-commits]

cameron314 added inline comments.


================
Comment at: include/clang/Frontend/PrecompiledPreamble.h:100
 
   /// PreambleBounds used to build the preamble
   PreambleBounds getBounds() const;
----------------
ilya-biryukov wrote:
> Not introduced by this change, but could you also add a full stop here for constistency?
Sure.


================
Comment at: include/clang/Frontend/PrecompiledPreamble.h:103
 
+  /// The temporary file path at which the preamble PCH was placed
+  StringRef GetPCHPath() const { return PCHFile.getFilePath(); }
----------------
ilya-biryukov wrote:
> NIT: comment should end with a full stop.
OK!


================
Comment at: lib/Frontend/ASTUnit.cpp:1021
+  if (Buf)
+    PCHFS->addFile(PCHFilename, 0, std::move(*Buf));
+  IntrusiveRefCntPtr<vfs::OverlayFileSystem>
----------------
ilya-biryukov wrote:
> Maybe return original `VFS` if `PCHFilename` could not be read and not create any empty overlays?
Makes sense, will do.


================
Comment at: lib/Frontend/ASTUnit.cpp:1053
+  IntrusiveRefCntPtr<vfs::FileSystem> RealFS = vfs::getRealFileSystem();
+  if (OverrideMainBuffer && VFS && RealFS && VFS != RealFS &&
+      !VFS->exists(Preamble->GetPCHPath())) {
----------------
ilya-biryukov wrote:
> The check `&& RealFS` is redundant and can be removed.
It's not redundant, but looking at the implementation of `vfs::getRealFileSystem` it will always return a non-null pointer.


https://reviews.llvm.org/D37474