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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 11 01:56:08 PDT 2017


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Looks good, just a few minor style comments.



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


================
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(); }
----------------
NIT: comment should end with a full stop.


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


================
Comment at: lib/Frontend/ASTUnit.cpp:1053
+  IntrusiveRefCntPtr<vfs::FileSystem> RealFS = vfs::getRealFileSystem();
+  if (OverrideMainBuffer && VFS && RealFS && VFS != RealFS &&
+      !VFS->exists(Preamble->GetPCHPath())) {
----------------
The check `&& RealFS` is redundant and can be removed.


https://reviews.llvm.org/D37474





More information about the cfe-commits mailing list