Page MenuHomePhabricator

fix inverted logic for HideUnrelatedOptions?
ClosedPublic

Authored by vtjnash on Nov 24 2021, 9:17 PM.

Details

Summary

It seems clearer to me that this would check for *any of* instead of
*all of* these option categories, as it looks to me like that was the
intent. But apparently this logic has always has been inverted, and
possibly never fully used?

Or am I just tired and reading this backwards?

Introduced in https://reviews.llvm.org/D61574

Diff Detail

Event Timeline

vtjnash created this revision.Nov 24 2021, 9:17 PM
vtjnash requested review of this revision.Nov 24 2021, 9:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 24 2021, 9:17 PM
vtjnash retitled this revision from fix inverted logic for HideUnrelatedOptions to fix inverted logic for HideUnrelatedOptions?.Nov 24 2021, 9:34 PM
vtjnash edited the summary of this revision. (Show Details)
vtjnash added reviewers: hintonda, MaskRay.

Do you have an example where "all of" makes more sense?

I couldn't think of an example where you'd want to make sure all of the assigned categories were related to the command being used, instead of showing the option if at least one was related to it. Currently it hides the option as soon as any don't match, whereas it seems like you'd want to hide the option only if all categories don't match.

MaskRay accepted this revision.Nov 29 2021, 3:39 PM
This revision is now accepted and ready to land.Nov 29 2021, 3:39 PM
hintonda accepted this revision.Nov 29 2021, 11:35 PM

LGTM, thanks for fixing this...

This revision was landed with ongoing or failed builds.Nov 30 2021, 11:59 AM
This revision was automatically updated to reflect the committed changes.