[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.
Nathan James via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Mar 7 00:33:41 PST 2020
njames93 added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:74
+void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+ const auto IntegerType = qualType(allOf(isInteger(), unless(isAnyCharacter()),
+ unless(booleanType())))
----------------
All node matchers are implicitly `allOf` matchers so you can safely drop the `allOf` matcher.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97
+ const auto CompareOperator =
+ expr(binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ anyOf(allOf(hasLHS(SignedCharCastExpr),
----------------
`binaryOperator(hasAnyOperatorName("==", "!=") ...`
Also whats the reason for not checking `<`, `>`, `<=`, `>=` or `<=>`?
================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:103
+ .bind("comparison");
+ ;
+
----------------
Any reason for this trailing `;`?
================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:125
- diag(CastExpression->getBeginLoc(),
- "'signed char' to %0 conversion; "
- "consider casting to 'unsigned char' first.")
- << *IntegerType;
+ if (Comparison) {
+ const auto *UnSignedCastExpression =
----------------
This should probably be a condition variable and bring the init from the start of `check` inside.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:141-143
+ } else {
+ const auto *IntegerType = Result.Nodes.getNodeAs<QualType>("integerType");
+ assert(IntegerType);
----------------
```} else if (const auto *IntegerType = Result.Nodes.getNodeAs<QualType>("integerType")) {
...
} else
llvm_unreachable("Unexpected match");
```
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp:210
+}
\ No newline at end of file
----------------
New line
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75749/new/
https://reviews.llvm.org/D75749
More information about the cfe-commits
mailing list