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

[Don Hinton via Phabricator via llvm-commits]

hintonda added a comment.

In D63565#1553263 <https://reviews.llvm.org/D63565#1553263>, @jhenderson wrote:

> 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'!).


That's because they're processing `-h` after `cl::ParseCommandLineOptions` returns.  But as you say, InputFiles requires at least one argument, so `cl::ParseCommandLineOptions` never does return if this condition isn't met.  The only way to fix that is to remove the special `-h` processing.

  static list<std::string> InputFiles(Positional, OneOrMore,
                                      desc("<input files>"), cat(DsymCategory));


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63565