[PATCH] D62256: [llvm-readobj] Implement GNU-style output for dynamic table

Jordan Rupprecht via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 22 13:53:50 PDT 2019


rupprecht added a subscriber: grimar.
rupprecht added a comment.

FYI there's another patch that changes .dynamic output, including the readelf output -- D62179 <https://reviews.llvm.org/D62179>. You may need to update that test if it gets committed first.

I do agree with the others that parens is better for compatibility. But I think it's fine to deviate with the other whitespace differences; breaking compatibility makes it easier to read the table. LGTM, but please wait for James/Ray to approve.

btw, I noticed other missing differences unrelated to this patch -- filed https://bugs.llvm.org/show_bug.cgi?id=41987.



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3348-3350
+  OS << "  Tag" << (Is64 ? "                " : "        ") << "Type"
+     << "                 "
+     << "Name/Value\n";
----------------
I know this (and the LLVMStyle below) is just copied from the old code... but I don't think there needs to be any `<<` between strings, they can be implicitly concatenated (or joined into a single string, I don't know why it needs to be so narrow)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62256





More information about the llvm-commits mailing list