]> granicus.if.org Git - clang/commitdiff
Fix a pair of bugs relating to properties in ARC.
authorJohn McCall <rjmccall@apple.com>
Mon, 20 Aug 2012 23:36:59 +0000 (23:36 +0000)
committerJohn McCall <rjmccall@apple.com>
Mon, 20 Aug 2012 23:36:59 +0000 (23:36 +0000)
First, when synthesizing an explicitly strong/retain/copy property
of Class type, don't pretend during compatibility checking that the
property is actually assign.  Instead, resolve incompatibilities
by secretly changing the type of *implicitly* __unsafe_unretained
Class ivars to be strong.  This is moderately evil but better than
what we were doing.

Second, when synthesizing the setter for a strong property of
non-retainable type, be sure to use objc_setProperty.  This is
possible when the property is decorated with the NSObject
attribute.  This is an ugly, ugly corner of the language, and
we probably ought to deprecate it.

The first is rdar://problem/12039404;  the second was noticed by
inspection while fixing the first.

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

docs/AutomaticReferenceCounting.html
lib/CodeGen/CGObjC.cpp
lib/Sema/SemaObjCProperty.cpp
test/CodeGenObjC/arc-property.m

index 3f1ccaf672ca87ea9779b035f09d396e089185d5..2fd82dda65c577c68b60119627059cbaa378a9de 100644 (file)
@@ -888,6 +888,15 @@ from non-ARC practice was acceptable because we had conservatively
 banned the synthesis in order to give ourselves exactly this
 leeway.</p></div>
 
+<p>Applying <tt>__attribute__((NSObject))</tt> to a property not of
+retainable object pointer type has the same behavior it does outside
+of ARC:  it requires the property type to be some sort of pointer and
+permits the use of modifiers other than <tt>assign</tt>.  These
+modifiers only affect the synthesized getter and setter; direct
+accesses to the ivar (even if synthesized) still have primitive
+semantics, and the value in the ivar will not be automatically
+released during deallocation.</p>
+
 </div> <!-- ownership.spelling.property -->
 
 </div> <!-- ownership.spelling -->
