Add a warning (off by default) for repeated use of the same weak property.
The motivating example:
if (self.weakProp)
use(self.weakProp);
As with any non-atomic test-then-use, it is possible a weak property to be
non-nil at the 'if', but be deallocated by the time it is used. The correct
way to write this example is as follows:
id tmp = self.weakProp;
if (tmp)
use(tmp);
The warning is controlled by -Warc-repeated-use-of-receiver, and uses the
property name and base to determine if the same property on the same object
is being accessed multiple times. In cases where the base is more
complicated than just a single Decl (e.g. 'foo.bar.weakProp'), it picks a
Decl for some degree of uniquing and reports the problem under a subflag,
-Warc-maybe-repeated-use-of-receiver. This gives a way to tune the
aggressiveness of the warning for a particular project.
The warning is not on by default because it is not flow-sensitive and thus
may have a higher-than-acceptable rate of false positives, though it is
less noisy than -Wreceiver-is-weak. On the other hand, it will not warn
about some cases that may be legitimate issues that -Wreceiver-is-weak
will catch, and it does not attempt to reason about methods returning weak
values.
Even though this is not a real "analysis-based" check I've put the bug
emission code in AnalysisBasedWarnings for two reasons: (1) to run on
every kind of code body (function, method, block, or lambda), and (2) to
suggest that it may be enhanced by flow-sensitive analysis in the future.
The second (smaller) half of this work is to extend it to weak locals
and weak ivars. This should use most of the same infrastructure.
Part of <rdar://problem/12280249>
llvm-svn: 164854
diff --git a/clang/test/SemaObjC/arc-repeated-weak.mm b/clang/test/SemaObjC/arc-repeated-weak.mm
new file mode 100644
index 0000000..728ffcb
--- /dev/null
+++ b/clang/test/SemaObjC/arc-repeated-weak.mm
@@ -0,0 +1,203 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -Warc-repeated-use-of-weak -verify %s
+
+@interface Test {
+@public
+ Test *ivar;
+}
+@property(weak) Test *weakProp;
+@property(strong) Test *strongProp;
+
+- (__weak id)implicitProp;
+
++ (__weak id)weakProp;
+@end
+
+extern void use(id);
+extern id get();
+extern bool condition();
+#define nil ((id)0)
+
+void sanity(Test *a) {
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(a.weakProp); // expected-note{{also accessed here}}
+
+ use(a.strongProp);
+ use(a.strongProp); // no-warning
+
+ use(a.weakProp); // expected-note{{also accessed here}}
+}
+
+void singleUse(Test *a) {
+ use(a.weakProp); // no-warning
+ use(a.strongProp); // no-warning
+}
+
+void assignsOnly(Test *a) {
+ a.weakProp = get(); // no-warning
+
+ id next = get();
+ if (next)
+ a.weakProp = next; // no-warning
+}
+
+void assignThenRead(Test *a) {
+ a.weakProp = get(); // expected-note{{also accessed here}}
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void twoVariables(Test *a, Test *b) {
+ use(a.weakProp); // no-warning
+ use(b.weakProp); // no-warning
+}
+
+void doubleLevelAccess(Test *a) {
+ use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(a.strongProp.weakProp); // expected-note{{also accessed here}}
+}
+
+void doubleLevelAccessIvar(Test *a) {
+ use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(a.strongProp.weakProp); // expected-note{{also accessed here}}
+}
+
+void implicitProperties(Test *a) {
+ use(a.implicitProp); // expected-warning{{weak property is accessed multiple times}}
+ use(a.implicitProp); // expected-note{{also accessed here}}
+}
+
+void classProperties() {
+ use(Test.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ use(Test.weakProp); // expected-note{{also accessed here}}
+}
+
+void classPropertiesAreDifferent(Test *a) {
+ use(Test.weakProp); // no-warning
+ use(a.weakProp); // no-warning
+ use(a.strongProp.weakProp); // no-warning
+}
+
+
+void assignToStrongWrongInit(Test *a) {
+ id val = a.weakProp; // expected-note{{also accessed here}}
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void assignToStrongWrong(Test *a) {
+ id val;
+ val = a.weakProp; // expected-note{{also accessed here}}
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void assignToStrongOK(Test *a) {
+ if (condition()) {
+ id val = a.weakProp; // no-warning
+ (void)val;
+ } else {
+ id val;
+ val = a.weakProp; // no-warning
+ (void)val;
+ }
+}
+
+void assignToStrongConditional(Test *a) {
+ id val = (condition() ? a.weakProp : a.weakProp); // no-warning
+ id val2 = a.implicitProp ?: a.implicitProp; // no-warning
+}
+
+void testBlock(Test *a) {
+ use(a.weakProp); // no-warning
+
+ use(^{
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this block}}
+ use(a.weakProp); // expected-note{{also accessed here}}
+ });
+}
+
+
+@interface Test (Methods)
+@end
+
+@implementation Test (Methods)
+- (void)sanity {
+ use(self.weakProp); // expected-warning{{weak property is accessed multiple times in this method but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(self.weakProp); // expected-note{{also accessed here}}
+}
+
+- (void)doubleLevelAccessForSelf {
+ use(self.strongProp.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ use(self.strongProp.weakProp); // expected-note{{also accessed here}}
+
+ use(self->ivar.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ use(self->ivar.weakProp); // expected-note{{also accessed here}}
+}
+
+- (void)distinctFromOther:(Test *)other {
+ use(self.strongProp.weakProp); // no-warning
+ use(other.strongProp.weakProp); // no-warning
+
+ use(self->ivar.weakProp); // no-warning
+ use(other->ivar.weakProp); // no-warning
+}
+@end
+
+
+class Wrapper {
+ Test *a;
+
+public:
+ void fields() {
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
+ use(a.weakProp); // expected-note{{also accessed here}}
+ }
+
+ void distinctFromOther(Test *b, const Wrapper &w) {
+ use(a.weakProp); // no-warning
+ use(b.weakProp); // no-warning
+ use(w.a.weakProp); // no-warning
+ }
+
+ static void doubleLevelAccessField(const Wrapper &x, const Wrapper &y) {
+ use(x.a.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(y.a.weakProp); // expected-note{{also accessed here}}
+ }
+};
+
+
+// -----------------------
+// False positives
+// -----------------------
+
+// Most of these would require flow-sensitive analysis to silence correctly.
+
+void assignAfterRead(Test *a) {
+ if (!a.weakProp) // expected-warning{{weak property is accessed multiple times}}
+ a.weakProp = get(); // expected-note{{also accessed here}}
+}
+
+void assignNil(Test *a) {
+ if (condition())
+ a.weakProp = nil; // expected-note{{also accessed here}}
+
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+}
+
+void branch(Test *a) {
+ if (condition())
+ use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
+ else
+ use(a.weakProp); // expected-note{{also accessed here}}
+}
+
+void doubleLevelAccess(Test *a, Test *b) {
+ use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(b.strongProp.weakProp); // expected-note{{also accessed here}}
+
+ use(a.weakProp.weakProp); // no-warning
+}
+
+void doubleLevelAccessIvar(Test *a, Test *b) {
+ use(a->ivar.weakProp); // expected-warning{{weak property may be accessed multiple times}}
+ use(b->ivar.weakProp); // expected-note{{also accessed here}}
+
+ use(a.strongProp.weakProp); // no-warning
+}