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

Don Hinton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 10 10:33:28 PDT 2019


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





More information about the cfe-commits mailing list