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

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

In D71174#1815830 <https://reviews.llvm.org/D71174#1815830>, @lebedev.ri wrote:

> A little bit late to the party, but still.
>  I'm honestly wondering if this should be a proper clang static analyzer data-flow aware check.
>  This is basically diagnosing every `signed char` -> `signed int` promotion, regardless of whether 
>  the `char` is used as an ASCII char or not. It really seems like a wrong, noise-filled approach..


I've considered creating a static analyzer check instead before I implemented this clang-tidy check
and it seemed to me path analysis would not help too much. At least in those cases, what I found
in LibreOffice codebase, there was no additional knowledge in the code about the character's value range.
(Actually, in one use case the method name contained "ASCII", but this information can't be used by the
static analyzer I guess.)
For example, when the code reads a file it's not known whether the file will contain only ASCII characters
or not. Or if you have a function working with characters (without having any explicit check in it about
character value range), then the static analyzer won't help there anyway.
I'm not sure how many test cases can be filtered out with a static analyzer check in practice, but I feel like
it's not much more effective than a clang-tidy check in this case. As I recall path analysis stops working if it
reaches a loop, right? It loses the information from before the loop or something like that, which makes a static
analyzer check even less useful, because characters are rare to go alone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174