[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#1288822, @jhenderson wrote:

> Wow, that's some spooky timing.


Happy belated Halloween! :)

> I didn't realise somebody else was thinking the exact same thing as me at the time! I brought this up on the mailing list earlier today, not realising that this review existed!

I'd like to keep that thread going, as there are more people reading llvm-dev than reading this patch.

> Some thoughts:
> 
> 1. Why not break -s for both llvm-readobj and llvm-readelf? Yes, it will cause some test churn, but I think we should seriously consider avoiding weird lingering issues like this if we can.

I'm unsure if that's allowed given how long the tool has existed. But I'm happy to do that if others agree it's a good idea.
As a measurement, I ran `ninja check-all` with -s removed, and ~450 tests broke. (Keep in mind I can't run arm/windows/etc. tests on my machine, so the real number is higher).

> 1. What do you think about removing -t as an alias for --symbols in llvm-readelf (and possibly llvm-readobj)? -t means --section-details in GNU readelf, so has nothing to do with symbols. Of course, llvm-readelf doesn't (currently?) support --section-details, but maybe if we're breaking the meaning of switches we should get it completely right?

Done -- looks like lld tests were the only user of this; changed those in https://reviews.llvm.org/rL346260.

> 1. Are we comfortable breaking the behaviour for end-users with no prior warning? In my email, I proposed a few options, including "deprecating" old switch behaviour for a release, or simply providing a new switch that can be enabled by default in downstream ports to change the meaning of incompatible switches like -s.

For llvm-readelf, I am comfortable, since it seems like this is the right behavior. Maybe it would be worth putting in release notes though.
I'm also fine doing it for llvm-readobj with some support.


Repository:
  rL LLVM

https://reviews.llvm.org/D54124