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

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:487
+                 DObj->getMacroSection().Data))
+    parseAndDumpMacroMacinfo(Macro, OS, DumpOpts, /*IsLittleEndian=*/true,
+                             dwarf::Macro);
----------------
jhenderson wrote:
> I'm not particularly keen that dumping and parsing happen in the same function. It feels like those are two separate tasks that should be kept independent. Perhaps a small lambda near here instead would make sense to factor out the sequence "parse -> check not null -> dump"?
Are you suggesting something sort of - 
`parseMacroOrMacinfo`  handles parsing and error handling, this can be a member function instead of a lamba for uniformities sake.
```
if (shouldDump(Explicit, ".debug_macro", DIDT_ID_DebugMacro,
                  DObj->getMacroSection().Data)) {
     parseMacroOrMacinfo(Macro, OS, DumpOpts, /*IsLittleEndian=*/true,
                              dwarf::Macro);
     if (Macro)
     Macro->dump(OS);
   }
```
I'm okay with this too :)


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

https://reviews.llvm.org/D73086