Nebiroth added inline comments.
================
Comment at: lib/Frontend/PrecompiledPreamble.cpp:242
std::shared_ptr<PCHContainerOperations> PCHContainerOps, bool
StoreInMemory,
- PreambleCallbacks &Callbacks) {
+ PreambleCallbacks &Callbacks, std::unique_ptr<PPCallbacks> PPCallbacks) {
assert(VFS && "VFS is null");
----------------
ilya-biryukov wrote:
> Nebiroth wrote:
> > ilya-biryukov wrote:
> > > Could we add a method to `PreambleCallbacks` to create `PPCallbacks`
> > > instead?
> > > Otherwise we have both `MacroDefined` in `PreambleCallbacks` and a
> > > separate set of PPCallbacks, so we have two ways of doing the same thing.
> > >
> > > ```
> > > class PreambleCallbacks {
> > > public:
> > > // ...
> > >
> > > /// Remove this.
> > > virtual void HandleMacroDefined(...);
> > >
> > > // Add this instead.
> > > virtual std::unique_ptr<PPCallbacks> createPPCallbacks();
> > >
> > > }
> > > ```
> > >
> > > Alternatively, follow the current pattern in `PreambleCallbacks` and add
> > > some extra functions from the `PPCallbacks` interface to it. This would
> > > not require changes to the existing usages of `PrecompiledPreamble` in
> > > `ASTUnit`. Albeit, I'd prefer the first option.
> > > ```
> > > class PreambleCallbacks {
> > > public:
> > > // ...
> > >
> > > // Keep this
> > > virtual void HandleMacroDefined(...);
> > > // Add the ones you need, e.g.
> > > virtual void InclusionDirective(...);
> > > virtual void FileChanged(...);
> > > };
> > > ```
> > If we were to do that, one would then be required to define a wrapper class
> > for PPCallbacks and create an instance of it inside createPPCallbacks() for
> > the purpose of creating a unique_ptr? Then that unique_ptr would be sent
> > from within the PreambleCallbacks parameter in the Build function?
> We're already passing our own wrapper around `PreambleCallbacks` anyway (see
> `PreambleMacroCallbacks`), we'll pass the `unique_ptr<PPCallbacks>` instead.
> But, yes, the clients will have to write their own implementation of
> `PPCallbacks` and pass it as `unique_ptr`. Is there something wrong with that?
>
> Or, have I misunderstood the question entirely?
No this is fine. I was just making sure I understood correctly.
Repository:
rC Clang
https://reviews.llvm.org/D39375
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
