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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

As a heads-up, I am off work after today for 6 weeks, so won't be able to review further after today. Sorry about that.



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7563
+template <class ELFT>
+void JSONELFDumper<ELFT>::printSymbol(const Elf_Sym &Symbol, unsigned SymIndex,
+                                      DataRegion<Elf_Word> ShndxTable,
----------------
Could we avoid the mass code-duplication by just introducing another function "printSymbolOtherField" (or something like that) that is implemented differently for the two dumper types?

Maybe even "printZeroSymbolOtherField" so that only the very niche bit that needs to differ is distinguished. This function could then just delegate to the regular print symbol st_other code, to avoid needing to duplicate that in any way.

With this change, I think you could minimise the additional testing to just demonstrate basic behaviour difference for zero/non-zero st_other values, without needing to have additional cases for the various other aspects of this field printing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137088