index 4ac172d1cbb5a81d5320332cd98c365348595aa8..e0762846dc1a11af7384163d08157909264678fd 100644 (file)
@@ -613,7 +613,16 @@ PropertyImplStrategy::PropertyImplStrategy(CodeGenModule &CGM,
     // which translates to objc_storeStrong.  This isn't required, but
     // it's slightly nicer.
     } else if (CGM.getLangOpts().ObjCAutoRefCount && !IsAtomic) {
-      Kind = Expression;
+      // Using standard expression emission for the setter is only
+      // acceptable if the ivar is __strong, which won't be true if
+      // the property is annotated with __attribute__((NSObject)).
+      // TODO: falling all the way back to objc_setProperty here is
+      // just laziness, though;  we could still use objc_storeStrong
+      // if we hacked it right.
+      if (ivarType.getObjCLifetime() == Qualifiers::OCL_Strong)
+        Kind = Expression;
+      else
+        Kind = SetPropertyAndExpressionGet;
       return;
 
     // Otherwise, we need to at least use setProperty.  However, if
index 27deab226f911261c1f8bb757f53aff824be9d85..8a8927d362f183a6511280c93ed721f22fbc83a0 100644 (file)
@@ -43,7 +43,7 @@ static Qualifiers::ObjCLifetime getImpliedARCOwnership(
   if (attrs & (ObjCPropertyDecl::OBJC_PR_retain |
                ObjCPropertyDecl::OBJC_PR_strong |
                ObjCPropertyDecl::OBJC_PR_copy)) {
-    return type->getObjCARCImplicitLifetime();
+    return Qualifiers::OCL_Strong;
   } else if (attrs & ObjCPropertyDecl::OBJC_PR_weak) {
     return Qualifiers::OCL_Weak;
   } else if (attrs & ObjCPropertyDecl::OBJC_PR_unsafe_unretained) {
@@ -543,6 +543,23 @@ static void checkARCPropertyImpl(Sema &S, SourceLocation propertyImplLoc,
       ivarLifetime == Qualifiers::OCL_Autoreleasing)
     return;
 
+  // If the ivar is private, and it's implicitly __unsafe_unretained
+  // becaues of its type, then pretend it was actually implicitly
+  // __strong.  This is only sound because we're processing the
+  // property implementation before parsing any method bodies.
+  if (ivarLifetime == Qualifiers::OCL_ExplicitNone &&
+      propertyLifetime == Qualifiers::OCL_Strong &&
+      ivar->getAccessControl() == ObjCIvarDecl::Private) {
+    SplitQualType split = ivarType.split();
+    if (split.Quals.hasObjCLifetime()) {
+      assert(ivarType->isObjCARCImplicitlyUnretainedType());
+      split.Quals.setObjCLifetime(Qualifiers::OCL_Strong);
+      ivarType = S.Context.getQualifiedType(split);
+      ivar->setType(ivarType);
+      return;
+    }
+  }
+
   switch (propertyLifetime) {
   case Qualifiers::OCL_Strong:
     S.Diag(propertyImplLoc, diag::err_arc_strong_property_ownership)
index 6c5180b1c347e24fcc184cdf79f44838cd85e919..68344b83b027a561542832aa6a4c7210f1c42818 100644 (file)
@@ -13,3 +13,75 @@ void test0(Test0 *t0, id value) {
 // CHECK: @objc_msgSend
 // CHECK: call void @objc_release(
 // CHECK: call void @objc_release(
+
+struct S1 { Class isa; };
+@interface Test1
+@property (nonatomic, strong) __attribute__((NSObject)) struct S1 *pointer;
+@end
+@implementation Test1
+@synthesize pointer;
+@end
+//   The getter should be a simple load.
+// CHECK:    define internal [[S1:%.*]]* @"\01-[Test1 pointer]"(
+// CHECK:      [[OFFSET:%.*]] = load i64* @"OBJC_IVAR_$_Test1.pointer"
+// CHECK-NEXT: [[T0:%.*]] = bitcast [[TEST1:%.*]]* {{%.*}} to i8*
+// CHECK-NEXT: [[T1:%.*]] = getelementptr inbounds i8* [[T0]], i64 [[OFFSET]]
+// CHECK-NEXT: [[T2:%.*]] = bitcast i8* [[T1]] to [[S1]]**
+// CHECK-NEXT: [[T3:%.*]] = load [[S1]]** [[T2]], align 8
+// CHECK-NEXT: ret [[S1]]* [[T3]]
+
+//   The setter should be using objc_setProperty.
+// CHECK:    define internal void @"\01-[Test1 setPointer:]"(
+// CHECK:      [[T0:%.*]] = bitcast [[TEST1]]* {{%.*}} to i8*
+// CHECK-NEXT: [[OFFSET:%.*]] = load i64* @"OBJC_IVAR_$_Test1.pointer"
+// CHECK-NEXT: [[T1:%.*]] = load [[S1]]** {{%.*}}
+// CHECK-NEXT: [[T2:%.*]] = bitcast [[S1]]* [[T1]] to i8*
+// CHECK-NEXT: call void @objc_setProperty(i8* [[T0]], i8* {{%.*}}, i64 [[OFFSET]], i8* [[T2]], i1 zeroext false, i1 zeroext false)
+// CHECK-NEXT: ret void
+
+
+// rdar://problem/12039404
+@interface Test2 {
+@private
+  Class _theClass;
+}
+@property (copy) Class theClass;
+@end
+
+static Class theGlobalClass;
+@implementation Test2
+@synthesize theClass = _theClass;
+- (void) test {
+  _theClass = theGlobalClass;
+}
+@end
+// CHECK:    define internal void @"\01-[Test2 test]"(
+// CHECK:      [[T0:%.*]] = load i8** @theGlobalClass, align 8
+// CHECK-NEXT: [[T1:%.*]] = load [[TEST2:%.*]]**
+// CHECK-NEXT: [[OFFSET:%.*]] = load i64* @"OBJC_IVAR_$_Test2._theClass"
+// CHECK-NEXT: [[T2:%.*]] = bitcast [[TEST2]]* [[T1]] to i8*
+// CHECK-NEXT: [[T3:%.*]] = getelementptr inbounds i8* [[T2]], i64 [[OFFSET]]
+// CHECK-NEXT: [[T4:%.*]] = bitcast i8* [[T3]] to i8**
+// CHECK-NEXT: call void @objc_storeStrong(i8** [[T4]], i8* [[T0]]) nounwind
+// CHECK-NEXT: ret void
+
+// CHECK:    define internal i8* @"\01-[Test2 theClass]"(
+// CHECK:      [[OFFSET:%.*]] = load i64* @"OBJC_IVAR_$_Test2._theClass"
+// CHECK-NEXT: [[T0:%.*]] = call i8* @objc_getProperty(i8* {{.*}}, i8* {{.*}}, i64 [[OFFSET]], i1 zeroext true)
+// CHECK-NEXT: ret i8* [[T0]]
+
+// CHECK:    define internal void @"\01-[Test2 setTheClass:]"(
+// CHECK:      [[T0:%.*]] = bitcast [[TEST2]]* {{%.*}} to i8*
+// CHECK-NEXT: [[OFFSET:%.*]] = load i64* @"OBJC_IVAR_$_Test2._theClass"
+// CHECK-NEXT: [[T1:%.*]] = load i8** {{%.*}}
+// CHECK-NEXT: call void @objc_setProperty(i8* [[T0]], i8* {{%.*}}, i64 [[OFFSET]], i8* [[T1]], i1 zeroext true, i1 zeroext true)
+// CHECK-NEXT: ret void
+
+// CHECK:    define internal void @"\01-[Test2 .cxx_destruct]"(
+// CHECK:      [[T0:%.*]] = load [[TEST2]]**
+// CHECK-NEXT: [[OFFSET:%.*]] = load i64* @"OBJC_IVAR_$_Test2._theClass"
+// CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST2]]* [[T0]] to i8*
+// CHECK-NEXT: [[T2:%.*]] = getelementptr inbounds i8* [[T1]], i64 [[OFFSET]]
+// CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to i8**
+// CHECK-NEXT: call void @objc_storeStrong(i8** [[T3]], i8* null) nounwind
+// CHECK-NEXT: ret void