[llvm] [llvm-readobj][COFF] Add JSON Output Style (PR #95074)

[James Henderson via llvm-commits]

https://github.com/jh7370 commented:

In principle, I support this change. I haven't looked at the detail yet, but some high-level comments for discussion:

1) Should we deprecate the existing `--elf-output-style` option in favour of a new generic `--output-style` option? This new option would apply to both COFF and ELF output and would take the existing `gnu`, `llvm` and `json` values. We'd need to decide what to do about the `gnu` value for COFF output (possibly reject it, possibly treat it as `llvm`). If we decide we want this option, it should be a separate, earlier PR, so that we never introduce `--coff-output-style`.

2) I think this PR needs splitting into multiple individual ones. The first (excluding any new option) should establish the basic framework for JSON output, i.e. the class and creating it when the option is specified appropriately, but doesn't do any of the implementation (i.e. the class is essentially a thin wrapper around the regular dumper class). Subsequent PRs would focus on adding support for JSON output one feature at a time. This will make reviewing them easier.

3) You'll want a release note for the whole thing at some point in the PR stack.

4) The testing for ELF JSON output is mostly in the same files as the other output styles. I'm not sure why file-summary-json.test isn't, but it seems to be the exception rather than the norm. I prefer having the format tested in the same file as the existing format, as it is easier to ensure we test the same set of edge cases that we care about.

Finally, I'm sure there's a valid one, but could you outline your motivation for having JSON support for COFF files, please?

https://github.com/llvm/llvm-project/pull/95074