[clang-tidy] Simplify delete null ptr check

[steveire]

Summary

Because it no longer relies on finding implicit casts, this check now
works on templates which are not instantiated in the translation unit.

[njames93]

Seems that your test cases are failing on windows, I'm guessing this is a MSVC template compatibility issue.
Probably need to add -fno-delayed-template-parsing to the clang driver args (after -- --).

[steveire]

Update

[njames93]

LGTM

clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h

 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability-delete-null-pointer.html
 class DeleteNullPointerCheck : public ClangTidyCheck {
 public:
   DeleteNullPointerCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H

clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp

 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 void DeleteNullPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto DeleteExpr =
-      cxxDeleteExpr(has(castExpr(has(declRefExpr(
-                        to(decl(equalsBoundNode("deletedPointer"))))))))
+      cxxDeleteExpr(
+          has(declRefExpr(to(decl(equalsBoundNode("deletedPointer"))))))
           .bind("deleteExpr");
 
   const auto DeleteMemberExpr =
-      cxxDeleteExpr(has(castExpr(has(memberExpr(hasDeclaration(
-                        fieldDecl(equalsBoundNode("deletedMemberPointer"))))))))
+      cxxDeleteExpr(has(memberExpr(hasDeclaration(
+                        fieldDecl(equalsBoundNode("deletedMemberPointer"))))))
           .bind("deleteMemberExpr");
 
-  const auto PointerExpr = ignoringImpCasts(anyOf(
+  const auto PointerExpr = anyOf(
       declRefExpr(to(decl().bind("deletedPointer"))),
-      memberExpr(hasDeclaration(fieldDecl().bind("deletedMemberPointer")))));
+      memberExpr(hasDeclaration(fieldDecl().bind("deletedMemberPointer"))));
 
-  const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean),
-                                         hasSourceExpression(PointerExpr));
-  const auto BinaryPointerCheckCondition = binaryOperator(
-      hasOperands(castExpr(hasCastKind(CK_NullToPointer)), PointerExpr));
+  const auto BinaryPointerCheckCondition = binaryOperator(hasOperands(
+      anyOf(cxxNullPtrLiteralExpr(), integerLiteral(equals(0))), PointerExpr));
 
   Finder->addMatcher(
-      traverse(TK_AsIs,
-               ifStmt(hasCondition(
-                          anyOf(PointerCondition, BinaryPointerCheckCondition)),
-                      hasThen(anyOf(DeleteExpr, DeleteMemberExpr,
-                                    compoundStmt(anyOf(has(DeleteExpr),
-                                                       has(DeleteMemberExpr)),
-                                                 statementCountIs(1))
-                                        .bind("compound"))))
-                   .bind("ifWithDelete")),
+      ifStmt(hasCondition(anyOf(PointerExpr, BinaryPointerCheckCondition)),
+             hasThen(anyOf(
+                 DeleteExpr, DeleteMemberExpr,
+                 compoundStmt(anyOf(has(DeleteExpr), has(DeleteMemberExpr)),
+          .bind("ifWithDelete"),
       this);
 }
 
 void DeleteNullPointerCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *IfWithDelete = Result.Nodes.getNodeAs<IfStmt>("ifWithDelete");
   const auto *Compound = Result.Nodes.getNodeAs<CompoundStmt>("compound");
 
   auto Diag = diag(

clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp

-// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t -- -- -fno-delayed-template-parsing
 
 #define NULL 0
 
 template <typename T>
 struct Templ {
   void foo() {
     // t1
     if (mem) // t2

 
 void instantiate() {
   Templ<int *> ti2;
   ti2.foo();
   TemplPtr<int> ti3;
   ti3.foo();
 }
 
+template <typename T>
+struct NeverInstantiated {
+  void foo() {
+    // t1
+    if (mem) // t2
+      delete mem;
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 'if' statement is unnecessary;
+    // CHECK-FIXES: // t1
+    // CHECK-FIXES-NEXT: {{^    }}// t2
+    // CHECK-FIXES-NEXT: delete mem;
+  }
+  T mem;
+};
+
+template <typename T>
+struct NeverInstantiatedPtr {
+  void foo() {
+    // t3
+    if (mem) // t4
+      delete mem;
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 'if' statement is unnecessary;
+    // CHECK-FIXES: // t3
+    // CHECK-FIXES-NEXT: {{^    }}// t4
+    // CHECK-FIXES-NEXT: delete mem;
+  }
+  T *mem;
+};
+
 void f() {
   int *ps = 0;
   if (ps /**/) // #0
     delete ps;
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
 
   // CHECK-FIXES: int *ps = 0;
   // CHECK-FIXES-NEXT: {{^  }}// #0