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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
