[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97-98
+  diag(CastExpression->getBeginLoc(),
+       "singed char -> integer (%0) conversion; "
+       "consider to cast to unsigned char first.")
+      << *IntegerType;
----------------
How about: `'signed char' to %0 conversion; consider casting to 'unsigned char' first`?

Also, should we have a fix-it to automatically insert the cast to `unsigned char` in the proper location for the user?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:25
+See also:
+`STR34-C. Cast characters to unsigned char before converting to larger integer sizes
+<https://wiki.sei.cmu.edu/confluence/display/c/STR34-C.+Cast+characters+to+unsigned+char+before+converting+to+larger+integer+sizes>`_
----------------
ztamas wrote:
> aaron.ballman wrote:
> > Should this check also be registered in the CERT module?
> This check does not cover the whole CERT description. I guess a cert check should catch all bugous code related to the CERT issue, right?
So long as this covers a decent amount of the CERT rule, I think it is fine to document the areas where we don't quite match the CERT rule.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:21
+by default and so it is caught by this check or not. To change the default behavior
+you can use -funsigned-char and -fsigned-char compilation options.
+
----------------
Backticks around the command line options.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:23
+
+By now, this check is limited to assignments and variable declarations,
+where a ``signed char`` is assigned to an integer variable. There are other
----------------
By now -> Currently


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:25
+where a ``signed char`` is assigned to an integer variable. There are other
+use cases where the same misinterpretation might lead to similar bugous
+behavior.
----------------
bugous -> bogus


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174