[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:1187
+// FIXME: This is needed, but not sure why.
+updateArgStr(Opt, NewArg, ChosenSubCommand);
+Opt->setArgStr(NewArg);

Looks like this is a bug in the way sub commands are handled, but I'd like to 
get feedback on how it *should* work.  

The problem is that if an option is added with `cl::sub(*AllSubCommands)` it 
gets added to all registered subcommands, including `TopLevelSubCommand`.  
However, `TopLevelSubCommand` isn't included in `Option::Subs`, so I have to 
update/remove via two commands, one at the parser level for the 
`ChosenSubCommand`, which happens to be `TopLevelSubCommand` in this case, and 
another for the Option itself, which doesn't have `TopLevelSubCommand` in it's 
subs.

Should `CommandLineParser::addOption` specifically exclude `TopLevelSubCommand` 
when it add subs to an Option?  That seems like a bug to me, but I'm not sure I 
grok the reason it was excluded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194539.
hintonda added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

- Add DefaultOption logic.

Still needs tests, but wanted to get early feedback on basic approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -144,6 +144,13 @@
   }
 
   void addOption(Option *O, SubCommand *SC) {
+// Handle DefaultOptions
+if (O->getMiscFlags() & cl::DefaultOption) {
+  std::string DefaultArg(O->ArgStr.str() + ":default_option");
+  SC->DefaultOptions.push_back(DefaultArg);
+  O->setArgStr(SC->DefaultOptions.back());
+}
+
 bool HadErrors = false;
 if (O->hasArgStr()) {
   // Add argument to the argument map!
@@ -1167,6 +1174,22 @@
   auto &SinkOpts = ChosenSubCommand->SinkOpts;
   auto &OptionsMap = ChosenSubCommand->OptionsMap;
 
+  // Handle DefaultOptions.
+  for (auto const &DA : ChosenSubCommand->DefaultOptions) {
+StringRef Arg(DA);
+StringRef Val;
+if (Option *Opt = LookupOption(*ChosenSubCommand, Arg, Val)) {
+  StringRef NewArg = Arg.substr(0, Arg.find(":"));
+  if (LookupOption(*ChosenSubCommand, NewArg, Val)) {
+removeOption(Opt, ChosenSubCommand);
+  } else {
+// FIXME: This is needed, but not sure why.
+updateArgStr(Opt, NewArg, ChosenSubCommand);
+Opt->setArgStr(NewArg);
+  }
+}
+  }
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2169,10 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::cat(GenericCategory), cl::sub(*AllSubCommands),
+  cl::DefaultOption);
+
 static cl::opt>
 HHOp("help-hidden", cl::desc("Display all available options"),
  cl::location(WrappedHiddenPrinter), cl::Hidden, cl::ValueDisallowed,
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -231,6 +234,7 @@
   SmallVector PositionalOpts;
   SmallVector SinkOpts;
   StringMap OptionsMap;
+  SmallVector DefaultOptions;
 
   Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
 };
@@ -270,7 +274,7 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option.
 
Index: llvm/docs/CommandLine.rst
===
--- llvm/docs/CommandLine.rst
+++ llvm/docs/CommandLine.rst
@@ -128,6 +128,7 @@
   USAGE: compiler [options]
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -194,6 +195,7 @@
   USAGE: compiler [options] 
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -1251,6 +1253,14 @@
   with ``cl::CommaSeparated``, this modifier only makes sense with a `cl::list`_
   option.
 
+.. _cl::DefaultOption:
+
+* The **cl::DefaultOption** modifier is used to specify that the option is a
+  default that can be overridden by application specific parsers. For example,
+  the ``-help`` alias, ``-h``, is registered this way, so it can be overridden
+  by applications that need to use the ``-h`` option for another purpose,
+  either as a regular option or an alias for another option.
+
 .. _response files:
 
 Response files
Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ c

[PATCH] D59746: [LibTooling] Fix '-h' option

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Just chiming in from a LLVM binutils developer perspective. Some of the 
binutils are missing -h as an alias, when they really should have it for 
compatibility (specifically I checked llvm-nm and llvm-symbolizer). As a 
result, a solution that auto-adds -h as an alias would be good, as long as we 
have the ability to override it somehow. I wouldn't mind having logic in 
llvm-objdump/llvm-readobj to explicitly override the short alias if it is 
required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Have a working prototype for aliases, but underneath, an option is an option, 
so should be able to support both without any trouble.  Will upload as soon as 
I clean it up and add tests, etc...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1459906 , @hintonda wrote:

> In D59746#1459826 , @hintonda wrote:
>
> > In D59746#1459672 , @klimek wrote:
> >
> > > If we can extend cl::alias to support this, we could give it a priority 
> > > and take the highest prio :)
> >
> >
> > Okay, I'll give it that old college try, and see if I can come up with 
> > something not too kludgy.  ;-)
>
>
> Btw, do you just want default aliases, or default options in general?


