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