alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:29
+ const auto BinaryPointerCheckCondition = binaryOperator(
+ allOf(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))),
+ hasEitherOperand(ignoringImpCasts(declRefExpr()))));
----------------
Since `binaryOperator` (and most if not all other node matchers) is
`VariadicDynCastAllOfMatcher`, `allOf` is redundant here (similar to Piotr's
comment below).
Same for `compoundStmt` below.
================
Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:6
+
+Checks the 'if' statements where a pointer's existence is checked and then
deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
----------------
Enclose `if` in double backquotes instead of single quotes.
================
Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:7
+Checks the 'if' statements where a pointer's existence is checked and then
deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
----------------
Either `null pointer` or `nullptr` (enclosed in double backquotes).
================
Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:10
+.. code:: c++
+ int *p;
+ if (p)
----------------
Insert an empty line above and check that Sphinx can build this.
================
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+ int *p = 0;
+ // CHECK-FIXES: // comment that should not be deleted
+ if (p) {
----------------
This check (also combined with the ones below) is useless. It will work for the
unchanged file as well: it will skip the `if (p)` line and find the comment,
the next CHECK-FIXES will find the `delete p;` line and the CHECK-FIXES-NOT
will find no lines matching `if (p)` between them.
I'd use something like this:
// comment1
if (p) { // comment 2
delete p;
} // comment 3
// CHECK-FIXES: {{^ }}// comment1
// CHECK-FIXES-NEXT: {{^ }} // comment2
// CHECK-FIXES-NEXT: {{^ }} delete p;
// CHECK-FIXES-NEXT: {{^ }} // comment3
Same for patterns below.
https://reviews.llvm.org/D21298
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