I hope you don't mind, but I've got a few more design questions:

- should the default api be public, usable by Clang Tooling, or just usable 
within CommandLine.cpp?
- which options should be hard-coded, i.e., status quo, and which should be 
defaulted.
- should we provide some sort of diagnostic to user to alert them that they 
have overwritten a defaulted options -- note this has to happen at runtime.  
Guess it could be a help option, a la hidden-help, that shows status of each 
option. I actually like the idea of option origin in general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1459826 , @hintonda wrote:

> In D59746#1459672 , @klimek wrote:
>
> > In D59746#1458539 , @hintonda 
> > wrote:
> >
> > > In D59746#1458461 , @hintonda 
> > > wrote:
> > >
> > > > In D59746#1458432 , @klimek 
> > > > wrote:
> > > >
> > > > > If we make it an alias by default, can somebody overwrite that?
> > > >
> > > >
> > > > Unfortunately, that produces a runtime error:
> > > >
> > > >   lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
> > > > bin/llvm-objdump -h
> > > >   : CommandLine Error: Option 'h' registered more than once!
> > > >   LLVM ERROR: inconsistency in registered CommandLine options
> > > >
> > > >
> > > > The operative lines:
> > > >
> > > >   llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
> > > > SectionHeadersShorter("h",
> > > >   llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
> > > > cl::desc("Alias for -help"), cl::aliasopt(HOp));
> > > >
> > >
> > >
> > > The other problem is that these are statics, so you can't count on the 
> > > order, i.e., did the user overwrite get processed before or after the one 
> > > in `CommandLine.cpp`?
> >
> >
> > If we can extend cl::alias to support this, we could give it a priority and 
> > take the highest prio :)
>
>
> Okay, I'll give it that old college try, and see if I can come up with 
> something not too kludgy.  ;-)


Btw, do you just want default aliases, or default options in general?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1459672 , @klimek wrote:

> In D59746#1458539 , @hintonda wrote:
>
> > In D59746#1458461 , @hintonda 
> > wrote:
> >
> > > In D59746#1458432 , @klimek 
> > > wrote:
> > >
> > > > If we make it an alias by default, can somebody overwrite that?
> > >
> > >
> > > Unfortunately, that produces a runtime error:
> > >
> > >   lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
> > > bin/llvm-objdump -h
> > >   : CommandLine Error: Option 'h' registered more than once!
> > >   LLVM ERROR: inconsistency in registered CommandLine options
> > >
> > >
> > > The operative lines:
> > >
> > >   llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
> > > SectionHeadersShorter("h",
> > >   llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
> > > cl::desc("Alias for -help"), cl::aliasopt(HOp));
> > >
> >
> >
> > The other problem is that these are statics, so you can't count on the 
> > order, i.e., did the user overwrite get processed before or after the one 
> > in `CommandLine.cpp`?
>
>
> If we can extend cl::alias to support this, we could give it a priority and 
> take the highest prio :)


Okay, I'll give it that old college try, and see if I can come up with 
something not too kludgy.  ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D59746#1458539 , @hintonda wrote:

> In D59746#1458461 , @hintonda wrote:
>
> > In D59746#1458432 , @klimek wrote:
> >
> > > If we make it an alias by default, can somebody overwrite that?
> >
> >
> > Unfortunately, that produces a runtime error:
> >
> >   lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
> > bin/llvm-objdump -h
> >   : CommandLine Error: Option 'h' registered more than once!
> >   LLVM ERROR: inconsistency in registered CommandLine options
> >
> >
> > The operative lines:
> >
> >   llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
> > SectionHeadersShorter("h",
> >   llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
> > cl::desc("Alias for -help"), cl::aliasopt(HOp));
> >
>
>
> The other problem is that these are statics, so you can't count on the order, 
> i.e., did the user overwrite get processed before or after the one in 
> `CommandLine.cpp`?


If we can extend cl::alias to support this, we could give it a priority and 
take the highest prio :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1458461 , @hintonda wrote:

> In D59746#1458432 , @klimek wrote:
>
> > If we make it an alias by default, can somebody overwrite that?
>
>
> Unfortunately, that produces a runtime error:
>
>   lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
> bin/llvm-objdump -h
>   : CommandLine Error: Option 'h' registered more than once!
>   LLVM ERROR: inconsistency in registered CommandLine options
>
>
> The operative lines:
>
>   llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
> SectionHeadersShorter("h",
>   llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
> cl::desc("Alias for -help"), cl::aliasopt(HOp));
>


The other problem is that these are statics, so you can't count on the order, 
i.e., did the user overwrite get processed before or after the one in 
`CommandLine.cpp`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1458432 , @klimek wrote:

