hokein added inline comments.
================
Comment at: clang-tidy/misc/ComparisonMisuseCheck.cpp:31
@@ +30,3 @@
+ unless(anyOf(hasOperatorName("=="), hasOperatorName("!="))),
+ hasEitherOperand(ignoringImpCasts(gnuNullExpr())))
+ .bind("compareToNull"),
----------------
We should use `nullPointerConstant()` here to cover more null cases.
================
Comment at: clang-tidy/misc/ComparisonMisuseCheck.cpp:39
@@ +38,3 @@
+ Result.Nodes.getNodeAs<BinaryOperator>("charToLiteral");
+ if (CharToLiteral)
+ diag(CharToLiteral->getOperatorLoc(),
----------------
The code can be simplified as follows:
```
if (const auto * a = Result.Nodes.getNodeAs<BinaryOperator>("charToLiteral")) {
...
} else if (const auto *CompareToNull =
Result.Nodes.getNodeAs<BinaryOperator>("compareToNull")) {
...
}
```
================
Comment at: clang-tidy/misc/ComparisonMisuseCheck.cpp:41
@@ +40,3 @@
+ diag(CharToLiteral->getOperatorLoc(),
+ "char* is compared to a string literal");
+
----------------
I'm wondering if we can automatically fix this case. Use `strcmp()` function is
sufficient to fix it, IMO?
================
Comment at: clang-tidy/misc/ComparisonMisuseCheck.cpp:52
@@ +51,2 @@
+} // namespace clang
+
----------------
an extra blank line.
================
Comment at: clang-tidy/misc/ComparisonMisuseCheck.h:20
@@ +19,3 @@
+/// This checker reports errors related to the misuse of the comparison
operator.
+/// It should warn for the following cases:
+/// - strcmp,strncmp,memcmp misuse.
----------------
s/should warn/warns
================
Comment at: clang-tidy/misc/ComparisonMisuseCheck.h:21
@@ +20,3 @@
+/// It should warn for the following cases:
+/// - strcmp,strncmp,memcmp misuse.
+/// - char* is compared to a string literal
----------------
Eugene.Zelenko wrote:
> Spaces after commas,
Seems like your check doesn't warn any usage about the strcmp family functions.
================
Comment at: docs/clang-tidy/checks/misc-comparison-misuse.rst:3
@@ +2,3 @@
+
+misc-comparison-misuse
+======================
----------------
Please also update `docs/ReleaseNotes.rst`.
================
Comment at: docs/clang-tidy/checks/misc-comparison-misuse.rst:10
@@ +9,3 @@
+Case 1:
+ ``char*`` is compared to a string literal.
+
----------------
Does the case cover `char[]` ?
================
Comment at: test/clang-tidy/misc-comparison-misuse.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s misc-comparison-misuse %t
+
----------------
Also clang-format this example.
================
Comment at: test/clang-tidy/misc-comparison-misuse.cpp:13
@@ +12,3 @@
+void test_null_to_pointer(int *p){
+ if (NULL>=p);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: comparison to nullptr
[misc-comparison-misuse]
----------------
Add `if (p <= NULL);` test case.
Repository:
rL LLVM
https://reviews.llvm.org/D23427
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
