[PATCH] D16135: Macro Debug Info support in Clang

[Richard Smith via cfe-commits]

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