[PATCH] D58499: [CommandLine] Do not crash if an option has both ValueRequired and Grouping.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added a comment.

In D58499#1408521 <https://reviews.llvm.org/D58499#1408521>, @jhenderson wrote:

> LGTM.


Thanks!

> I just want to make sure that the behaviour of something like "-abc" where -a, -b and -c are all options, and -b is ValueOptional makes sense (I'm not sure whether it should be equivalent to "-a -b=c" or "-a -b -c" though).

If I understand it right, a `ValueOptional` option can accept its value only if it is explicitly used in the `-opt=value` form. Moreover, as boolean options are of a `ValueOptional` kind implicitly, your sample is something very basic for checking how `Grouping` works at all. "-abc" should definitely be equivalent to "-a -b -c". It is a bit strange that there are no tests for that. Maybe the test suite is worth to be extended.

> Although the code change is purely to do with ValueRequired, the documentation change is not specific to ValueRequired.

You are right. We either need better wording in the documentation or improve the code so that the "=val" form is accepted with a group. I would prefer changing the documentation because no one needed that new behavior yet.

> Perhaps you could do a follow-up patch with at least a test for this behaviour, if it just works?

Let's create a separate patch with additional tests for the existing behavior.


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

https://reviews.llvm.org/D58499