[PATCH] D137088: [readobj] Give JSON output a custom printSymbol

Tue Nov 1 01:36:34 PDT 2022

jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

Rather than duplicating, then refactoring, do the refactor first and then use it, so that the code is reasonably clean in every landed commit.

Also, please provide before and after examples, and test cases for the fix. Also, please fix the title to use "llvm-readobj" in the commit message/patch description, not simply "readobj".

In D137088#3897022 <https://reviews.llvm.org/D137088#3897022>, @paulkirth wrote:

> FYI @jhenderson This whole stack is in need of tests, and I plan to address that. I think that if we find this stack preferable to D135419 <https://reviews.llvm.org/D135419>, then we can just abandon that approach.

Sorry, only just saw this comment. Either way, the before and after examples are needed for me to visualise the problems. One of the aims with the JSON output being built on top of the LLVM output was that they should have largely been the same structurally, just with some formatting details changing.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137088