[PATCH] [analyzer] Fix ObjC Dealloc Checker to operate only on classes with retained properties

[Jordan Rose]

More comments. Thanks for working on this!

================
Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:91
@@ +90,3 @@
+  QualType T = (*ID)->getType();
+  if (!T->isObjCObjectPointerType())
+    return false;
----------------
This should use isObjCRetainableType, to handle blocks as well.

================
Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:94-96
@@ +93,5 @@
+
+  (*PD) = I->getPropertyDecl();
+  if (!(*PD))
+    return false;
+
----------------
I //think// you can just assert this. Sema should already reject `@synthesize` without `@property`, and it will certainly never auto-synthesize a property that doesn't exist.

================
Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:98
@@ +97,3 @@
+
+  if ((*PD)->isReadOnly())
+    return false;
----------------
I don't think this matters. As soon as the property is synthesized, it needs to be released, right?

================
Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:123-125
@@ -102,3 +122,5 @@
 
-    containsPointerIvar = true;
-    break;
+    // Property must be released if and only if the kind of setter
+    // was not 'assign'.
+    if (PD->getSetterKind() != ObjCPropertyDecl::Assign) {
+      containsRetainedSynthesizedWritablePointerProperty = true;
----------------
...or Weak.

================
Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:203
@@ -194,2 +202,3 @@
+    // was not 'assign'.
     bool requiresRelease = PD->getSetterKind() != ObjCPropertyDecl::Assign;
     if (scan_ivar_release(MD->getBody(), ID, PD, RS, SelfII, Ctx)
----------------
'Weak' also doesn't require a release.

================
Comment at: test/Analysis/DeallocMissingRelease.m:2-5
@@ +1,6 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK %s
+// CHECK: DeallocMissingRelease.m:99:1: warning: The '_ivar' instance variable in 'MyPropertyClass1' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK: DeallocMissingRelease.m:110:1: warning: The '_ivar' instance variable in 'MyPropertyClass2' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK: DeallocMissingRelease.m:127:1: warning: The '_ivar' instance variable in 'MyPropertyClass3' was retained by a synthesized property but was not released in 'dealloc'
+// CHECK: 3 warnings generated.
+
----------------
Please move these checks inline. That way they're associated with what they're checking, and you can use `[[@LINE]]` (or `[[@LINE+1]]` or `[[@LINE-1]]` to provide a relative line number.

================
Comment at: test/Analysis/DeallocMissingRelease.m:16-25
@@ +15,12 @@
+// RUN: not %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.Dealloc -fblocks -triple x86_64-apple-darwin10 -fobjc-arc %s 2>&1 | FileCheck -check-prefix=CHECK-ARC %s
+// CHECK-ARC: DeallocMissingRelease.m:58:17: error: ARC forbids explicit message send of 'retain'
+// CHECK-ARC: DeallocMissingRelease.m:63:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:82:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:90:10: error: ARC forbids explicit message send of 'release'
+// CHECK-ARC: DeallocMissingRelease.m:91:17: error: ARC forbids explicit message send of 'retain'
+// CHECK-ARC: DeallocMissingRelease.m:105:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:116:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:130:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: DeallocMissingRelease.m:150:10: error: ARC forbids explicit message send of 'dealloc'
+// CHECK-ARC: 9 errors generated.
+
----------------
This is not helpful; the analyzer doesn't even get run when there are AST-level errors. Can you #if out the problematic lines when testing under ARC?

http://reviews.llvm.org/D5023