[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 06:37:32 PST 2019


aaron.ballman added a comment.

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'm not opposed to having some notion of severity for our checks (basically every tool and many coding standards have the same concept), but I am not certain why we would say CodeChecker's way is the way we want clang-tidy to progress. Also, does this tie in with the clang static analyzer checks, or is the severity stuff only for clang-tidy?



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:414
+
+- **STYLE**: A true positive indicates that the source code is against a specific coding guideline or could improve readability.
+  Example: LLVM Coding Guideline: Do not use else or else if after something that interrupts control flow (break, return, throw, continue).
----------------
vingeldal wrote:
> It seems like there is a bit of overlap between STYLE and LOW. They both mention readability.
> Might I suggest that style could be only issues related to naming convention and text formatting, like indentation, line breaks -like things that could be clang format rules.
I'm not keen on "is against a specific coding guideline" because things like the CERT secure coding standard and HIC++ are both "coding guidelines" yet failures against them are almost invariably not stylistic.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:417
+
+- **LOW**: A true positive indicates that the source code is hard to read/understand or could be easily optimized.
+  Example: Unused variables, Dead code.
----------------
"Hard to read/understand" code can also be based on style choices -- so how to differentiate between low and style checks?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:423
+
+- **HIGH**: A true positive indicates that the source code will cause a run-time error.
+  Example of this category: out of bounds array access, division by zero, memory leak.
----------------
vingeldal wrote:
> Does something need to always result in division by zero to be of high severity or is it enough that it introduces the possibility for division by zero to occur?
"Run-time error" is a strange bar because any logical mistake in the code is a run-time error. However, we also don't want to limit high severity cases to only things that cause crashes because some kinds of crashes are not bad (like assertion failures) while other kinds of non-crashes are very bad (like dangerous uses of format specifiers that can lead to arbitrary code execution).


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