alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
I've noticed a few more minor issues. Otherwise looks good.
Thank you for the new check!
================
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:27-38
+ const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean));
+ const auto BinaryPointerCheckCondition =
+ binaryOperator(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))),
+ hasEitherOperand(ignoringImpCasts(declRefExpr())));
+
+ Finder->addMatcher(
+ ifStmt(
----------------
This can be simplified. Something like this (modulo formatting):
const auto PointerExpr =
ignoringImpCasts(declRefExpr(to(decl().bind("deletedPointer"))));
const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean),
hasSourceExpression(PointerExpr));
const auto BinaryPointerCheckCondition =
binaryOperator(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))),
hasEitherOperand(PointerExpr));
(and remove the second `hasCondition`).
================
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:53
+ "'if' statement is unnecessary; deleting null pointer has no effect");
+ if (IfWithDelete->getElse())
+ return;
----------------
The check can suggest a fix in this case as well, but it's a bit more involved.
Please add a FIXME.
================
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.
+
----------------
alexfh wrote:
> Either `null pointer` or `nullptr` (enclosed in double backquotes).
Sorry for not being clear enough: "null pointer" is not an inline code snippet,
it shouldn't be enclosed in double backquotes or anything else. The "(enclosed
in double backquotes)" part was meant to apply to `nullptr` only (since it's a
keyword and should be highlighted as a code snippet).
================
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:8
+
+ // comment that should not be deleted #1
+ if (p) { // comment that should not be deleted #2
----------------
The `that should not be deleted` part is superfluous, IMO. You could even leave
just `// #1`, `// #2`, etc.
https://reviews.llvm.org/D21298
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
