Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -4,6 +4,7 @@ AvoidConstParamsInDecls.cpp BracesAroundStatementsCheck.cpp ContainerSizeEmptyCheck.cpp + DeleteNullPointerCheck.cpp DeletedDefaultCheck.cpp ElseAfterReturnCheck.cpp FunctionSizeCheck.cpp Index: clang-tidy/readability/DeleteNullPointerCheck.h =================================================================== --- /dev/null +++ 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 Index: clang-tidy/readability/DeleteNullPointerCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/DeleteNullPointerCheck.cpp @@ -0,0 +1,78 @@ +//===--- 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 PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean)); + const auto BinaryPointerCheckCondition = binaryOperator( + allOf(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))), + hasEitherOperand(ignoringImpCasts(declRefExpr())))); + + Finder->addMatcher( + ifStmt( + hasCondition(anyOf(PointerCondition, BinaryPointerCheckCondition)), + hasCondition(anyOf( + ignoringImpCasts(declRefExpr(to(decl().bind("deletedPointer")))), + binaryOperator(hasEitherOperand(ignoringImpCasts( + declRefExpr(to(decl().bind("deletedPointer")))))))), + hasThen(anyOf(DeleteExpr, compoundStmt(allOf(has(DeleteExpr), + statementCountIs(1))) + .bind("compound")))) + .bind("ifWithDelete"), + this); +} + +void DeleteNullPointerCheck::check(const MatchFinder::MatchResult &Result) { + const auto *IfWithDelete = Result.Nodes.getNodeAs("ifWithDelete"); + const auto *Compound = Result.Nodes.getNodeAs("compound"); + + auto Diag = diag( + IfWithDelete->getLocStart(), + "'if' statement is unnecessary; deleting null pointer has no effect"); + if (IfWithDelete->getElse()) { + return; + } + + 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 Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ 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" @@ -45,6 +46,8 @@ "readability-braces-around-statements"); CheckFactories.registerCheck( "readability-container-size-empty"); + CheckFactories.registerCheck( + "readability-delete-null-pointer"); CheckFactories.registerCheck( "readability-deleted-default"); CheckFactories.registerCheck( Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -127,6 +127,7 @@ 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 Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-delete-null-pointer.rst @@ -0,0 +1,12 @@ +.. 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 nullpointer has no effect. + +.. code:: c++ + int *p; + if (p) + delete p; Index: test/clang-tidy/readability-delete-null-pointer.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-delete-null-pointer.cpp @@ -0,0 +1,73 @@ +// RUN: %check_clang_tidy %s readability-delete-null-pointer %t + +#define NULL 0 + +void f() { + int *p = 0; + // CHECK-FIXES: // comment that should not be deleted + if (p) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer] + // comment that should not be deleted + delete p; + } + // CHECK-FIXES-NOT: if (p) { + // CHECK-FIXES: delete p; + + + int *p2 = new int[3]; + // CHECK-FIXES: // another comment to keep + if (p2) + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; + // another comment to keep + delete[] p2; + // CHECK-FIXES-NOT: if (p2) + // CHECK-FIXES: 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; + } +}