From 6ff978fd54f09d2ad902457c39a2358e53669cf6 Mon Sep 17 00:00:00 2001 From: Mads Ravn Date: Sun, 12 Feb 2017 20:09:59 +0000 Subject: [PATCH] [clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work for class members I have made a small fix for readability-delete-null-pointer check so it also checks for class members. Example of case that it fixes ``` struct A { void foo() { if(mp) delete mp; } int *mp; }; ``` Reviewers: JDevlieghere, aaron.ballman, alexfh, malcolm.parsons Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D29726 llvm-svn: 294912 --- .../readability/DeleteNullPointerCheck.cpp | 19 ++++++++++++++----- .../readability/DeleteNullPointerCheck.h | 3 ++- .../readability-delete-null-pointer.cpp | 10 ++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp index fc2bc503ee29..81ef39132471 100644 --- a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp @@ -24,8 +24,15 @@ void DeleteNullPointerCheck::registerMatchers(MatchFinder *Finder) { to(decl(equalsBoundNode("deletedPointer")))))))) .bind("deleteExpr"); - const auto PointerExpr = - ignoringImpCasts(declRefExpr(to(decl().bind("deletedPointer")))); + const auto DeleteMemberExpr = + cxxDeleteExpr(has(castExpr(has(memberExpr(hasDeclaration( + fieldDecl(equalsBoundNode("deletedMemberPointer")))))))) + .bind("deleteMemberExpr"); + + const auto PointerExpr = ignoringImpCasts(anyOf( + declRefExpr(to(decl().bind("deletedPointer"))), + memberExpr(hasDeclaration(fieldDecl().bind("deletedMemberPointer"))))); + const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean), hasSourceExpression(PointerExpr)); const auto BinaryPointerCheckCondition = @@ -34,9 +41,11 @@ void DeleteNullPointerCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( ifStmt(hasCondition(anyOf(PointerCondition, BinaryPointerCheckCondition)), - hasThen(anyOf(DeleteExpr, - compoundStmt(has(DeleteExpr), statementCountIs(1)) - .bind("compound")))) + hasThen(anyOf( + DeleteExpr, DeleteMemberExpr, + compoundStmt(has(anyOf(DeleteExpr, 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 87ee46fe9c13..501f6f7d926a 100644 --- a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h +++ b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h @@ -16,7 +16,8 @@ namespace clang { namespace tidy { namespace readability { -/// Check whether the 'if' statement is unnecessary before calling 'delete' on a pointer. +/// 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 diff --git a/clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp b/clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp index 400714b44d9e..b46e52a754b7 100644 --- a/clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp +++ b/clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp @@ -59,6 +59,16 @@ void f() { } else { c2 = c; } + struct A { + void foo() { + if (mp) // #6 + delete mp; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer] + // CHECK-FIXES: {{^ }}// #6 + // CHECK-FIXES-NEXT: delete mp; + } + int *mp; + }; } void g() {