[PATCH] D137094: [llvm-readobj] Fix "Section" output when emitting relocations in JSON

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6724
+template <class ELFT>
+void LLVMELFDumper<ELFT>::printRelocationInfo(const Elf_Shdr &Sec,
+                                              const StringRef Name,
----------------
jhenderson wrote:
> I wonder if this would be better named `printRelocationSectionInfo`, since it's not printing any info about the relocations themselves.
That's a good point. I'll update this to reflect.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7635
+  DictScope Group(this->W);
+  this->W.printNumber("SectionIdx", SecNdx);
+  ListScope D(this->W, "Relocs");
----------------
jhenderson wrote:
> Any particular reason we don't just name this key "SectionIndex" or similar?
No, I don't recall any a particular reason. I just went w/ the `Idx` suffix since it was still clear, and is a pretty common way to shorten or identify index variables. I don't have a preference here so I can change this when I address the other comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137094