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

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h:288
+  const DWARFDebugMacro *
+  parseMacroOrMacinfo(std::unique_ptr<DWARFDebugMacro> &MacroPtr,
+                      function_ref<void(Error)> RecoverableErrorHandler,
----------------
  - Make `parseMacroOrMacinfo()` private.
  - `parseMacroOrMacinfo()` should return a newly created `std::unique_ptr<DWARFDebugMacro>`.
  - Remove the `MacroPtr` argument. Caching the result is the responsibility of getters.
  - Remove the `RecoverableErrorHandler` argument. All getters pass the same value, and this method, being a non-static member of the same class, also has access to it.






================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:488-490
+    auto Macinfo = getDebugMacinfo();
+    if (Macinfo)
+      Macinfo->dump(OS);
----------------
This might be simplified to
```
    if (auto Macinfo = getDebugMacinfo())
      Macinfo->dump(OS);
```


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:867-871
 const DWARFDebugMacro *DWARFContext::getDebugMacinfo() {
   if (Macinfo)
     return Macinfo.get();
+  return parseMacroOrMacinfo(Macinfo, getRecoverableErrorHandler(), MACINFO);
+}
----------------
The getters should look like
```
const DWARFDebugMacro *DWARFContext::getDebugMacinfo() {
  if (!Macinfo)
    Macinfo = parseMacroOrMacinfo(MACINFO);
  return Macinfo.get();
}
```


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

https://reviews.llvm.org/D73086