> In D59746#1458424 , @hintonda wrote:
>
> > In D59746#1458086 , @klimek wrote:
> >
> > > In D59746#1440756 , @hintonda 
> > > wrote:
> > >
> > > > A better alternative would have been to add a cl::aliasopt for '-h' in 
> > > > llvm's CommandLineParser when '-help' was first added.  However, that's 
> > > > no longer possible since some llvm based tools already use '-h' for 
> > > > other purposes.
> > >
> > >
> > > Is that intentional? Can you point at samples?
> >
> >
> > A quick grep found these -- there could be more.
> >
> > llvm-opt-report adds `-h` for help and handles it after 
> > `ParseCommandLineOptions` returns.
> >  llvm-objdump adds `-h` as an alias for `--section-headers`.
> >  dsymutil adds `-h` for help and handles it after `ParseCommandLineOptions` 
> > returns.
> >  llvm-readobj adds `-h` as an alias for `--file-headers`.
> >  llvm-dwarfdump adds `-h` for help and handles it after 
> > `ParseCommandLineOptions` returns.
> >  llvm-mt rolls its own via llvm-tblgen.
> >
> > So, the only ones I found that aren't essentially aliases for help are in 
> > llvm-objdump and llvm-readobj.
>
>
> If we make it an alias by default, can somebody overwrite that?


Unfortunately, that produces a runtime error:

  lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
bin/llvm-objdump -h
  : CommandLine Error: Option 'h' registered more than once!
  LLVM ERROR: inconsistency in registered CommandLine options

The operative lines:

  llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
SectionHeadersShorter("h",
  llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
cl::desc("Alias for -help"), cl::aliasopt(HOp));




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D59746#1458424 , @hintonda wrote:

> In D59746#1458086 , @klimek wrote:
>
> > In D59746#1440756 , @hintonda 
> > wrote:
> >
> > > A better alternative would have been to add a cl::aliasopt for '-h' in 
> > > llvm's CommandLineParser when '-help' was first added.  However, that's 
> > > no longer possible since some llvm based tools already use '-h' for other 
> > > purposes.
> >
> >
> > Is that intentional? Can you point at samples?
>
>
> A quick grep found these -- there could be more.
>
> llvm-opt-report adds `-h` for help and handles it after 
> `ParseCommandLineOptions` returns.
>  llvm-objdump adds `-h` as an alias for `--section-headers`.
>  dsymutil adds `-h` for help and handles it after `ParseCommandLineOptions` 
> returns.
>  llvm-readobj adds `-h` as an alias for `--file-headers`.
>  llvm-dwarfdump adds `-h` for help and handles it after 
> `ParseCommandLineOptions` returns.
>  llvm-mt rolls its own via llvm-tblgen.
>
> So, the only ones I found that aren't essentially aliases for help are in 
> llvm-objdump and llvm-readobj.


If we make it an alias by default, can somebody overwrite that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1458086 , @klimek wrote:

> In D59746#1440756 , @hintonda wrote:
>
> > A better alternative would have been to add a cl::aliasopt for '-h' in 
> > llvm's CommandLineParser when '-help' was first added.  However, that's no 
> > longer possible since some llvm based tools already use '-h' for other 
> > purposes.
>
>
> Is that intentional? Can you point at samples?


A quick grep found these -- there could be more.

llvm-opt-report adds `-h` for help and handles it after 
`ParseCommandLineOptions` returns.
llvm-objdump adds `-h` as an alias for `--section-headers`.
dsymutil adds `-h` for help and handles it after `ParseCommandLineOptions` 
returns.
llvm-readobj adds `-h` as an alias for `--file-headers`.
llvm-dwarfdump adds `-h` for help and handles it after 
`ParseCommandLineOptions` returns.
llvm-mt rolls its own via llvm-tblgen.

So, the only ones I found that aren't essentially aliases for help are in 
llvm-objdump and llvm-readobj.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D59746#1440756 , @hintonda wrote:

> A better alternative would have been to add a cl::aliasopt for '-h' in llvm's 
> CommandLineParser when '-help' was first added.  However, that's no longer 
> possible since some llvm based tools already use '-h' for other purposes.


Is that intentional? Can you point at samples?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1456115 , @alexfh wrote:

> Can you give a specific example of how this problem manifests?


Any tool using `tooling::CommonOptionsParser` with the `llvm::cl::OneOrMore` 
flag will have this problem, i.e., the `-h` flag will be seen, but no action 
taken because it was never wired up.  Please note that we changed `clang-tidy` 
to use `cl::ZeroOrMore` a few years ago, so it'll spit out help with or without 
any arguments, including `-h`.  However, if you pass a bad argument with `-h` 
you'll get an error, but no help.

