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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.


================
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.
----------------
sammccall wrote:
> Simpler just as "/// Requires that CanReuse() is true."
Much more concise this way, thanks.


================
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
----------------
sammccall wrote:
> 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.
> 
Thanks, your wording looks much better.


Repository:
  rC Clang

https://reviews.llvm.org/D41990