[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