[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.

[William Enright via Phabricator via cfe-commits]

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:
> 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?


Repository:
  rC Clang

https://reviews.llvm.org/D39375