[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