[PATCH] D78736: [DWARF5]: Added support for dumping strx forms in llvm-dwarfdump

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D78736#2030810 <https://reviews.llvm.org/D78736#2030810>, @SouraVX wrote:

> In D78736#2030799 <https://reviews.llvm.org/D78736#2030799>, @dblaikie wrote:
>
> > (the usual convention  is to address outstanding comments that are made when a patch is approved (such as the naming and parentheses I mentioned) before committing (this is done to save an extra feedback loop when the changes seem sufficiently obvious to not need another check to ensure they were done as requested) - so if you could do those in a follow-up commit (no need to send it for review, but please follow-up here and mention the commit hashes of the commits that address the feedback))
>
>
> Ah Sorry! But I've addressed/incorporated `if (!StrOffset)` and `(&(*StrOffset)` related comments Correctly right?.


Oh, right, so you did! My mistake! (I thought I'd checked the Phab close email & thought it didn't mention any further changes - but it did! totally my mistake)

> Only the Variable renaming and Error message is remaining which I plan to address separately(Since making the error recoverable won't be trivial because, in case of error(MacroContribution missing or StrOffsets missing) we have to correctly skip the entire CU macro contribution and start parsing the next CU if it exist.).
>  Right now I'm rebasing rest of the 3 revision on top of this.

*nod* Sounds good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78736