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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
