[PATCH] D73086: [DWARF5] Added support for debug_macro section parsing and dumping in llvm-dwarfdump.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 24 02:39:52 PDT 2020
jhenderson added inline comments.
================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:487
+ DObj->getMacroSection().Data))
+ parseAndDumpMacroMacinfo(Macro, OS, DumpOpts, /*IsLittleEndian=*/true,
+ dwarf::Macro);
----------------
SouraVX wrote:
> 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 :)
Most of the other sections have a pattern of "get/parse" followed by "dump", often in one line, without the error check. I think we should follow that if possible (but with the error check, or possibly by initialising the pointer to some "default" empty section in the event of a failure). In other words, yes, esssentially what you suggested. In particular, this will allow the parsing code to be used and tested independently of the printing.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73086/new/
https://reviews.llvm.org/D73086
More information about the llvm-commits
mailing list