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

2019-04-10 Thread 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);

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

2019-04-10 Thread 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

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

2019-04-10 Thread 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

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

2019-04-09 Thread 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

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

2019-04-09 Thread 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

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

2019-04-09 Thread 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

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

2019-04-09 Thread 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

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

2019-04-09 Thread 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

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

2019-04-08 Thread 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

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

2019-04-08 Thread 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

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

2019-04-08 Thread 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

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

2019-04-08 Thread 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

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

2019-04-08 Thread 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

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

2019-04-05 Thread 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

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

2019-04-05 Thread 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

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

2019-04-01 Thread 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

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

2019-03-24 Thread 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

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

2019-03-24 Thread 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