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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:46
+                 DeleteExpr, DeleteMemberExpr,
+                 compoundStmt(anyOf(has(DeleteExpr), has(DeleteMemberExpr)),
+                              statementCountIs(1))
----------------
nit: Will `has(anyOf(DeleteExpr, DeleteMemberExpr))` work?


================
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:69
+  };
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+
----------------
nit: I'd put this line closer to the line with the warning (or right before it), e.g.

        // CHECK-MESSAGES: :[[@LINE+1]]:...
        if (mp)
          delete mp;



================
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:71
+
+  // CHECK-FIXES: delete mp;
 }
----------------
This doesn't check that the `if` is deleted. It will pass if the check doesn't do any replacements. One way to properly check this is to add a unique comment after the `if` and match it with an anchor to the start of line:

  if (mp) // should be deleted 1
    delete mp;
  // CHECK-FIXES: {{^   }}// should be deleted 1
  // CHECK-FIXES-NEXT: delete mp;

or alternatively:

  // next line should be deleted
  if (mp)
    delete mp;
  // CHECK-FIXES: // next line should be deleted
  // CHECK-FIXES-NEXT:  {{^  }}delete mp;



https://reviews.llvm.org/D29726