From: John McCall Date: Thu, 19 Nov 2015 02:28:03 +0000 (+0000) Subject: Don't actually add the __unsafe_unretained qualifier in MRC; X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2419506e4a00819d96f1bb4baa812dea412a94bc;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@253534 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h index e9aadec802..0c08130da6 100644 --- a/include/clang/AST/Type.h +++ b/include/clang/AST/Type.h @@ -1668,6 +1668,7 @@ public: bool isObjCQualifiedClassType() const; // Class 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: diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 611218ff91..7dd38cba22 100644 --- a/lib/AST/Type.cpp +++ b/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(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 typeArgs, ArrayRef 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: diff --git a/lib/AST/TypePrinter.cpp b/lib/AST/TypePrinter.cpp index b4810d63c8..e360512462 100644 --- a/lib/AST/TypePrinter.cpp +++ b/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 || diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp index 18e766dba5..87165fe13a 100644 --- a/lib/CodeGen/CGBlocks.cpp +++ b/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()) { diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 61e86a03fc..f28918fb09 100644 --- a/lib/Sema/SemaType.cpp +++ b/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); diff --git a/test/CodeGenObjC/mrc-weak.m b/test/CodeGenObjC/mrc-weak.m index 9072b1a083..e2c78f0733 100644 --- a/test/CodeGenObjC/mrc-weak.m +++ b/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() {} diff --git a/test/CodeGenObjCXX/mrc-weak.mm b/test/CodeGenObjCXX/mrc-weak.mm new file mode 100644 index 0000000000..17ceb31231 --- /dev/null +++ b/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 void test11(void) { + typedef __unsafe_unretained Foo *UnsafeFooPtr; + UnsafeFooPtr p = get_object(); + use_block(^{ [p run]; }); +} +template void test11<0>(); +void test11_fin() {} diff --git a/test/SemaObjC/mrc-weak.m b/test/SemaObjC/mrc-weak.m index ec03cf7f00..e961e0ab75 100644 --- a/test/SemaObjC/mrc-weak.m +++ b/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}} }