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

Don Hinton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 17 09:41:26 PDT 2019


hintonda added a comment.

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

> @hintonda / @beanz : I've been thinking about this, and I'm now convinced that this review is probably not the good approach. What we need is a good way to track which options are actually important for a given binary, correctly set the categories of each option and *reallyhide* the others. So I'll pause this review and start working on sanitizing the output of several tools until I get a better understanding of the actual API needs.


I created an example application yesterday to test this stuff out, and was just about to add a similar comment.

The problem with this API -- not your changes, but the API in general -- is that the HiddenFlag attribute is per `cl::Option`, not per `cl::SubCommand`.  So, if you hide an option, it's hidden everywhere.  However, finding the options you want to hide can be tricky.  Only the special `cl::SubCommand`, `cl::TopLevelSubCommand`, which is the default, can see all registered options, so passing it is the only way to truly hide options not associated with the `cl::Category`s of interest.  Any other combination of calls to `cl::HideUnrelatedOptions` with various `cl::SubCommand`s is unnecessary and error-prone.

Therefore, I recommend removing the `SubCommand` parameter from this API and always using `cl::TopLevelSubCommand`.


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

https://reviews.llvm.org/D61870





More information about the llvm-commits mailing list