[PATCH] D137089: [llvm-readobj] Fix JSON output for Relocations

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 17 01:24:17 PDT 2023


jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with some nits.



================
Comment at: llvm/test/tools/llvm-readobj/ELF/relocations.test:114
 
+# Show that JSON relocations are always expanded
+# RUN: llvm-readobj -r %t64 --elf-output-style=JSON --pretty-print \
----------------
jhenderson wrote:
> 
Nit isn't fully addressed (missing full stop at end of comment).


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6785-6786
+void LLVMELFDumper<ELFT>::printExpandedRelRelaReloc(const Relocation<ELFT> &R,
+                                                    const StringRef SymbolName,
+                                                    const StringRef RelocName) {
+  DictScope Group(W, "Relocation");
----------------
Do these really make sense as `const`? I'm pretty sure `StringRef` isn't usually a `const` since you can't modify the string.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6797
+void LLVMELFDumper<ELFT>::printDefaultRelRelaReloc(const Relocation<ELFT> &R,
+                                                   const StringRef SymbolName,
+                                                   const StringRef RelocName) {
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137089



More information about the llvm-commits mailing list