[PATCH] D73086: [DWARF5] Added support for debug_macro section parsing and dumping in llvm-dwarfdump.

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX marked 2 inline comments as done.
SouraVX added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:111-113
+    MACINFO = 1,
+    MACINFODWO,
+    MACRO
----------------
jhenderson wrote:
> Why did you change the naming style of this? LLVM coding standards say that enums should use the same naming scheme as types: https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly.
> 
> Also, why the `= 1`?
Ah Sorry, I renamed them this way, to avoid confusion of naming while calling
`parseMacroOrMacinfo(Macro, getRecoverableErrorHandler(), MACRO);`

I'll change this as per Coding Standard.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:114-115
+    MACRO
+    /* FIXME: Add support for
+   .debug_macro.dwo section */
+  };
----------------
jhenderson wrote:
> Use C++ style comments, and put this all on one line. Also add missing trailing full stop.
Ah, seems like clang-formatting is getting in way(maybe due to /**/ style comment), I'll correct it such that clang-format does't put a new line.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:300
+  auto Macro = std::make_unique<DWARFDebugMacro>();
+  auto parseAndDump = [&](DWARFDataExtractor &Data, bool IsMacro) {
+    if (Error Err = Macro->parse(getStringExtractor(), Data, IsMacro))
----------------
jhenderson wrote:
> `ParseAndDump`. This is a (callable) object, not a function.
Sorry, didn't get what you meant here ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73086/new/

https://reviews.llvm.org/D73086