[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

Sun May 12 16:44:10 PDT 2019

Szelethus added a subscriber: o.gyorgy.
Szelethus added a comment.

In D57858#1498640 <https://reviews.llvm.org/D57858#1498640>, @NoQ wrote:

> So, like, the global picture is as follows. In our case the Driver (i.e., --analyze) is not much more user facing than frontend flags. It's still fairly unusable directly, as our Static Analyzer is generally not a command-line tool. The real user-facing stuff is the GUIs such as scan-build or CodeChecker. These GUIs decide themselves on what options they want to expose. For instance, you have a right to decide that CodeChecker shouldn't support the aggressive mode of the move-checker and don't expose it as an option in your GUI. In this sense it's not really useful to provide a centralized `-help` of all user-facing options.
>
> But it sounds as if you want to change this situation and provide a single source of truth on what are the user-facing options. Particular GUIs can still ignore them, but you don't want to hardcode flags in CodeChecker, but instead you want to rely on clang to provide the list of supported options for you and, as a side effect, for scan-build users (if you also add a scan-build help flag). I'm totally in favor of crystallizing such list of user-facing flags, and this patch sounds like a good step in that direction, assuming that non-user-facing options are hidden.

That describes my intention quite well :)

> I'm still in favor of hiding alpha checkers (as they are for development only, much like debug flags; i'd recommend hiding them in the CodeChecker UI as well)
> 
> Now, why do we care about frontend/driver flags when they're unusable by definition? That's because we have a mental trauma after seeing a few powerusers actively explore those flags, observe that they don't work, and then tell everybody that the Analyzer is broken. So there's a threshold, based on a tiny but painful bit of practical experience, that says that documentation of developer-only features on llvm.org or in code comments is fine, but documentation printed by the released binary itself is not fine.

What you say sounds very reasonable. Still, I am kind of hesitant about hiding all alpha checkers: I initially intended to hide checkers that are developer-only checkers (modeling, debug). I guess if we define alpha checkers (as you stated numerous times) as incomplete, under development checkers, that are missing half their limbs and crash if you look at them the wrong way, sure, they belong in the developer-only category. But checkers such as mine (UninitializedObjectChecker), for the longest time were very stable, have been enabled by default for our internal projects, despite only recently moving out of alpha.

Then agaaain, if we're that stubborn about alpha checkers, we could might as well dig them out of `-analyzer-checker-help`, and leave the rest there. Untangling what alpha checkers depend on one another could be solved by making yet another frontend flag that would display checker dependencies, which would be super easy since D54438 <https://reviews.llvm.org/D54438>. @dkrupp @o.gyorgy Do you have any feelings on this?

> and we should probably automatically hide options of checker that are hidden.

Checker options are a different kind of animal entirely. think we should definitely let the user fine-tune some modeling checkers, for instance, `unix.DynamicMemoryModeling:Optimistic`, despite us not really wanting to let  anyone (even developers really) mess around whether `unix.DynamicMemoryModeling` should be enabled. While that specific option is, to put it nicely, a little esoteric, making some decisions the analyzer makes less conservative, or limiting state splits to help performance may be desirable in the future.

Let's move the rest of the discussion directly related to hiding checker options to D61839 <https://reviews.llvm.org/D61839>!

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

https://reviews.llvm.org/D57858