r253534 - Don't actually add the __unsafe_unretained qualifier in MRC;

John McCall via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 18 18:28:03 PST 2015


Author: rjmccall
Date: Wed Nov 18 20:28:03 2015
New Revision: 253534

URL: http://llvm.org/viewvc/llvm-project?rev=253534&view=rev
Log:
Don't actually add the __unsafe_unretained qualifier in MRC;
driving a canonical difference between that and an unqualified
type is a really bad idea when both are valid.  Instead, remember
that it was there in a non-canonical way, then look for that in
the one place we really care about it: block captures.  The net
effect closely resembles the behavior of a decl attribute, except
still closely following ARC's standard qualifier parsing rules.

Added:
    cfe/trunk/test/CodeGenObjCXX/mrc-weak.mm
      - copied, changed from r253533, cfe/trunk/test/CodeGenObjC/mrc-weak.m
Modified:
    cfe/trunk/include/clang/AST/Type.h
    cfe/trunk/lib/AST/Type.cpp
    cfe/trunk/lib/AST/TypePrinter.cpp
    cfe/trunk/lib/CodeGen/CGBlocks.cpp
    cfe/trunk/lib/Sema/SemaType.cpp
    cfe/trunk/test/CodeGenObjC/mrc-weak.m
    cfe/trunk/test/SemaObjC/mrc-weak.m

Modified: cfe/trunk/include/clang/AST/Type.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=253534&r1=253533&r2=253534&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Wed Nov 18 20:28:03 2015
@@ -1668,6 +1668,7 @@ public:
   bool isObjCQualifiedClassType() const;        // Class<foo>
   bool isObjCObjectOrInterfaceType() const;
   bool isObjCIdType() const;                    // id
+  bool isObjCInertUnsafeUnretainedType() const;
 
   /// Whether the type is Objective-C 'id' or a __kindof type of an
   /// object type, e.g., __kindof NSView * or __kindof id
@@ -3636,6 +3637,7 @@ public:
     attr_nullable,
     attr_null_unspecified,
     attr_objc_kindof,
+    attr_objc_inert_unsafe_unretained,
   };
 
 private:

Modified: cfe/trunk/lib/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=253534&r1=253533&r2=253534&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Wed Nov 18 20:28:03 2015
@@ -510,6 +510,28 @@ bool Type::isObjCClassOrClassKindOfType(
   return OPT->isObjCClassType() || OPT->isObjCQualifiedClassType();
 }
 
