[PATCH] D61870: Make cl::HideUnrelatedOptions more flexible

Chris Bieneman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 15 13:08:49 PDT 2019


beanz added a comment.

In D61870#1503490 <https://reviews.llvm.org/D61870#1503490>, @serge-sans-paille wrote:

> @beanz 
>  As pointed out by @hintonda, current default argument is *never* explicitly used in the codebase.


Yea, but that's going to change as soon as you start calling this in more places. Have you looked at how this might apply to a tool that has sub-commands? My concern is that the ordering of default parameters in functions that have more than one really needs to be in the order of decreasing likelihood of whether or not it will be overridden. By my count we have 2 tools with subcommands that account for 16 SubCommand, and you have one example of overriding the enum. At the moment it seems to me like overriding the SubCommand may be the more common case.

An alternative may be to provide two APIs, one that doesn't take a SubCommand, and one that does. The one that doesn't take a SubCommand could just call the API that does providing the default argument.

> This may also mean that each option should have a category, but I'm unsure.

That probably isn't completely unreasonable. In fact there are some big benefits to that.


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

https://reviews.llvm.org/D61870





More information about the llvm-commits mailing list