[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC
Jan Svoboda via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 1 08:10:21 PST 2020
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: clang/include/clang/Frontend/PrecompiledPreamble.h:108
bool CanReuse(const CompilerInvocation &Invocation,
- const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds,
- llvm::vfs::FileSystem *VFS) const;
+ const llvm::MemoryBufferRef &MainFileBuffer,
+ PreambleBounds Bounds, llvm::vfs::FileSystem &VFS) const;
----------------
dexonsmith wrote:
> kadircet wrote:
> > why not accept a value directly here? (i.e. drop const and ref)
> Ah, yes, I've done this a few times, and it still seems not quite right. But the alternative also doesn't feel right when it's not necessary:
> ```
> #include "llvm/Basic/MemoryBufferRef.h"
> ```
> I'm happy either way since that file is fairly careful to avoid bloating includes.
I agree this looks a bit odd, but avoiding an unnecessary include seems like a good excuse.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91297/new/
https://reviews.llvm.org/D91297
More information about the cfe-commits
mailing list