[PATCH] D63565: [llvm-dwarfdump] Remove unnecessary explicit -h behaviour

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 21 03:06:52 PDT 2019


jhenderson added a comment.

In D63565#1552064 <https://reviews.llvm.org/D63565#1552064>, @hintonda wrote:

> In D63565#1552004 <https://reviews.llvm.org/D63565#1552004>, @jhenderson wrote:
>
> > In D63565#1551983 <https://reviews.llvm.org/D63565#1551983>, @hintonda wrote:
> >
> > > LGTM, but you'll also want to fix `llvm/test/Support/check-default-options.txt`.  It checks your special handling to make sure the default `-h` support didn't break it.
> >
> >
> > Thanks, I wasn't aware of that one (and hadn't yet bothered running every lit test for this). Just to be clear, are you recommending that I just delete the bits of that test relating to llvm-dwarfdump, since it will now follow the default process, or do you think there should be something else to replace it? I can replace it with llvm-opt-report, which seems to do something similar to llvm-dwarfdump.
>
>
> Yes, it can be removed.  If you want, you can add it to the ones above that do use the new default `-h`, but I don't think that's necessary.  And yes, if there's another one that does something special, adding it, i.e., s/llvm-dwarfdump/llvm-opt-report/ makes sense.


Okay, I'm going to remove it and then commit, without replacing the removed test part. My initial evaluation was using an out-of-date llvm-opt-report, and I was unable to find any other suitable candidates. I reviewed the other tools, and the only tools I could find with explicit -h help handling are dsymutil and clang-offload-bundler. In both cases, I think the existing handling is a bug (in dsymutil at least, you actually have to provide a positional argument before it will process '-h'!).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63565





More information about the llvm-commits mailing list