sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: include/clang/Frontend/PrecompiledPreamble.h:107
/// is accessible.
- /// For in-memory preambles, PrecompiledPreamble instance continues to own
- /// the MemoryBuffer with the Preamble after this method returns. The caller
- /// is reponsible for making sure the PrecompiledPreamble instance outlives
- /// the compiler run and the AST that will be using the PCH.
+ /// Callers of this function must ensure that that CanReuse() returns true
for
+ /// the specified file before calling this function.
----------------
Simpler just as "/// Requires that CanReuse() is true."
================
Comment at: include/clang/Frontend/PrecompiledPreamble.h:119
+ /// the PCH stored by this instance. This function is similar to
+ /// AddImplicitPreamble, but does require checking that CanReuse() returns
+ /// true prior to calling this function. Using this function allows to avoid
----------------
i think you mean "does not".
I find some of the wording here confusing - "does not require checking" seems
like too weak a claim. It's also a bit long, and hard to quickly see at a
glance which function you might want to call.
I'd suggest:
/// Configure \CI to use this preamble for \p MainFileBuffer.
/// Like AddImplicitPreamble, but doesn't assume or check CanReuse()!
/// If this preamble doesn't match the file, it may parse differently.
================
Comment at: lib/Frontend/PrecompiledPreamble.cpp:683
+ llvm::MemoryBuffer *MainFileBuffer) const {
+ assert(VFS && "VFS must not be null");
+
----------------
nit: this is just assert(VFS)
Repository:
rC Clang
https://reviews.llvm.org/D41990
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
