[PATCH] D54124: [llvm-readelf] Make llvm-readelf more compatible with GNU readelf.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added a comment.

In https://reviews.llvm.org/D54124#1292583, @jhenderson wrote:

> Okay, from my point of view, LGTM (with a nit). However, we should probably get some more buy-in on the RFC before breaking people's builds etc, so you might want to hold off on the commit for now.
>
> Any thoughts on how to drum up some more interest?


I'll ping the thread you started, and I'd like to commit on Monday if there are no objections. I'm hoping this change is not so controversial.

After this I'd like to switch to tablegen/liboption and support some merged args (e.g. "readelf -SW" seems to be a common usage)... that will be slightly more dangerous :)



================
Comment at: test/tools/llvm-readobj/symbols.test:14-15
+RUN: cmp %t.symbols %t.t
+RUN: llvm-readelf -s -elf-output-style LLVM %p/Inputs/trivial.obj.elf-i386 > %t.lowers
+RUN: cmp %t.symbols %t.lowers
+
----------------
jhenderson wrote:
> Nit: for consistency with the other filenames in this set of cases, maybe use %t.s, if you don't think it's too confusing with assembly?
I'd prefer .lowers (consistent with the sections test) instead of .s just to highlight that it's a lowercase s given the general confusion of -s and -S for both (sometimes) meaning --sections, even though the same doesn't apply to --symbols.

Also just to note -- I chose "lowers" and "uppers" instead of "s" and "S" just to make sure this didn't mess with any build bots that are case insensitive with filenames.


Repository:
  rL LLVM

https://reviews.llvm.org/D54124