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

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/llvm-vs-json-format.test:1
+# Check that the LLVM and JSON formats differ when their implementations diverge.
+
----------------
jhenderson wrote:
> Having looked at this again, rather than a separate test showing the differences, I think I'd prefer the test cases being added to existing tests that test LLVM and GNU output styles. For the st_other case, that would be https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/symbol-visibility.test (which is essentially the "main" st_other test) and potentially both https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/aarch64-symbols-stother.test and https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test.
> 
> I was hoping we'd be able to share CHECK patterns, but of course, for JSON output, the keys are quoted, so that doesn't really work.
I'm a bit concerned that the number of tests and test cases will start to rapidly grow as we progress through this stack, since there is basically zero JSON testing as it stands, but let's see how this goes.

I'd also like to keep this test since it documents how the two formats differ in a single place, and subsequent patches build on top of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137088