This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by madsravn on Feb 8 2017, 12:11 PM.

Diff Detail

Event Timeline

madsravn created this revision.Feb 8 2017, 12:11 PM
alexfh requested changes to this revision.Feb 8 2017, 3:39 PM
alexfh added inline comments.
clang-tidy/readability/DeleteNullPointerCheck.cpp
46

nit: Will has(anyOf(DeleteExpr, DeleteMemberExpr)) work?

test/clang-tidy/readability-delete-null-pointer.cpp
69

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;
71

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;
This revision now requires changes to proceed.Feb 8 2017, 3:39 PM
madsravn updated this revision to Diff 87829.Feb 9 2017, 10:02 AM
madsravn edited edge metadata.

Small change to check.
Minor changes to the lit test.

madsravn marked 3 inline comments as done.Feb 9 2017, 10:03 AM
madsravn added inline comments.
clang-tidy/readability/DeleteNullPointerCheck.cpp
46

I could swear that I had tried that. It seems like the obvious way to do it. It worked, though.

alexfh requested changes to this revision.Feb 9 2017, 5:11 PM
alexfh added inline comments.
test/clang-tidy/readability-delete-null-pointer.cpp
67

This should match the comment to verify the if is deleted.

This revision now requires changes to proceed.Feb 9 2017, 5:11 PM
madsravn updated this revision to Diff 88123.Feb 12 2017, 6:54 AM
madsravn edited edge metadata.
madsravn marked an inline comment as done.

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

madsravn marked an inline comment as done.Feb 12 2017, 6:54 AM
This revision is now accepted and ready to land.Feb 12 2017, 10:38 AM
This revision was automatically updated to reflect the committed changes.