klimek added inline comments.
================
Comment at: include/clang/Frontend/PrecompiledPreamble.h:124
+/// CanReusePreamble + AddImplicitPreamble to make use of it.
+class PrecompiledPreamble {
+public:
----------------
ilya-biryukov wrote:
> klimek wrote:
> > If a user doesn't care about the things above this class, can we move those
> > into an extra header?
> Do you have any suggestions of where to put it and how to call it?
> I didn't think it's a good idea to put something like 'PreambleFileHash.h'
> and 'TempPCHFile.h' into 'include/clang/Frontend/'. (Given that they are
> essential an implementation detail of PrecompiledPreamble, exposing them in
> public include folders seems like a bad idea).
TempPCHFile looks like something we just want to put into the .cc file and
store as a unique_ptr.
PreambleFileHash seems fine as an extra header.
================
Comment at: include/clang/Frontend/PrecompiledPreamble.h:157-160
+ /// We don't expose PCHFile to avoid changing interface when we'll add an
+ /// in-memory PCH, so we declare this function as friend so that it has
access
+ /// to PCHFile field.
+ friend void AddImplicitPreamble(CompilerInvocation &CI,
----------------
ilya-biryukov wrote:
> klimek wrote:
> > Why not make it a member instead?
> To keep BuildPreamble, CanReusePreamble and AddImplicitPreamble close to each
> other.
> I.e. PrecompiledPreamble only stores data used by these functions.
>
> We could make all of those members, of course. Do you think that would make
> API better?
Generally, if these are closely coupled to implementation details of
PrecompiledPreample, I think that coupling is strong enough to warrant making
them members.
On the other hand, making some functions members, and others non-members, and
putting them next to each other in the .cc file also would work.
https://reviews.llvm.org/D34287
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
