SilverGeri created this revision.
SilverGeri added a reviewer: xazax.hun.
SilverGeri added a subscriber: cfe-commits.

removes the unnecessary if statements, if it was used to check a pointer's 
validity just to call delete on that pointer

http://reviews.llvm.org/D21298

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/DeleteNullCheck.cpp
  clang-tidy/misc/DeleteNullCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-delete-null.rst
  test/clang-tidy/misc-delete-null.cpp

Index: test/clang-tidy/misc-delete-null.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-delete-null.cpp
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s misc-delete-null %t
+
+void f() {
+  int *p = 0;
+  if (p) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete_null]
+    delete p;
+  }
+  // CHECK-FIXES: delete p;
+
+  int *p3 = new int[3];
+  if (p3)
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete_null]
+    delete[] p3;
+  // CHECK-FIXES: delete[] p3;
+}
+
+void g() {
+  int *p, *p2;
+  if (p)
+    delete p2;
+
+  if (p && p2)
+    delete p;
+
+  if (p2) {
+    int x = 5;
+    delete p2;
+  }
+}
Index: docs/clang-tidy/checks/misc-delete-null.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-delete-null.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - misc-delete-null
+
+misc-delete-null
+================
+
+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.
+
+.. code:: c++
+  int *p;
+  if (p)
+    delete p;
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -53,6 +53,7 @@
    misc-bool-pointer-implicit-conversion
    misc-dangling-handle
    misc-definitions-in-headers
+   misc-delete-null
    misc-fold-init-type
    misc-forward-declaration-namespace
    misc-inaccurate-erase
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -16,6 +16,7 @@
 #include "BoolPointerImplicitConversionCheck.h"
 #include "DanglingHandleCheck.h"
 #include "DefinitionsInHeadersCheck.h"
+#include "DeleteNullCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
 #include "InaccurateEraseCheck.h"
@@ -69,6 +70,8 @@
         "misc-dangling-handle");
     CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
         "misc-definitions-in-headers");
+    CheckFactories.registerCheck<DeleteNullCheck>(
+        "misc-delete-null");
     CheckFactories.registerCheck<FoldInitTypeCheck>(
         "misc-fold-init-type");
     CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
Index: clang-tidy/misc/DeleteNullCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/DeleteNullCheck.h
@@ -0,0 +1,36 @@
+//===--- DeleteNullCheck.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_MISC_DELETE_NULL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds if statements which check a pointer's validity
+/// and wants to free the pointer's data in the body
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-delete-null.html
+class DeleteNullCheck : public ClangTidyCheck {
+public:
+  DeleteNullCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_H
Index: clang-tidy/misc/DeleteNullCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/DeleteNullCheck.cpp
@@ -0,0 +1,65 @@
+//===--- DeleteNullCheck.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 "DeleteNullCheck.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 misc {
+
+void DeleteNullCheck::registerMatchers(MatchFinder *Finder) {
+  const auto HasDeleteExpr =
+      cxxDeleteExpr(hasDescendant(declRefExpr().bind("pointerToDelete")))
+          .bind("deleteExpr");
+
+  Finder->addMatcher(
+      ifStmt(
+          allOf(hasCondition(implicitCastExpr(
+                                 hasDescendant(declRefExpr().bind("condition")))
+                                 .bind("castExpr")),
+                hasThen(
+                    anyOf(HasDeleteExpr,
+                          compoundStmt(has(HasDeleteExpr)).bind("compound")))))
+          .bind("ifWithDelete"),
+      this);
+}
+
+void DeleteNullCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *IfWithDelete = Result.Nodes.getNodeAs<IfStmt>("ifWithDelete");
+  const auto *CastExpr = Result.Nodes.getNodeAs<ImplicitCastExpr>("castExpr");
+  const auto *PointerToDelete =
+      Result.Nodes.getNodeAs<DeclRefExpr>("pointerToDelete");
+  const auto *Cond = Result.Nodes.getNodeAs<DeclRefExpr>("condition");
+  const auto *Compound = Result.Nodes.getNodeAs<CompoundStmt>("compound");
+  const auto *Delete = Result.Nodes.getNodeAs<CXXDeleteExpr>("deleteExpr");
+
+  if ((CastExpr->getCastKind() == CastKind::CK_PointerToBoolean) &&
+      (PointerToDelete->getDecl() == Cond->getDecl()) &&
+      (!Compound || Compound->size() == 1)) {
+    auto D = diag(
+        IfWithDelete->getLocStart(),
+        "if statement is unnecessary (deleting null pointer has no effect)");
+    std::string ReplacementText = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(Delete->getSourceRange()),
+        *Result.SourceManager, Result.Context->getLangOpts());
+    ReplacementText += ';';
+
+    D << FixItHint::CreateReplacement(IfWithDelete->getSourceRange(),
+                                      ReplacementText);
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -7,6 +7,7 @@
   BoolPointerImplicitConversionCheck.cpp
   DanglingHandleCheck.cpp
   DefinitionsInHeadersCheck.cpp
+  DeleteNullCheck.cpp
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   InaccurateEraseCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to