This is an archive of the discontinued LLVM Phabricator instance.

Add a clang-tidy flag to support temporary destructor-aware analysis (workaround for bug 15599).
ClosedPublic

Authored by alexmc on Apr 29 2014, 7:42 PM.

Diff Detail

Event Timeline

alexmc updated this revision to Diff 8949.Apr 29 2014, 7:42 PM
alexmc retitled this revision from to Add a clang-tidy flag to support temporary destructor-aware analysis (workaround for bug 15599)..
alexmc added a reviewer: alexfh.
alexmc added subscribers: djasper, klimek, jordan_rose.

This is my first time sending a review with arc & phabricator: please let me know if I've set anything up incorrectly!

alexmc updated this revision to Diff 8950.Apr 29 2014, 7:46 PM
  • Fix typo.
klimek added a subscriber: Unknown Object (MLST).Apr 30 2014, 12:25 AM

+cfe-commits

Alex, you'll always want to cc' cfe-commits when sending patches to clang-related things.

alexfh accepted this revision.Apr 30 2014, 4:41 AM
alexfh edited edge metadata.

Looks good modulo comments. Thanks for working on this!

clang-tidy/ClangTidy.h
116

I'd not make any assumptions about the life-time of Options. Please just copy it here, not store a reference.

test/clang-tidy/temporaries.cpp
1

Add -disable-checks='', so that we don't rely on its default value.

2

Could you instead add test code which actually generates a core.NullDereference warning, so that we can be sure it's not just completely disabled.

// CHECK-NOT: warning
void testNullPointerDereferencePositive() {
  int *value = 0;
  // CHECK: :[[@LINE+1]]:3: warning: .....
  *value = 1;
}

// CHECK-NOT: warning
<the rest of your test case>
This revision is now accepted and ready to land.Apr 30 2014, 4:41 AM
alexmc updated this revision to Diff 8977.Apr 30 2014, 6:58 AM
alexmc edited edge metadata.
  • Respond to review comments: copy options instead of maintaining a reference, and add more test cases.

Submitting now.

clang-tidy/ClangTidy.h
116

Done.

test/clang-tidy/temporaries.cpp
1

Done.

2

Done.

alexfh added inline comments.Apr 30 2014, 7:21 AM
test/clang-tidy/temporaries.cpp
4

You don't need this now as the correct output is guaranteed to be non-empty. And you can now just pipe clang-tidy output to FileCheck

alexmc added inline comments.Apr 30 2014, 7:26 AM
test/clang-tidy/temporaries.cpp
4

Good catch: fixed in llvm-reviews.chandlerc.com/rL207653

alexmc closed this revision.Apr 30 2014, 9:02 AM