From d20961c6575c00fc998d901ded78c1a41edd61b1 Mon Sep 17 00:00:00 2001
From: Stephen Kelly <steveire@gmail.com>
Date: Tue, 29 Dec 2020 13:44:18 +0000
Subject: [PATCH] [clang-tidy] Simplify delete null ptr check

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

Differential Revision: https://reviews.llvm.org/D96138
---
 .../readability/DeleteNullPointerCheck.cpp         | 34 ++++++++++------------
 .../readability/DeleteNullPointerCheck.h           |  3 ++
 .../checkers/readability-delete-null-pointer.cpp   | 30 ++++++++++++++++++-
 3 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
index 16e86c2..9f33504 100644
--- a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -20,34 +20,30 @@ 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)),
+                              statementCountIs(1))
+                     .bind("compound"))))
+          .bind("ifWithDelete"),
       this);
 }
 
diff --git a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h
index 0e310f7..c0d07d5 100644
--- a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h
@@ -26,6 +26,9 @@ public:
       : 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
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp
index 1c044e9..36e8f05 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp
@@ -1,4 +1,4 @@
-// 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
 
@@ -37,6 +37,34 @@ void instantiate() {
   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
-- 
2.7.4