[PATCH] D16135: Macro Debug Info support in Clang

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.


================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:62
+    : DebugInfo(DI), PP(PP), FirstInclude(false), FirstIncludeDone(false),
+      CommandIncludeFiles(0), SkipFiles(2) {
+  Parents.push_back(nullptr);
----------------
aprantl wrote:
> Comment why SkipFiles is initialized to 2?
Thanks. I think this comment should now go into the header and be promoted to the Docygen comment for the CommandIncludeFiles. Also consider doing `CommandIncludeFiles = 0` in the class definition.


================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:31
+      MacroInfo::arg_iterator AI = MI.arg_begin(), E = MI.arg_end();
+      for (; AI + 1 != E; ++AI) {
+        Name << (*AI)->getName();
----------------
std::next() ?


================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:44
+    if (MI.isGNUVarargs())
+      Name << "..."; // #define foo(x...)
+
----------------
LLVM style prefers comments to be full sentences and above the code.


================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:83
+      break;
+  // Fall though!
+  case MainFileScope:
----------------
I think there is an LLVM_FALLTHROUGH macro (or something to that end)


================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:154
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  // Only care about "include" directives.
+  if (!IncludeTok.is(tok::identifier))
----------------
#include as opposed to #import? If so, why?


================
Comment at: lib/CodeGen/ModuleBuilder.cpp:68
     SmallVector<CXXMethodDecl *, 8> DeferredInlineMethodDefinitions;
+    CGDebugInfo *DebugInfoRef;
 
----------------
What does Ref stand for?


https://reviews.llvm.org/D16135