[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 3 inline comments as done.
SouraVX added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:484
+                 DObj->getMacroSection().Data)) {
+    parseMacroOrMacinfo(Macro, OS, DumpOpts, /*IsLittleEndian=*/true,
+                        dwarf::Macro);
----------------
ikudrin wrote:
> I am not sure that `Macro` as an in-out parameter of a void method is the perfect design. It would be better if `parseMacroOrMacinfo()` would just return `std::unique_ptr<DWARFDebugMacro>`. The code here, in that case, will look like:
> 
> ```
> if (!Macro)
>   Macro = parseMacroOrMacinfo(...);
> if (Macro)
>   Macro->dump(OS);
> ```
> 
> Much more clear, no?
Removed this
```if (!Macro)  --this seemed redundant to me since during first parsing of section, this will always be NULL.
  Macro = parseMacroOrMacinfo(...);
```



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

https://reviews.llvm.org/D73086