Here are a few you can verify (first try with `-h`, then `-help`):

  $ bin/clang-tidy -h -x
  LLVM ERROR: CommonOptionsParser: failed to parse command-line arguments. 
[CommonOptionsParser]: clang-tidy: Unknown command line argument '-x'.  Try: 
'bin/clang-tidy -help'
  clang-tidy: Did you mean '-h'?
  
  $ bin/clang-tidy -help -x
  USAGE: clang-tidy [options]  [... ]
  
  
  
  $ bin/clang-refactor -h
  error: no refactoring action given
  note: the following actions are supported:
local-rename
extract
  
  $ bin/clang-refactor -help
  OVERVIEW: Clang-based refactoring tool for C, C++ and Objective-C
  USAGE: clang-refactor [options]  [... ]
  
  
  
  $ bin/clang-apply-replacements -h
  clang-apply-replacements: Unknown command line argument '-h'.  Try: 
'bin/clang-apply-replacements -help'
  clang-apply-replacements: Did you mean '-help'?
  clang-apply-replacements: Not enough positional command line arguments 
specified!
  Must specify at least 1 positional argument: See: 
bin/clang-apply-replacements -help
  
  $ bin/clang-apply-replacements -help
  USAGE: clang-apply-replacements [options] 
  
  
  
  $ bin/clang-change-namespace -h
  clang-change-namespace: for the -old_namespace option: must be specified at 
least once!
  clang-change-namespace: for the -new_namespace option: must be specified at 
least once!
  clang-change-namespace: for the -file_pattern option: must be specified at 
least once!
  LLVM ERROR: CommonOptionsParser: failed to parse command-line arguments. 
[CommonOptionsParser]: clang-change-namespace: Not enough positional command 
line arguments specified!
  Must specify at least 1 positional argument: See: bin/clang-change-namespace 
-help
  
  $ bin/clang-change-namespace -help
  USAGE: clang-change-namespace [options]  [... ]
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Can you give a specific example of how this problem manifests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a subscriber: arphaman.
hintonda added a comment.

@arphaman, since you added the `-h` option in D37618 
, could you let me know if this change is okay 
with you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

A better alternative would have been to add a cl::aliasopt for '-h' in llvm's 
CommandLineParser when '-help' was first added.  However, that's no longer 
possible since some llvm based tools already use '-h' for other purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added a reviewer: alexfh.
Herald added a project: clang.

LibTooling's '-h' option was intended to be as an alias for
'-help', but since the '-help' option is declared as a function scoped
static, it isn't visible, leaving it to individual clients to handle
'-h' -- which none seem to do.

This fix scans argv and expands '-h' to '-help' so that llvm's
CommandLineParser can handle it appropriately.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp


Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ clang/lib/Tooling/CommonOptionsParser.cpp
@@ -114,8 +114,24 @@
   if (!ErrorMessage.empty())
 ErrorMessage.append("\n");
   llvm::raw_string_ostream OS(ErrorMessage);
+
+  // Expand "-h" to "-help" so it's static handler can be used.
+  SmallVector NewArgv;
+  // Append options from command line.
+  bool SawDoubleDash = false;
+  for (int I = 0; I < argc; ++I) {
+StringRef Arg(argv[I]);
+if (Arg == "--")
+  SawDoubleDash = true;
+if(!SawDoubleDash && Arg == "-h")
+  NewArgv.push_back("-help");
+else
+  NewArgv.push_back(argv[I]);
+  }
+  int NewArgc = static_cast(NewArgv.size());
+
   // Stop initializing if command-line option parsing failed.
-  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+  if (!cl::ParseCommandLineOptions(NewArgc, &NewArgv[0], Overview, &OS)) {
 OS.flush();
 return llvm::make_error("[CommonOptionsParser]: " +
ErrorMessage,


Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ clang/lib/Tooling/CommonOptionsParser.cpp
@@ -114,8 +114,24 @@
   if (!ErrorMessage.empty())
 ErrorMessage.append("\n");
   llvm::raw_string_ostream OS(ErrorMessage);
+
+  // Expand "-h" to "-help" so it's static handler can be used.
+  SmallVector NewArgv;
+  // Append options from command line.
+  bool SawDoubleDash = false;
+  for (int I = 0; I < argc; ++I) {
+StringRef Arg(argv[I]);
+if (Arg == "--")
+  SawDoubleDash = true;
+if(!SawDoubleDash && Arg == "-h")
+  NewArgv.push_back("-help");
+else
+  NewArgv.push_back(argv[I]);
+  }
+  int NewArgc = static_cast(NewArgv.size());
+
   // Stop initializing if command-line option parsing failed.
-  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+  if (!cl::ParseCommandLineOptions(NewArgc, &NewArgv[0], Overview, &OS)) {
 OS.flush();
 return llvm::make_error("[CommonOptionsParser]: " +
ErrorMessage,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits