etienneb added a comment.
Some comments after a quick look to the code.
================
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:22
@@ +21,3 @@
+void DeleteNullCheck::registerMatchers(MatchFinder *Finder) {
+ const auto HasDeleteExpr =
+ cxxDeleteExpr(hasDescendant(declRefExpr().bind("pointerToDelete")))
----------------
HasDeleteExpr -> DeleteExpr
================
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:23
@@ +22,3 @@
+ const auto HasDeleteExpr =
+ cxxDeleteExpr(hasDescendant(declRefExpr().bind("pointerToDelete")))
+ .bind("deleteExpr");
----------------
The use of hasDescendant is Expensive.
There is cast to ignore? You could probably just skip cast/parens.
If the intend of this match-statement is to match comparison against NULL, it
should be expanded to be more precise.
================
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:47
@@ +46,3 @@
+
+ if ((CastExpr->getCastKind() == CastKind::CK_PointerToBoolean) &&
+ (PointerToDelete->getDecl() == Cond->getDecl()) &&
----------------
This check could be moved to the matcher part above/
see ASTMatcher: hasCastKind
================
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:48
@@ +47,3 @@
+ if ((CastExpr->getCastKind() == CastKind::CK_PointerToBoolean) &&
+ (PointerToDelete->getDecl() == Cond->getDecl()) &&
+ (!Compound || Compound->size() == 1)) {
----------------
see: equalsBoundNode
================
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:49
@@ +48,3 @@
+ (PointerToDelete->getDecl() == Cond->getDecl()) &&
+ (!Compound || Compound->size() == 1)) {
+ auto D = diag(
----------------
This check should be moved to the matcher part too.
see: statementCountIs
Repository:
rL LLVM
http://reviews.llvm.org/D21298
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
