[PATCH] D16135: Macro Debug Info support in Clang

[Adrian Prantl via cfe-commits]

aprantl added a comment.

Added a few stylistic comments and request for more documentation.


================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:1
@@ +1,2 @@
+//===--- MacroPPCallbacks.h ---------------------------------------*- C++ -*-===//
+//
----------------
Please run your patch through clang-format.

================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:62
@@ +61,3 @@
+    : DebugInfo(DI), PP(PP), FirstInclude(false), FirstIncludeDone(false),
+      CommandIncludeFiles(0), SkipFiles(2) {
+  Parents.push_back(nullptr);
----------------
Comment why SkipFiles is initialized to 2?

================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:74
@@ +73,3 @@
+      FirstIncludeFile = Loc;
+    }
+    if (CommandIncludeFiles) {
----------------
Comments?

================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:91
@@ +90,3 @@
+    }
+    else if (SkipFiles) {
+      if (!(--SkipFiles)) {
----------------
Please add a comment explaining this condition.

================
Comment at: lib/CodeGen/MacroPPCallbacks.h:34
@@ +33,3 @@
+  int CommandIncludeFiles;
+  int SkipFiles;
+  std::vector<llvm::DIMacroFile *> Parents;
----------------
Please comment the purpose of these fields.

================
Comment at: lib/CodeGen/MacroPPCallbacks.h:45
@@ +44,3 @@
+
+  /// \brief Callback invoked whenever a source file is entered or exited.
+  ///
----------------
Since r237417 LLVM enabled autobrief, so it's no longer recommended to use \brief for one-liners.


http://reviews.llvm.org/D16135