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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 31 10:25:40 PST 2019


aaron.ballman added a comment.

In D71963#1800056 <https://reviews.llvm.org/D71963#1800056>, @sylvestre.ledru wrote:

> > I may have missed this in prior discussions, and if so, I'm sorry -- but why are we taking CodeChecker as the model for this?
>
> I went ahead and use it because:
>
> - it is there and maintained (I contributed to the list a few time)


There are other models that exist and are maintained.

> - it is pretty good from my experience (it is rare that I see the list of checker/severity and disagree with the evaluation)

Other models are also pretty good.

> - it is a good start to trigger some discussions

Definitely agreed! However, I think an RFC would have been a less invasive approach to starting those discussions.

> - codechecker upstream is also involved in clang-tidy

As are other tools and coding standards.

For me personally, I like the idea of giving users some idea of the severity for any given check. I think it provides valuable information to users and is a good thing to document, but only when applied consistently across checks. If we can't find a consistent heuristic to use across all the coding standards and one-off checks we support, the utility of telling users about the severity is pretty hampered (or worse yet, gives a false sense of how bad a problem may be). I'd rather see the severity stuff reverted out of trunk until we've got some broader community agreement that 1) we want this feature, 2) we enumerate concrete steps for how to pick a severity for any given check, and 3) what to do when our severity differs from an external severity in the case of checkers for coding standards with that concept.

FWIW, I didn't notice this change was even proposed because it happened in a review about moving from a long list of checkers to one using tables and from what I can tell, it looks like the patch landed without that review being accepted (in fact, it seems to have landed with a code owner having marked it as requiring changes).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71963





More information about the cfe-commits mailing list