rsmith added inline comments.

================
Comment at: include/clang/AST/ASTConsumer.h:163
+  /// The caller takes ownership on the returned pointer.
+  virtual std::unique_ptr<PPCallbacks> 
CreatePreprocessorCallbacks(Preprocessor &PP);
 };
----------------
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.


================
Comment at: lib/CodeGen/CMakeLists.txt:76
   ItaniumCXXABI.cpp
+  MacroPPCallbacks.cpp
   MicrosoftCXXABI.cpp
----------------
This file is missing from the diff.


================
Comment at: lib/CodeGen/CodeGenModule.cpp:28
 #include "CoverageMappingGen.h"
+#include "MacroPPCallbacks.h"
 #include "TargetInfo.h"
----------------
This file is missing from the diff.


https://reviews.llvm.org/D16135



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to