aaboud added a comment. Thanks Richard for reviewing the patch and for the comments. Please, see answers below.
Regards, Amjad ================ Comment at: include/clang/AST/ASTConsumer.h:163 + /// The caller takes ownership on the returned pointer. + virtual std::unique_ptr<PPCallbacks> CreatePreprocessorCallbacks(Preprocessor &PP); }; ---------------- rsmith wrote: > aaboud wrote: > > Richard, > > I know that you suggested not to introduce the Preprocessor in anyway to > > the ASTConsumer. > > However, as I could not understand the other way to support macros in Clang > > without applying a huge redesign, I decided to upload the code I suggested > > in the proposal. > > http://lists.llvm.org/pipermail/llvm-dev/2015-November/092449.html > > > > If you still think that this approach is not the right one, I would > > appreciate it if you can explain again how to implement the macro support > > differently. > Don't add this to `ASTConsumer`; an AST consumer and a PP consumer are two > largely separate ideas. Instead, you could create and register a > `PPCallbacks` object directly from `CodeGenAction::CreateASTConsumer`. In > fact, if you look there, a `PPCallbacks` subclass is already registered (for > code coverage); you can do something similar to register your > `MacroPPCallbacks` object. Thanks, I will try this approach, it is indeed cleaner. ================ Comment at: lib/CodeGen/CMakeLists.txt:76 ItaniumCXXABI.cpp + MacroPPCallbacks.cpp MicrosoftCXXABI.cpp ---------------- rsmith wrote: > This file is missing from the diff. Sorry about that, it seems when I updated the code these files were removed from the patch. Also, I need to improve the implementation in these two files. By the way, you can find an old version of them in the previous diff. I will update them and upload them to the review together with the above suggestion of yours. https://reviews.llvm.org/D16135 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
