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. llvm-svn: 253534
2015-11-19 02:28:03 +00:00 · 2015-11-19 02:28:03 +00:00 · 00b2bbb7bb
commit 00b2bbb7bb
parent d80218fa42
8 changed files with 275 additions and 7 deletions
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@ -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:
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@ -510,6 +510,28 @@ bool Type::isObjCClassOrClassKindOfType() const {
  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() const {
  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:
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@ -1191,6 +1191,10 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,

  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 ||
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@ -399,9 +399,15 @@ static void computeBlockInfo(CodeGenModule &CGM, CodeGenFunction *CGF,

    // 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()) {
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@ -4452,6 +4452,7 @@ static AttributeList::Kind getAttrListKind(AttributedType::Kind kind) {
  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(TypeProcessingState &state,
      << 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);
--- a/clang/test/CodeGenObjC/mrc-weak.m
+++ b/clang/test/CodeGenObjC/mrc-weak.m
@ -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() {}
--- a/clang/test/CodeGenObjCXX/mrc-weak.mm
+++ b/clang/test/CodeGenObjCXX/mrc-weak.mm
@ -0,0 +1,183 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-10.10 -emit-llvm -fblocks -fobjc-weak -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-MODERN
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -fobjc-runtime=macosx-fragile-10.10 -emit-llvm -fblocks -fobjc-weak -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-FRAGILE
+
+@interface Object
+- (instancetype) retain;
+- (void) run;
+@end
+
+// CHECK-MODERN: @OBJC_CLASS_NAME_{{.*}} = {{.*}} c"\01\00"
+// CHECK-MODERN: @"\01l_OBJC_CLASS_RO_$_Foo" = {{.*}} { i32 772
+//   772 == 0x304
+//            ^ HasMRCWeakIvars
+//            ^ HasCXXDestructorOnly
+//              ^ HasCXXStructors
+
+// CHECK-FRAGILE: @OBJC_CLASS_NAME_{{.*}} = {{.*}} c"\01\00"
+// CHECK-FRAGILE: @OBJC_CLASS_Foo = {{.*}} i32 134225921,
+//   134225921 == 0x08002001
+//                   ^ HasMRCWeakIvars
+//                      ^ HasCXXStructors
+//                         ^ Factory
+@interface Foo : Object {
+  __weak id ivar;
+}
+@end
+
+@implementation Foo
+// CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"
+// CHECK: call void @objc_destroyWeak
+@end
+
+
+void test1(__weak id x) {}
+// CHECK-LABEL: define void @_Z5test1P11objc_object(
+// CHECK:      [[X:%.*]] = alloca i8*,
+// CHECK-NEXT: objc_initWeak
+// CHECK-NEXT: objc_destroyWeak
+// CHECK-NEXT: ret void
+
+void test2(id y) {
+  __weak id z = y;
+}
+// CHECK-LABEL: define void @_Z5test2P11objc_object(
+// CHECK:      [[Y:%.*]] = alloca i8*,
+// CHECK-NEXT: [[Z:%.*]] = alloca i8*,
+// CHECK-NEXT: store
+// CHECK-NEXT: [[T0:%.*]] = load i8*, i8** [[Y]]
+// CHECK-NEXT: call i8* @objc_initWeak(i8** [[Z]], i8* [[T0]])
+// CHECK-NEXT: call void @objc_destroyWeak(i8** [[Z]])
+// CHECK-NEXT: ret void
+
+void test3(id y) {
+  __weak id z;
+  z = y;
+}
+// CHECK-LABEL: define void @_Z5test3P11objc_object(
+// CHECK:      [[Y:%.*]] = alloca i8*,
+// CHECK-NEXT: [[Z:%.*]] = alloca i8*,
+// CHECK-NEXT: store
+// CHECK-NEXT: store i8* null, i8** [[Z]]
+// CHECK-NEXT: [[T0:%.*]] = load i8*, i8** [[Y]]
+// CHECK-NEXT: call i8* @objc_storeWeak(i8** [[Z]], i8* [[T0]])
+// CHECK-NEXT: call void @objc_destroyWeak(i8** [[Z]])
+// CHECK-NEXT: ret void
+
+void test4(__weak id *p) {
+  id y = *p;
+}
+// CHECK-LABEL: define void @_Z5test4PU6__weakP11objc_object(
+// CHECK:      [[P:%.*]] = alloca i8**,
+// CHECK-NEXT: [[Y:%.*]] = alloca i8*,
+// CHECK-NEXT: store
+// CHECK-NEXT: [[T0:%.*]] = load i8**, i8*** [[P]]
+// CHECK-NEXT: [[T1:%.*]] = call i8* @objc_loadWeak(i8** [[T0]])
+// CHECK-NEXT: store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT: ret void
+
+void test5(__weak id *p) {
+  id y = [*p retain];
+}
+// CHECK-LABEL: define void @_Z5test5PU6__weakP11objc_object
+// CHECK:      [[P:%.*]] = alloca i8**,
+// CHECK-NEXT: [[Y:%.*]] = alloca i8*,
+// CHECK-NEXT: store
+// CHECK-NEXT: [[T0:%.*]] = load i8**, i8*** [[P]]
+// CHECK-NEXT: [[T1:%.*]] = call i8* @objc_loadWeakRetained(i8** [[T0]])
+// CHECK-NEXT: store i8* [[T1]], i8** [[Y]]
+// CHECK-NEXT: ret void
+
+void test6(__weak Foo **p) {
+  Foo *y = [*p retain];
+}
+// CHECK-LABEL: define void @_Z5test6PU6__weakP3Foo
+// CHECK:      [[P:%.*]] = alloca [[FOO:%.*]]**,
+// CHECK-NEXT: [[Y:%.*]] = alloca [[FOO]]*,
+// CHECK-NEXT: store
+// CHECK-NEXT: [[T0:%.*]] = load [[FOO]]**, [[FOO]]*** [[P]]
+// CHECK-NEXT: [[T1:%.*]] = bitcast [[FOO]]** [[T0]] to i8**
+// CHECK-NEXT: [[T2:%.*]] = call i8* @objc_loadWeakRetained(i8** [[T1]])
+// CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[FOO]]*
+// CHECK-NEXT: store [[FOO]]* [[T3]], [[FOO]]** [[Y]]
+// CHECK-NEXT: ret 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 @_Z5test7v
+// CHECK:       [[P:%.*]] = alloca [[FOO]]*,
+// CHECK:       [[T0:%.*]] = call i8* @get_object()
+// CHECK-NEXT:  [[T1:%.*]] = bitcast i8* [[T0]] to [[FOO]]*
+// CHECK-NEXT:  [[T2:%.*]] = bitcast [[FOO]]** [[P]] to i8**
+// CHECK-NEXT:  [[T3:%.*]] = bitcast [[FOO]]* [[T1]] to i8*
+// CHECK-NEXT:  call i8* @objc_initWeak(i8** [[T2]], i8* [[T3]])
+// CHECK:       call void @objc_copyWeak
+// CHECK:       call void @use_block
+// CHECK:       call void @objc_destroyWeak
+
+// CHECK-LABEL: define internal void @__copy_helper_block
+// CHECK:       @objc_copyWeak
+
+// CHECK-LABEL: define internal void @__destroy_helper_block
+// CHECK:       @objc_destroyWeak
+
+void test8(void) {
+  __block __weak Foo *p = get_object();
+  use_block(^{ [p run ]; });
+}
+// CHECK-LABEL: define void @_Z5test8v
+// CHECK:       call i8* @objc_initWeak
+// CHECK-NOT:   call void @objc_copyWeak
+// CHECK:       call void @use_block
+// CHECK:       call void @objc_destroyWeak
+
+// CHECK-LABEL: define internal void @__Block_byref_object_copy
+// CHECK:       call void @objc_moveWeak
+
+// 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() {}
--- a/clang/test/SemaObjC/mrc-weak.m
+++ b/clang/test/SemaObjC/mrc-weak.m
@ -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}}
-@property (strong) id sc; // expected-note {{property declared here}}
+@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}}
-@synthesize sc = _sc; // expected-note {{property synthesized here}}
+@synthesize sc = _sc;
@synthesize sd = _sd;
@end

@ -62,6 +62,6 @@ void test_unsafe_unretained_cast(id *value) {

 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}}
 }