+/// Was this type written with the special inert-in-MRC __unsafe_unretained
+/// qualifier?
+///
+/// This approximates the answer to the following question: if this
+/// translation unit were compiled in ARC, would this type be qualified
+/// with __unsafe_unretained?
+bool Type::isObjCInertUnsafeUnretainedType() const {
+  const Type *cur = this;
+  while (true) {
+    if (auto attributed = dyn_cast<AttributedType>(cur)) {
+      if (attributed->getAttrKind() ==
+            AttributedType::attr_objc_inert_unsafe_unretained)
+        return true;
+    }
+
+    // Single-step desugar until we run out of sugar.
+    QualType next = cur->getLocallyUnqualifiedSingleStepDesugaredType();
+    if (next.getTypePtr() == cur) return false;
+    cur = next.getTypePtr();
+  }
+}
+
 ObjCObjectType::ObjCObjectType(QualType Canonical, QualType Base,
                                ArrayRef<QualType> typeArgs,
                                ArrayRef<ObjCProtocolDecl *> protocols,
@@ -2952,6 +2974,7 @@ bool AttributedType::isQualifier() const
   case AttributedType::attr_address_space:
   case AttributedType::attr_objc_gc:
   case AttributedType::attr_objc_ownership:
+  case AttributedType::attr_objc_inert_unsafe_unretained:
   case AttributedType::attr_nonnull:
   case AttributedType::attr_nullable:
   case AttributedType::attr_null_unspecified:
@@ -3010,6 +3033,7 @@ bool AttributedType::isCallingConv() con
   case attr_neon_polyvector_type:
   case attr_objc_gc:
   case attr_objc_ownership:
+  case attr_objc_inert_unsafe_unretained:
   case attr_noreturn:
   case attr_nonnull:
   case attr_nullable:

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=253534&r1=253533&r2=253534&view=diff
==============================================================================
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Wed Nov 18 20:28:03 2015
@@ -1191,6 +1191,10 @@ void TypePrinter::printAttributedAfter(c
 
   printAfter(T->getModifiedType(), OS);
 
+  // Don't print the inert __unsafe_unretained attribute at all.
+  if (T->getAttrKind() == AttributedType::attr_objc_inert_unsafe_unretained)
+    return;
+
   // Print nullability type specifiers that occur after
   if (T->getAttrKind() == AttributedType::attr_nonnull ||
       T->getAttrKind() == AttributedType::attr_nullable ||

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=253534&r1=253533&r2=253534&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Wed Nov 18 20:28:03 2015
@@ -399,9 +399,15 @@ static void computeBlockInfo(CodeGenModu
 
     // Block pointers require copy/dispose.  So do Objective-C pointers.
     } else if (variable->getType()->isObjCRetainableType()) {
-      info.NeedsCopyDispose = true;
-      // used for mrr below.
-      lifetime = Qualifiers::OCL_Strong;
+      // But honor the inert __unsafe_unretained qualifier, which doesn't
+      // actually make it into the type system.
+       if (variable->getType()->isObjCInertUnsafeUnretainedType()) {
+        lifetime = Qualifiers::OCL_ExplicitNone;
+      } else {
+        info.NeedsCopyDispose = true;
+        // used for mrr below.
+        lifetime = Qualifiers::OCL_Strong;
+      }
 
     // So do types that require non-trivial copy construction.
     } else if (CI.hasCopyExpr()) {

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=253534&r1=253533&r2=253534&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Wed Nov 18 20:28:03 2015
@@ -4452,6 +4452,7 @@ static AttributeList::Kind getAttrListKi
   case AttributedType::attr_objc_gc:
     return AttributeList::AT_ObjCGC;
   case AttributedType::attr_objc_ownership:
+  case AttributedType::attr_objc_inert_unsafe_unretained:
     return AttributeList::AT_ObjCOwnership;
   case AttributedType::attr_noreturn:
     return AttributeList::AT_NoReturn;
@@ -5176,6 +5177,25 @@ static bool handleObjCOwnershipTypeAttr(
       << TDS_ObjCObjOrBlock << type;
   }
 
+  // Don't actually add the __unsafe_unretained qualifier in non-ARC files,
+  // because having both 'T' and '__unsafe_unretained T' exist in the type
+  // system causes unfortunate widespread consistency problems.  (For example,
+  // they're not considered compatible types, and we mangle them identicially
+  // as template arguments.)  These problems are all individually fixable,
+  // but it's easier to just not add the qualifier and instead sniff it out
+  // in specific places using isObjCInertUnsafeUnretainedType().
+  //
+  // Doing this does means we miss some trivial consistency checks that
+  // would've triggered in ARC, but that's better than trying to solve all
+  // the coexistence problems with __unsafe_unretained.
+  if (!S.getLangOpts().ObjCAutoRefCount &&
+      lifetime == Qualifiers::OCL_ExplicitNone) {
+    type = S.Context.getAttributedType(
+                             AttributedType::attr_objc_inert_unsafe_unretained,
+                                       type, type);
+    return true;
+  }
+
   QualType origType = type;
   if (!NonObjCPointer)
     type = S.Context.getQualifiedType(underlyingType);

Modified: cfe/trunk/test/CodeGenObjC/mrc-weak.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/mrc-weak.m?rev=253534&r1=253533&r2=253534&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/mrc-weak.m (original)
+++ cfe/trunk/test/CodeGenObjC/mrc-weak.m Wed Nov 18 20:28:03 2015
@@ -160,3 +160,32 @@ void test8(void) {
 
 // CHECK-LABEL: define internal void @__Block_byref_object_dispose
 // CHECK:       call void @objc_destroyWeak
+
+// CHECK-LABEL: define void @test9_baseline()
+// CHECK:       define internal void @__copy_helper
+// CHECK:       define internal void @__destroy_helper
+void test9_baseline(void) {
+  Foo *p = get_object();
+  use_block(^{ [p run]; });
+}
+
+// CHECK-LABEL: define void @test9()
+// CHECK-NOT:   define internal void @__copy_helper
+// CHECK-NOT:   define internal void @__destroy_helper
+// CHECK:       define void @test9_fin()
+void test9(void) {
+  __unsafe_unretained Foo *p = get_object();
+  use_block(^{ [p run]; });
+}
+void test9_fin() {}
+
+// CHECK-LABEL: define void @test10()
+// CHECK-NOT:   define internal void @__copy_helper
+// CHECK-NOT:   define internal void @__destroy_helper
+// CHECK:       define void @test10_fin()
+void test10(void) {
+  typedef __unsafe_unretained Foo *UnsafeFooPtr;
+  UnsafeFooPtr p = get_object();
+  use_block(^{ [p run]; });
+}
+void test10_fin() {}

Copied: cfe/trunk/test/CodeGenObjCXX/mrc-weak.mm (from r253533, cfe/trunk/test/CodeGenObjC/mrc-weak.m)
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/mrc-weak.mm?p2=cfe/trunk/test/CodeGenObjCXX/mrc-weak.mm&p1=cfe/trunk/test/CodeGenObjC/mrc-weak.m&r1=253533&r2=253534&rev=253534&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/mrc-weak.m (original)
+++ cfe/trunk/test/CodeGenObjCXX/mrc-weak.mm Wed Nov 18 20:28:03 2015
@@ -6,26 +6,6 @@
 - (void) run;
 @end
 
-// The ivars in HighlyAlignedSubclass should be placed in the tail-padding
-// of the superclass.  Ensure that they're still covered by layouts.
- at interface HighlyAligned : Object {
-  __attribute__((aligned(32))) void *array[2];
-}
- at end
-// CHECK-MODERN: @"OBJC_IVAR_$_HighlyAlignedSubclass.ivar2" = global i64 24,
-// CHECK-MODERN: @"OBJC_IVAR_$_HighlyAlignedSubclass.ivar" = global i64 16,
-// CHECK-MODERN: @OBJC_CLASS_NAME_{{.*}} = {{.*}} c"\02\00"
-// CHECK-MODERN: @"\01l_OBJC_CLASS_RO_$_HighlyAlignedSubclass" = {{.*}} {
-// CHECK-FRAGILE: @OBJC_INSTANCE_VARIABLES_HighlyAlignedSubclass = {{.*}}, i32 8 }, {{.*}}, i32 12 }]
-// CHECK-FRAGILE: @OBJC_CLASS_NAME_{{.*}} = {{.*}} c"\02\00"
-// CHECK-FRAGILE: @OBJC_CLASS_HighlyAlignedSubclass
- at interface HighlyAlignedSubclass : HighlyAligned {
-  __weak id ivar;
-  __weak id ivar2;
-}
- at end
- at implementation HighlyAlignedSubclass @end
-
 // CHECK-MODERN: @OBJC_CLASS_NAME_{{.*}} = {{.*}} c"\01\00"
 // CHECK-MODERN: @"\01l_OBJC_CLASS_RO_$_Foo" = {{.*}} { i32 772
 //   772 == 0x304
@@ -51,7 +31,7 @@
 
 
 void test1(__weak id x) {}
-// CHECK-LABEL: define void @test1
+// CHECK-LABEL: define void @_Z5test1P11objc_object(
 // CHECK:      [[X:%.*]] = alloca i8*,
 // CHECK-NEXT: objc_initWeak
 // CHECK-NEXT: objc_destroyWeak
@@ -60,7 +40,7 @@ void test1(__weak id x) {}
 void test2(id y) {
   __weak id z = y;
 }
-// CHECK-LABEL: define void @test2
+// CHECK-LABEL: define void @_Z5test2P11objc_object(
 // CHECK:      [[Y:%.*]] = alloca i8*,
 // CHECK-NEXT: [[Z:%.*]] = alloca i8*,
 // CHECK-NEXT: store
@@ -73,7 +53,7 @@ void test3(id y) {
   __weak id z;
   z = y;
 }
-// CHECK-LABEL: define void @test3
+// CHECK-LABEL: define void @_Z5test3P11objc_object(
 // CHECK:      [[Y:%.*]] = alloca i8*,
 // CHECK-NEXT: [[Z:%.*]] = alloca i8*,
 // CHECK-NEXT: store
@@ -86,7 +66,7 @@ void test3(id y) {
 void test4(__weak id *p) {
   id y = *p;
 }
-// CHECK-LABEL: define void @test4
+// CHECK-LABEL: define void @_Z5test4PU6__weakP11objc_object(
 // CHECK:      [[P:%.*]] = alloca i8**,
 // CHECK-NEXT: [[Y:%.*]] = alloca i8*,
 // CHECK-NEXT: store
@@ -98,7 +78,7 @@ void test4(__weak id *p) {
 void test5(__weak id *p) {
   id y = [*p retain];
 }
-// CHECK-LABEL: define void @test5
+// CHECK-LABEL: define void @_Z5test5PU6__weakP11objc_object
 // CHECK:      [[P:%.*]] = alloca i8**,
 // CHECK-NEXT: [[Y:%.*]] = alloca i8*,
 // CHECK-NEXT: store
@@ -110,7 +90,7 @@ void test5(__weak id *p) {
 void test6(__weak Foo **p) {
   Foo *y = [*p retain];
 }
-// CHECK-LABEL: define void @test6
+// CHECK-LABEL: define void @_Z5test6PU6__weakP3Foo
 // CHECK:      [[P:%.*]] = alloca [[FOO:%.*]]**,
 // CHECK-NEXT: [[Y:%.*]] = alloca [[FOO]]*,
 // CHECK-NEXT: store
@@ -121,14 +101,14 @@ void test6(__weak Foo **p) {
 // CHECK-NEXT: store [[FOO]]* [[T3]], [[FOO]]** [[Y]]
 // CHECK-NEXT: ret void
 
-extern id get_object(void);
-extern void use_block(void (^)(void));
+extern "C" id get_object(void);
+extern "C" void use_block(void (^)(void));
 
 void test7(void) {
   __weak Foo *p = get_object();
   use_block(^{ [p run ]; });
 }
-// CHECK-LABEL: define void @test7
+// CHECK-LABEL: define void @_Z5test7v
 // CHECK:       [[P:%.*]] = alloca [[FOO]]*,
 // CHECK:       [[T0:%.*]] = call i8* @get_object()
 // CHECK-NEXT:  [[T1:%.*]] = bitcast i8* [[T0]] to [[FOO]]*
@@ -149,7 +129,7 @@ void test8(void) {
   __block __weak Foo *p = get_object();
   use_block(^{ [p run ]; });
 }
-// CHECK-LABEL: define void @test8
+// CHECK-LABEL: define void @_Z5test8v
 // CHECK:       call i8* @objc_initWeak
 // CHECK-NOT:   call void @objc_copyWeak
 // CHECK:       call void @use_block
@@ -160,3 +140,44 @@ void test8(void) {
 
 // CHECK-LABEL: define internal void @__Block_byref_object_dispose
 // CHECK:       call void @objc_destroyWeak
+
+// CHECK-LABEL: define void @_Z14test9_baselinev()
+// CHECK:       define internal void @__copy_helper
+// CHECK:       define internal void @__destroy_helper
+void test9_baseline(void) {
+  Foo *p = get_object();
+  use_block(^{ [p run]; });
+}
+
+// CHECK-LABEL: define void @_Z5test9v()
+// CHECK-NOT:   define internal void @__copy_helper
+// CHECK-NOT:   define internal void @__destroy_helper
+// CHECK:       define void @_Z9test9_finv()
+void test9(void) {
+  __unsafe_unretained Foo *p = get_object();
+  use_block(^{ [p run]; });
+}
+void test9_fin() {}
+
+// CHECK-LABEL: define void @_Z6test10v()
+// CHECK-NOT:   define internal void @__copy_helper
+// CHECK-NOT:   define internal void @__destroy_helper
+// CHECK:       define void @_Z10test10_finv()
+void test10(void) {
+  typedef __unsafe_unretained Foo *UnsafeFooPtr;
+  UnsafeFooPtr p = get_object();
+  use_block(^{ [p run]; });
+}
+void test10_fin() {}
+
+// CHECK-LABEL: define weak_odr void @_Z6test11ILj0EEvv()
+// CHECK-NOT:   define internal void @__copy_helper
+// CHECK-NOT:   define internal void @__destroy_helper
+// CHECK:       define void @_Z10test11_finv()
+template <unsigned i> void test11(void) {
+  typedef __unsafe_unretained Foo *UnsafeFooPtr;
+  UnsafeFooPtr p = get_object();
+  use_block(^{ [p run]; });
+}
+template void test11<0>();
+void test11_fin() {}

Modified: cfe/trunk/test/SemaObjC/mrc-weak.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/mrc-weak.m?rev=253534&r1=253533&r2=253534&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/mrc-weak.m (original)
+++ cfe/trunk/test/SemaObjC/mrc-weak.m Wed Nov 18 20:28:03 2015
@@ -12,7 +12,7 @@ __attribute__((objc_root_class))
 @property (unsafe_unretained) id ud;
 @property (strong) id sa;
 @property (strong) id sb; // expected-note {{property declared here}}
- at property (strong) id sc; // expected-note {{property declared here}}
+ at property (strong) id sc;
 @property (strong) id sd;
 @end
 
@@ -25,7 +25,7 @@ __attribute__((objc_root_class))
   __unsafe_unretained id _uc;
   id _sa;
   __weak id _sb; // expected-error {{existing instance variable '_sb' for strong property 'sb' may not be __weak}}
-  __unsafe_unretained id _sc; // expected-error {{existing instance variable '_sc' for strong property 'sc' may not be __unsafe_unretained}}
+  __unsafe_unretained id _sc;
 }
 @synthesize wa = _wa; // expected-note {{property synthesized here}}
 @synthesize wb = _wb;
@@ -37,7 +37,7 @@ __attribute__((objc_root_class))
 @synthesize ud = _ud;
 @synthesize sa = _sa;
 @synthesize sb = _sb; // expected-note {{property synthesized here}}
- at synthesize sc = _sc; // expected-note {{property synthesized here}}
+ at synthesize sc = _sc;
 @synthesize sd = _sd;
 @end
 
@@ -62,6 +62,6 @@ void test_unsafe_unretained_cast(id *val
 
 void test_cast_qualifier_inference(__weak id *value) {
   __weak id *a = (id*) value;
-  __unsafe_unretained id *b = (id*) value; // expected-error {{initializing '__unsafe_unretained id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
+  __unsafe_unretained id *b = (id*) value; // expected-error {{initializing 'id *' with an expression of type '__weak id *' changes retain/release properties of pointer}}
 }
 




More information about the cfe-commits mailing list