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

ztamas added a comment. In D71174#1818883 , @sylvestre.ledru wrote: > In D71174#1774249 , @ztamas wrote: > > > I run the new check on LibreOffice codebase with the option > > CharTypdefsToIgnore = "sal_Int8". > >

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

sylvestre.ledru added a comment. In D71174#1774249 , @ztamas wrote: > I run the new check on LibreOffice codebase with the option > CharTypdefsToIgnore = "sal_Int8". > The check produced 32 findings. Interesting. It found 31 issues on the Firefox code

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

ztamas added a comment. In 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

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

lebedev.ri added a comment. 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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG350da402ef6b: [clang-tidy] new check: bugprone-signed-char-misuse (authored by ztamas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/

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

ztamas added a comment. Thanks for the review! I applied for the renewal of my commit access. I had commit access only for the SVN repo. After I get access to the GitHub repo I'll push this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174

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

ztamas updated this revision to Diff 235790. ztamas added a comment. Update warning message in test files and rebase patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

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

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.") + <<

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

ztamas marked 2 inline comments as done. ztamas 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

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

ztamas updated this revision to Diff 235765. ztamas marked an inline comment as done. ztamas added a comment. Update docs / warning message, according to reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/

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

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.") + <<

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

ztamas marked 2 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:66 bugprone-posix-return + bugprone-signed-char-misuse bugprone-sizeof-container sylvestre.ledru wrote: > list.rst has

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

ztamas updated this revision to Diff 235470. ztamas added a comment. Rebase patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

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

sylvestre.ledru added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:66 bugprone-posix-return + bugprone-signed-char-misuse bugprone-sizeof-container list.rst has changed, you should try to rebase your patch!

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

ztamas marked an inline comment as done. ztamas added inline comments. Herald added a subscriber: whisperity. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:43-44 + + const auto SignedCharType = expr(hasType(qualType( + allOf(isAnyCharacter(),

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

ztamas added a comment. I added the above comments two weeks ago, I just forget to push the submit button. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 ___

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

ztamas updated this revision to Diff 233643. ztamas marked 2 inline comments as done. ztamas added a comment. Add newlines to the end of the files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

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

ztamas updated this revision to Diff 233638. ztamas added a comment. Update code based on reviewer comments. - Remove spurious semicolon. - Add tests about plain char. - Extend documentation describing how to control char signdness. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:43-44 + + const auto SignedCharType = expr(hasType(qualType( + allOf(isAnyCharacter(), isSignedInteger(), unless(IntTypedef); + Does this

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

ztamas updated this revision to Diff 232851. ztamas added a comment. Remove anonymus namespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

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

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:21 + +namespace { +static Matcher hasAnyListedName(const std::string ) { Anonymous namespace is not needed anymore. Repository: rG LLVM Github

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

ztamas updated this revision to Diff 232814. ztamas added a comment. Fixes small things mentioned by reviewer commments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

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

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:22 +namespace { +Matcher hasAnyListedName(const std::string ) { + const std::vector NameList = Please use static for functions. See LLVM Coding

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

ztamas added a comment. I run the new check on LLVM codebase with the option CharTypdefsToIgnore = "int8_t". The check produced 12 findings. All findings seem valid use cases, where a char -> integer conversion happens. I had a closer look at some of the catches. Somewhere I see only type

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

ztamas added a comment. I run the new check on LibreOffice codebase with the option CharTypdefsToIgnore = "sal_Int8". The check produced 32 findings. I see 3 false positives which are similar to the DereferenceWithTypdef test case where the code uses sal_Int8 typedef, but the check still

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

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. This check searches for signed char -> integer conversions which might indicate programming error, because of the misinterpretation of char values. A signed char might store the