ilya-biryukov 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");
----------------
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?


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

Reply via email to