[PATCH] D111658: Add JSON output skeleton to llvm-readelf

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D111658#3136536 <https://reviews.llvm.org/D111658#3136536>, @Jaysonyan wrote:

> In D111658#3121940 <https://reviews.llvm.org/D111658#3121940>, @Jaysonyan wrote:
>
>> I think that having `JSONScopedPrinter` as a subclass of `ScopedPrinter` would have more downsides than upsides without major refactoring to how we interact with `ScopedPrinter` in `llvm-readobj`. I believe the biggest driving force to having `JSONScopedPrinter` as a subclass to `ScopedPrinter` is with the hope that each implementation of `ObjDumper` can be agnostic to the type of `ScopedPrinter` it is using (json or regular). Although I don't believe that this is possible for a few reasons:
>
> Since making this comment I've been thinking of possible solutions to these problems. I've uploaded D114052: Add JSONScopedPrinter as a subclass to ScopedPrinter with history stack <https://reviews.llvm.org/D114052> and D114053: Add JSONScopedPrinter as a subclass to ScopedPrinter with array output <https://reviews.llvm.org/D114053> which are two approaches that attempt to solve the 3 issues laid out in this comment. Hopefully either of those 2 PRs (or this one) will be a sufficient solution to meet our needs.

Thanks for thinking about this, and sorry for not coming back sooner (I've been dealing with deadlines, but am now free for a bit, so hopefully will have a bit more time). I quite like the "history stack" idea of the two new approaches, and I think the solutions to the other cases are viable. Note however that your two new diffs appear to be identical? Either that or I'm missing where the problem 3 approach differs in the implementation.

Do you have a preference for an approach of the three?


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

https://reviews.llvm.org/D111658