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

Miguel A. Arroyo via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 11 10:40:38 PDT 2024


mayanez wrote:

> Should we deprecate the existing --elf-output-style option in favour of a new generic --output-style option? 

I considered this option andI opted against it to keep the changes localized to `COFF` for the sake of the PR. I'd be happy to make this change as a separate PR per your suggestion. I'd add we probably also need to handle `MachO`. I think simply reporting that the style is unsupported (ie. `gnu`) is a safer choice. The same type of error reporting would likely fit well when considering `MachO` until someone needs it implemented there.

> I think this PR needs splitting into multiple individual ones. 

I agree.  I wasn't sure what the process is to merge in features piecewise before things are fully complete. An alternative would be for me to split the commits in this PR following the same suggestions you proposed. I'm happy to go with either approach.

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

Noted, especially if we go with a generic `--output-style`.

> 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.

Fair enough. This shouldn't be a problem to change. I can additionally update `file-summary-json.test` in a separate PR to make it consistent if you think it prudent.

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

Of course. We're using `llvm-readobj` to analyze our build artifacts for various properties to make sure things match certain expectations (eg. which imports are being used, etc). Instead of writing a script to do string parsing on the `LLVM` style output, extending the tool to be able to output JSON makes the process of scripting the output a much more simple/robust process.

Also, not sure what the process is, but while we have these discussions, I figured I should change the PR to a draft. Let me know if you prefer something different.


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


More information about the llvm-commits mailing list