[clang-tidy] Add delete null pointer check.

This check detects and fixes redundant null checks before deletes.

Patch by: Gergely Angeli!

Differential Revision: https://reviews.llvm.org/D21298

llvm-svn: 290784

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

@@ -4,6 +4,7 @@ add_clang_library(clangTidyReadabilityModule
   AvoidConstParamsInDecls.cpp
   BracesAroundStatementsCheck.cpp
   ContainerSizeEmptyCheck.cpp
+  DeleteNullPointerCheck.cpp
   DeletedDefaultCheck.cpp
   ElseAfterReturnCheck.cpp
   FunctionSizeCheck.cpp

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

@@ -0,0 +1,76 @@
+//===--- DeleteNullPointerCheck.cpp - clang-tidy---------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "DeleteNullPointerCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+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"))))))))
+          .bind("deleteExpr");
+
+  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));
+
+  Finder->addMatcher(
+      ifStmt(hasCondition(anyOf(PointerCondition, BinaryPointerCheckCondition)),
+             hasThen(anyOf(DeleteExpr,
+                           compoundStmt(has(DeleteExpr), statementCountIs(1))
+                               .bind("compound"))))
+          .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(
+      IfWithDelete->getLocStart(),
+      "'if' statement is unnecessary; deleting null pointer has no effect");
+  if (IfWithDelete->getElse())
+    return;
+  // FIXME: generate fixit for this case.
+
+  Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+      IfWithDelete->getLocStart(),
+      Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getLocEnd(), 0,
+                                 *Result.SourceManager,
+                                 Result.Context->getLangOpts())));
+  if (Compound) {
+    Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+        Compound->getLBracLoc(),
+        Lexer::getLocForEndOfToken(Compound->getLBracLoc(), 0,
+                                   *Result.SourceManager,
+                                   Result.Context->getLangOpts())));
+    Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+        Compound->getRBracLoc(),
+        Lexer::getLocForEndOfToken(Compound->getRBracLoc(), 0,
+                                   *Result.SourceManager,
+                                   Result.Context->getLangOpts())));
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

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

@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Check whether the 'if' statement is unnecessary before calling 'delete' on a pointer.
+///
+/// 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;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H

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

@@ -13,6 +13,7 @@
 #include "AvoidConstParamsInDecls.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
 #include "DeletedDefaultCheck.h"
 #include "ElseAfterReturnCheck.h"
 #include "FunctionSizeCheck.h"
@@ -46,6 +47,8 @@ public:
         "readability-braces-around-statements");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
+    CheckFactories.registerCheck<DeleteNullPointerCheck>(
+        "readability-delete-null-pointer");
     CheckFactories.registerCheck<DeletedDefaultCheck>(
         "readability-deleted-default");
     CheckFactories.registerCheck<ElseAfterReturnCheck>(

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -132,6 +132,7 @@ Clang-Tidy Checks
    readability-avoid-const-params-in-decls
    readability-braces-around-statements
    readability-container-size-empty
+   readability-delete-null-pointer
    readability-deleted-default
    readability-else-after-return
    readability-function-size

clang-tools-extra/docs/clang-tidy/checks/readability-delete-null-pointer.rst

@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===============================
+
+Checks the ``if`` statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a null pointer has no effect.
+
+.. code:: c++
+
+  int *p;
+  if (p)
+    delete p;

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

@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+  int *p = 0;
+
+  // #1
+  if (p) { // #2
+    delete p;
+  } // #3
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+
+  // CHECK-FIXES: {{^  }}// #1
+  // CHECK-FIXES-NEXT: {{^  }}// #2
+  // CHECK-FIXES-NEXT: delete p;
+  // CHECK-FIXES-NEXT: {{^  }}// #3
+
+  int *p2 = new int[3];
+  // #4
+  if (p2) // #5
+    delete[] p2;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary;
+
+  // CHECK-FIXES: // #4
+  // CHECK-FIXES-NEXT: {{^  }}// #5
+  // CHECK-FIXES-NEXT: delete[] p2;
+
+  int *p3 = 0;
+  if (NULL != p3) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+    delete p3;
+  }
+  // CHECK-FIXES-NOT: if (NULL != p3) {
+  // CHECK-FIXES: delete p3;
+
+  int *p4 = nullptr;
+  if (p4 != nullptr) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+    delete p4;
+  }
+  // CHECK-FIXES-NOT: if (p4 != nullptr) {
+  // CHECK-FIXES: delete p4;
+
+  char *c;
+  if (c != 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+    delete c;
+  }
+  // CHECK-FIXES-NOT: if (c != 0) {
+  // CHECK-FIXES: delete c;
+
+  char *c2;
+  if (c2) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+    // CHECK-FIXES: } else {
+    // CHECK-FIXES: c2 = c;
+    delete c2;
+  } else {
+    c2 = c;
+  }
+}
+
+void g() {
+  int *p5, *p6;
+  if (p5)
+    delete p6;
+
+  if (p5 && p6)
+    delete p5;
+
+  if (p6) {
+    int x = 5;
+    delete p6;
+  }
+}