Extend checkUnsafeAssigns() to also handle assigning an object literal to a weak...

Thanks to Jordan Rose and John McCall for their sage code review.

Fixes <rdar://problem/12569201>.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170864 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index c49b875c6138eda0951d169691b73bb8dd3ecca8..c66b9702fadf69bfae6d9cb332143f9b0c1daa9d 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3689,6 +3689,11 @@ def warn_arc_retained_property_assign : Warning<
   "assigning retained object to unsafe property"
   "; object will be released after assignment">,
   InGroup<ARCUnsafeRetainedAssign>;
+def warn_arc_literal_assign : Warning<
+  "assigning %select{dictionary literal|array literal|block literal|boxed expression}0"
+  " to a weak %select{property|variable}1"
+  "; object will be released after assignment">,
+  InGroup<ARCUnsafeRetainedAssign>;
 def err_arc_new_array_without_ownership : Error<
   "'new' cannot allocate an array of %0 with no explicit ownership">;
 def err_arc_autoreleasing_var : Error<

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index eafe5956878c9f9d4acb4ccb45c4168c6c637f86..f58396f14bf9b4ec2f99b88eb105a95998f5e574 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -5766,6 +5766,39 @@ static bool checkUnsafeAssignObject(Sema &S, SourceLocation Loc,
   return false;
 }
 
+static bool checkUnsafeAssignLiteral(Sema &S, SourceLocation Loc,
+                                     Expr *RHS, bool isProperty) {
+  // Check if RHS is an Objective-C object literal, which also can get
+  // immediately zapped in a weak reference.  Note that we explicitly
+  // allow ObjCStringLiterals, since those are designed to never really die.
+  RHS = RHS->IgnoreParenImpCasts();
+  unsigned kind = 4;
+  switch (RHS->getStmtClass()) {
+    default:
+      break;
+    case Stmt::ObjCDictionaryLiteralClass:
+      kind = 0;
+      break;
+    case Stmt::ObjCArrayLiteralClass:
+      kind = 1;
+      break;
+    case Stmt::BlockExprClass:
+      kind = 2;
+      break;
+    case Stmt::ObjCBoxedExprClass:
+      kind = 3;
+      break;
+  }
+  if (kind < 4) {
+    S.Diag(Loc, diag::warn_arc_literal_assign)
+    << kind
+    << (isProperty ? 0 : 1)
+    << RHS->getSourceRange();
+    return true;
+  }
+  return false;
+}
+
 bool Sema::checkUnsafeAssigns(SourceLocation Loc,
                               QualType LHS, Expr *RHS) {
   Qualifiers::ObjCLifetime LT = LHS.getObjCLifetime();
@@ -5776,6 +5809,10 @@ bool Sema::checkUnsafeAssigns(SourceLocation Loc,
   if (checkUnsafeAssignObject(*this, Loc, LT, RHS, false))
     return true;
 
+  if (LT == Qualifiers::OCL_Weak &&
+      checkUnsafeAssignLiteral(*this, Loc, RHS, false))
+    return true;
+
   return false;
 }
 
@@ -5840,6 +5877,8 @@ void Sema::checkUnsafeExprAssigns(SourceLocation Loc,
     else if (Attributes & ObjCPropertyDecl::OBJC_PR_weak) {
       if (checkUnsafeAssignObject(*this, Loc, Qualifiers::OCL_Weak, RHS, true))
         return;
+      if (checkUnsafeAssignLiteral(*this, Loc, RHS, true))
+        return;
     }
   }
 }

diff --git a/test/SemaObjC/arc.m b/test/SemaObjC/arc.m
index bd30715a6b33c961dc8cbc0552e7a40787af06cc..08216b6d80ef233e8bfc00668f4412d5498f186a 100644 (file)
--- a/test/SemaObjC/arc.m
+++ b/test/SemaObjC/arc.m
@@ -4,6 +4,21 @@ typedef unsigned long NSUInteger;
 typedef const void * CFTypeRef;
 CFTypeRef CFBridgingRetain(id X);
 id CFBridgingRelease(CFTypeRef);
