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


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:855
+                               /*IsMacro=*/true))
+    return std::move(Err);
+  return Macro.get();
----------------
jhenderson wrote:
> ikudrin wrote:
> > If you call this method twice, and the first call returns an error, the second call will still return some partially parsed `DWARFDebugMacro` object. This is inconsistent. You should call `Macro.reset()` before returning an error.
> > 
> > The same for two other similar methods.
> This sort of change is exactly why I think we should avoid duplicating the logic in three places. It would have been easy to add it only in one place.
I'm investigating the opportunity you discussed/mentioned in last comment(Thanks for that!) If found with constructive cases, will push a patch or two for review.


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

https://reviews.llvm.org/D73086