[PATCH] D70705: [DebugInfo]Support for debug_macinfo.dwo section in llvm and llvm-dwarfdump.

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX added a comment.

In D70705#1767133 <https://reviews.llvm.org/D70705#1767133>, @dblaikie wrote:

> This looks to be missing test coverage - this one patch adds both parsing and emission support for macinfo.dwo, but only tests the parsing support (there's no llc test that tests the emission)
>
> Please separate patches like this into, first, a parsing patch (adds libDebugInfoDWARF functionality and test(s) like the one you have here) and then in a separate patch the emission support & tests for that.
>
> (to be fair, I'm sort of OK (& have done myself in the past) with /only/ testing the emission, and getting test coverage for the parsing as a consequence of the emission testing, without separate parsing test coverage - but the opposite (only testing the parsing, without the emission) isn't sufficient)




> (you can ignore my other feedback, since this is just not the right direction in general - but the feedback might be useful for ideas on future patches for this and other features)

Thanks David, I understand a lot cleanup has to be done for unification and to build DWARFv5 macro on top of this. I'll add emission test case soon. This `macinfo.dwo emission` and `loclist parsing test case` were in my TODO list, but I was carried away by other stuff. Will do those in separate patches/reviews.
Thanks again for sharing the implementation feedback also -- will work your inputs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70705