dexonsmith added inline comments.
================ 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; ---------------- jansvoboda11 wrote: > 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. @kadircet , WDYT? 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