[PATCH] D29726: [Clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work for class members

[Mads Ravn via Phabricator via cfe-commits]

madsravn updated this revision to Diff 88123.
madsravn marked an inline comment as done.
madsravn added a comment.

Added comment in CHECK-FIX to ensure the line we are referring to is uniquely identified.


https://reviews.llvm.org/D29726

Files:
  clang-tidy/readability/DeleteNullPointerCheck.cpp
  clang-tidy/readability/DeleteNullPointerCheck.h
  test/clang-tidy/readability-delete-null-pointer.cpp


Index: test/clang-tidy/readability-delete-null-pointer.cpp
===================================================================
--- test/clang-tidy/readability-delete-null-pointer.cpp
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -59,6 +59,16 @@
   } 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() {
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===================================================================
--- clang-tidy/readability/DeleteNullPointerCheck.h
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -16,7 +16,8 @@
 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
Index: clang-tidy/readability/DeleteNullPointerCheck.cpp
===================================================================
--- clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -24,19 +24,28 @@
                         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 =
       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"))))
+             hasThen(anyOf(
+                 DeleteExpr, DeleteMemberExpr,
+                 compoundStmt(has(anyOf(DeleteExpr, DeleteMemberExpr)),
+                              statementCountIs(1))
+                     .bind("compound"))))
           .bind("ifWithDelete"),
       this);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29726.88123.patch
Type: text/x-patch
Size: 3027 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170212/13d3b2cf/attachment.bin>