+@protocol NSCopying @end
+@interface NSDictionary
++ (id)dictionaryWithObjects:(const id [])objects forKeys:(const id <NSCopying> [])keys count:(NSUInteger)cnt;
+- (void)setObject:(id)object forKeyedSubscript:(id)key;
+@end
+@class NSFastEnumerationState;
+@protocol NSFastEnumeration
+- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id [])buffer count:(NSUInteger)len;
+@end
+@interface NSNumber 
++ (NSNumber *)numberWithInt:(int)value;
+@end
+@interface NSArray <NSFastEnumeration>
++ (id)arrayWithObjects:(const id [])objects count:(NSUInteger)cnt;
+@end
 
 void test0(void (*fn)(int), int val) {
   fn(val);
@@ -717,3 +732,19 @@ void _NSCalcBeze(NSColor* color, NSColor* bezelColors[]); // expected-error {{mu
 - init { return 0; }
 @end
 
+// <rdar://problem/12569201>.  Warn on cases of initializing a weak variable
+// with an Objective-C object literal.
+void rdar12569201(id key, id value) {
+    // Declarations.
+    __weak id x = @"foo"; // no-warning
+    __weak id y = @{ key : value }; // expected-warning {{assigning dictionary literal to a weak variable; object will be released after assignment}}
+    __weak id z = @[ value ]; // expected-warning {{assigning array literal to a weak variable; object will be released after assignment}}
+    __weak id b = ^() {}; // expected-warning {{assigning block literal to a weak variable; object will be released after assignment}}
+    __weak id e = @(42); // expected-warning {{assigning boxed expression to a weak variable; object will be released after assignment}}
+    
+    // Assignments.
+    y = @{ key : value }; // expected-warning {{assigning dictionary literal to a weak variable; object will be released after assignment}}
+    z = @[ value ]; // expected-warning {{assigning array literal to a weak variable; object will be released after assignment}}
+    b = ^() {}; // expected-warning {{assigning block literal to a weak variable; object will be released after assignment}}
+    e = @(42); // expected-warning {{assigning boxed expression to a weak variable; object will be released after assignment}}
+}

diff --git a/test/SemaObjC/warn-retain-block-property.m b/test/SemaObjC/warn-retain-block-property.m
index 3a54baf0f5a91b847cabb9b7030276f52a41b8bb..94a2f7ff9408e3ed0ba87067eed496c64f6f08f1 100644 (file)
--- a/test/SemaObjC/warn-retain-block-property.m
+++ b/test/SemaObjC/warn-retain-block-property.m
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -Wno-objc-root-class %s > %t.arc 2>&1 ; FileCheck --check-prefix=CHECK-ARC --input-file=%t.arc %s
 // rdar://9829425
-// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wno-objc-root-class %s
+
+// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-objc-root-class %s > %t 2>&1 ; FileCheck --input-file=%t %s
 // rdar://11761511
 
 extern void doSomething();
@@ -10,12 +11,12 @@ extern void doSomething();
 @public
   void (^aBlock)(void);
 }
-@property (retain) void (^aBlock)(void); // expected-warning {{retain'ed block property does not copy the block - use copy attribute instead}}
-@property (weak, retain) void (^aBlockW)(void); // expected-error {{property attributes 'retain' and 'weak' are mutually exclusive}} 
+@property (retain) void (^aBlock)(void);
+@property (weak, retain) void (^aBlockW)(void);
 @property (strong, retain) void (^aBlockS)(void); // OK
 @property (readonly, retain) void (^aBlockR)(void); // OK
-@property (copy, retain) void (^aBlockC)(void); // expected-error {{property attributes 'copy' and 'retain' are mutually exclusive}}
-@property (assign, retain) void (^aBlockA)(void); // expected-error {{property attributes 'assign' and 'retain' are mutually exclusive}}
+@property (copy, retain) void (^aBlockC)(void);
+@property (assign, retain) void (^aBlockA)(void);
 @end
 
 @implementation Test
@@ -30,3 +31,33 @@ int main() {
   t.aBlockS = ^{ doSomething(); };
 }
 
+// CHECK-ARC: 14:1: warning: retain'ed block property does not copy the block - use copy attribute instead
+// CHECK-ARC: @property (retain) void (^aBlock)(void);
+// CHECK-ARC: ^
+// CHECK-ARC: 15:1: error: property attributes 'retain' and 'weak' are mutually exclusive
+// CHECK-ARC: @property (weak, retain) void (^aBlockW)(void);
+// CHECK-ARC: ^
+// CHECK-ARC: 18:1: error: property attributes 'copy' and 'retain' are mutually exclusive
+// CHECK-ARC: @property (copy, retain) void (^aBlockC)(void);
+// CHECK-ARC: ^
+// CHECK-ARC: 19:1: error: property attributes 'assign' and 'retain' are mutually exclusive
+// CHECK-ARC: @property (assign, retain) void (^aBlockA)(void);
+// CHECK-ARC: ^
+// CHECK-ARC: 30:13: warning: assigning block literal to a weak property; object will be released after assignment
+// CHECK-ARC:   t.aBlockW = ^{ doSomething(); };
+// CHECK-ARC:             ^ ~~~~~~~~~~~~~~~~~~~
+// CHECK-ARC: 2 warnings and 3 errors generated.
+
+// CHECK: 14:1: warning: retain'ed block property does not copy the block - use copy attribute instead
+// CHECK: @property (retain) void (^aBlock)(void);
+// CHECK: ^
+// CHECK: 15:1: error: property attributes 'retain' and 'weak' are mutually exclusive
+// CHECK: @property (weak, retain) void (^aBlockW)(void);
+// CHECK: ^
+// CHECK: 18:1: error: property attributes 'copy' and 'retain' are mutually exclusive
+// CHECK: @property (copy, retain) void (^aBlockC)(void);
+// CHECK: ^
+// CHECK: 19:1: error: property attributes 'assign' and 'retain' are mutually exclusive
+// CHECK: @property (assign, retain) void (^aBlockA)(void);
+// CHECK: ^
+// CHECK: 1 warning and 3 errors generated.