[PATCH] D71963: clang-tidy doc: Add the severities description

Tue Dec 31 04:04:25 PST 2019

whisperity added a comment.

@vingeldal Apologies, in that case. However, I would still claim that `style`  (as a //potential// severity) has its purpose for Tidy checkers, not just for  `clang-format`.

In D71963#1798871 <https://reviews.llvm.org/D71963#1798871>, @vingeldal wrote:

> If severity levels must be exactly like they are currently defined in CodeChecker then IMO that is a rather strong reason not to introduce them in clang-tidy and just keep that stuff in CodeChecker.

This is exactly the reason why I urged @sylvestre.ledru in D36051 <https://reviews.llvm.org/D36051> to mention that the levels were taken from CodeChecker's classification. Which, by all means, could be viewed as "just another" classification. @dkrupp will hopefully explain the decision making behind the classification when he is back from holidays - give it a week more or so.

I believe the severity categories and assigned severity levels in CodeChecker are also malleable. Discussion brings us forward if there is a chance of coming up with better ideas, it's just a little bit of JS and database wrangling to put in a new severity into CodeChecker's system.

----

CodeChecker actually uses a two-dimensional classification: I think severities indicate some sort of a relative "power" of the report itself - the higher the severity, the more likely the reported issue will actually cause trouble. I don't know the nitty-gritty for where the demarcation line is drawn between `low` and `medium`. My previous comment, as follows:

In D71963#1798829 <https://reviews.llvm.org/D71963#1798829>, @whisperity wrote:

> A `low` diagnostics (and everything "above", assuming a(n at least) partial ordering on severities) should mean the coding construct is problematic: there is a chance -- perhaps once in a lifetime, perhaps not -- that doing this will cause a real error. A `style` thing should mean [snip]. No **real** "game-breaking" issue should ever arise from deciding on fixing or ignoring a `style` check's output.

was to indicate that any border between `style` and, let's say, //whatever-else// on the other hand, should be very clear immediately - `style` issues are not something to react with a //"STOP THE WORLD!"// approach - in case of a CI hook, if a developer receives a `style` report on their to-be-added patch, sure please fix it before the commit, but in case a badly formatted code had already made it into the upstream repository, it's not urgent to have it fixed. (This is the same we do in LLVM, too, except that we don't really have patch-CIs yet, sadly.) This is what I meant with `style` being applicable for //some// Tidy checkers, not just format.

Another classification, orthogonal to severity used in CodeChecker is dubbed "checker profiles" <https://github.com/Ericsson/codechecker/blob/cc93331a92261180cfc9a4ed29cf4d6e9b74ad0d/config/config.json#L11-L17> which assign checkers into, once again, arbitrarily named categories. When you run CodeChecker, you can do `--enable default --enable extreme --enable alpha` which will turn on every checker from the two categories, and the `alpha.` Clang-SA checker "group" or "namespace". These categories (or "profiles"), according to the comments in the file, were decided based on the "strength" of the **checker** (as opposed to the "strength" of the **report**), namely that checkers with higher false-positive rate (which directly translates to an inverse desire to even look at the reports) are grouped into "more extreme" categories. This is simply to give users a good enough "starting point" for their analysis, everyone can run the analyzers however they see fit.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71963