[PATCH] D41990: [Frontend] Allow to use PrecompiledPreamble without calling CanReuse

[Sam McCall via Phabricator via cfe-commits]

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