[clang-tidy] RFC: "Experimental" checkers

Dear List,

This is a generic interest pitch mail about adding the notion of “experimental” checks to Clang-Tidy.

(A very rudimentary patch about the experimental group is here: http://reviews.llvm.org/D76545.)

The reason behind this idea is from the discussion with Aaron on a check that I developed and proposing for the suite. Review of the check here: http://reviews.llvm.org/D69560.

Summary (of the experimental idea):

  • Certain rules (such as the one mentioned above) cannot (yet?) be implemented properly, due to various constraints
  • However, parts of the rule that could be implemented might turn out to be useful for the community
  • It’s not nice to “sell” a checker named “foo-bar” if it only does “bar” partially — but coming up with a different name is superfluous as we wish to convey the intent that we support “bar” fully later on.

The Clang Static Analyser people have a similar notion of “alpha.”, however, several concerns about modelling that approach for Tidy has been expressed, namely:

  • the lifespan of checkers in the alpha. group is too long for Tidy’s(?) taste
  • alpha checkers are allowed to have crashes, lots of false positives, senseless output, etc.
  • it’s generally a “you’re on your own” situation if you decide to run alpha checkers

It’s understandable that we wish to be something more stable, but this needs more input from a broader set of people.

I believe that requiring stability (i.e. no crashes in general, or performance issues) from upstramed experimental checkers is a good idea. The core idea of an experimental checker could be:

“Here’s a rule, it’s a nice rule, here’s an automated tool for that, let’s see what it does on live projects. This can help us develop further, or maybe even do rounds of revising/extending the rule/best practice itself!”

Since no-one picked this up, I’ll just comment although I’m not deeply involved. Maybe this will trigger someone else.

I personally like the idea in general. While there is a risk of stuff being experimental forever there is also more feedback and more contributions that way compared to those checkers being hidden somewhere privately.

Also I agree on the stability comments. It doesn’t make sense to add rules so bad that they just crash. The goal should be to collect real feedback, real bug reports, etc.

I’m also thinking of the const adding checker (Sorry I forgot the name), which had like 0,1% false positives and therefore cannot be declared fully ‘working’ yet (as far as I remember).

With that checker being easily available to more people maybe some simpler cases with false positives will emerge which are easier to analyze.

The other questions I leave to the experts :wink:

Regards,

Alex

Hi,

I like the idea of indicating the quality/readiness of checkers somehow. It is already the case that existing checkers have a non-uniform level of quality despite having similar names. What’s exacerbating the problem is that it is common to enable checkers using globs based on their names – but names don’t indicate quality, only themes.

The definition of “experimental” as proposed is still fuzzy for me though. I don’t see a difference between ClangSA’s:

  • the lifespan of checkers in the alpha. group is too long for Tidy’s(?) taste

  • alpha checkers are allowed to have crashes, lots of false positives, senseless output, etc.

  • it’s generally a “you’re on your own” situation if you decide to run alpha checkers

and the proposed

“Here’s a rule, it’s a nice rule, here’s an automated tool for that, let’s see what it does on live projects. This can help us develop further, or maybe even do rounds of revising/extending the rule/best practice itself!”

Either way, the checker might not be usable in production:

  • because the false positive/negative rate is not acceptable (either because the implementation is not good enough, or because heuristics are not great, or because the rule needs more thought),

  • because the definition of the rule might change significantly over time, so users can’t build a workflow around the rule.

I also don’t see a way to ensure that an experimental checker is graduated within a fixed time period – apart from removing the ones that didn’t make it. That might be OK, but might also feel like an arbitrary deadline to some contributors.

Dmitri