Skip to content

Commit

Permalink
Don't treat Swift methods named "init" as ObjC ARC init methods. (swi…
Browse files Browse the repository at this point in the history
…ftlang#2989)

Under ARC, methods in the "init" family are considered to have
NS_REPLACES_RECEIVER semantics ("consumes" self and returning a
value at +1). This is correct for Objective-C "init methods",
which are equivalent for Swift's initializers, but almost never
correct for any other methods that happen to start with the word
"init".

Note that Swift still follows all the other ARC conventions, so
if you name a method or property, say, "newItemController", the
value will be returned at +1. For methods this is probably
desirable, but for properties maybe not. We could do something
similar for property accessors to make sure they always have
the default "no method family" semantics in Objective-C.

rdar://problem/25759260
  • Loading branch information
jrose-apple authored Jun 20, 2016
1 parent 9ef626f commit e7fe0ab
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 11 deletions.
21 changes: 21 additions & 0 deletions lib/PrintAsObjC/PrintAsObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ static StringRef getNameForObjC(const ValueDecl *VD,
return VD->getName().str();
}

/// Returns true if the given selector might be mistaken for an init method
/// by Objective-C ARC.
static bool isMistakableForInit(ObjCSelector selector) {
ArrayRef<Identifier> selectorPieces = selector.getSelectorPieces();
assert(!selectorPieces.empty());
return selectorPieces.front().str().startswith("init");
}


namespace {
class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
Expand Down Expand Up @@ -474,6 +482,8 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
!isa<ProtocolDecl>(ctor->getDeclContext())) {
os << " OBJC_DESIGNATED_INITIALIZER";
}
} else if (isMistakableForInit(AFD->getObjCSelector())) {
os << " SWIFT_METHOD_FAMILY(none)";
}

os << ";\n";
Expand Down Expand Up @@ -695,6 +705,10 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
}
} else {
os << "\n";
if (isMistakableForInit(VD->getObjCGetterSelector()))
printAbstractFunctionAsMethod(VD->getGetter(), false);
if (isSettable && isMistakableForInit(VD->getObjCSetterSelector()))
printAbstractFunctionAsMethod(VD->getSetter(), false);
}
}

Expand Down Expand Up @@ -1816,6 +1830,13 @@ class ModuleWriter {
"#else\n"
"# define SWIFT_COMPILE_NAME(X)\n"
"#endif\n"
"#if defined(__has_attribute) && "
"__has_attribute(objc_method_family)\n"
"# define SWIFT_METHOD_FAMILY(X) "
"__attribute__((objc_method_family(X)))\n"
"#else\n"
"# define SWIFT_METHOD_FAMILY(X)\n"
"#endif\n"
"#if !defined(SWIFT_CLASS_EXTRA)\n"
"# define SWIFT_CLASS_EXTRA\n"
"#endif\n"
Expand Down
16 changes: 12 additions & 4 deletions lib/SIL/SILFunctionType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,10 @@ FOREACH_FAMILY(GET_LABEL)
}

/// Derive the ObjC selector family from an identifier.
///
/// Note that this will never derive the Init family, which is too dangerous
/// to leave to chance. Swift functions starting with "init" are always
/// emitted as if they are part of the "none" family.
static SelectorFamily getSelectorFamily(Identifier name) {
StringRef text = name.get();
while (!text.empty() && text[0] == '_') text = text.substr(1);
Expand All @@ -1396,12 +1400,16 @@ static SelectorFamily getSelectorFamily(Identifier name) {
return !islower(text[prefix.size()]);
};

#define CHECK_PREFIX(LABEL, PREFIX) \
if (hasPrefix(text, PREFIX)) return SelectorFamily::LABEL;
auto result = SelectorFamily::None;
if (false) /*for #define purposes*/;
#define CHECK_PREFIX(LABEL, PREFIX) \
else if (hasPrefix(text, PREFIX)) result = SelectorFamily::LABEL;
FOREACH_FAMILY(CHECK_PREFIX)
#undef CHECK_PREFIX
#undef CHECK_PREFIX

return SelectorFamily::None;
if (result == SelectorFamily::Init)
return SelectorFamily::None;
return result;
}

/// Get the ObjC selector family a SILDeclRef implicitly belongs to.
Expand Down
4 changes: 4 additions & 0 deletions test/IRGen/Inputs/usr/include/Gizmo.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ typedef long NSInteger;
+ (void) runce;
@end

@interface BaseClassForMethodFamilies : NSObject
- (BaseClassForMethodFamilies *)fakeInitFamily __attribute__((objc_method_family(init)));
@end

static inline int innerZero(void) { return 0; }
static inline int zero(void) { return innerZero(); }
static inline int wrappedZero(void) { return zero(); }
Expand Down
17 changes: 10 additions & 7 deletions test/IRGen/partial_apply.sil
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import Swift
import Foundation
import gizmo

@objc class ObjCClass {
func method(x x: Int) {}
@objc class ObjCClass : BaseClassForMethodFamilies {
func method(x: Int) {}

func initFamily() -> ObjCClass { return self }
override func fakeInitFamily() -> ObjCClass { return self }
}
sil_vtable ObjCClass {}
sil @_TFC13partial_apply9ObjCClassD : $@convention(method) (ObjCClass) -> ()
Expand All @@ -37,7 +37,7 @@ bb0(%0 : $Int, %1 : $ObjCClass):
return %v : $()
}

sil @_TToFC13partial_apply9ObjCClass10initFamilyfT_S0_ : $@convention(objc_method) (@owned ObjCClass) -> @owned ObjCClass {
sil @_TToFC13partial_apply9ObjCClass14fakeInitFamilyfT_S0_ : $@convention(objc_method) (@owned ObjCClass) -> @owned ObjCClass {
bb0(%0 : $ObjCClass):
return %0 : $ObjCClass
}
Expand Down Expand Up @@ -114,16 +114,19 @@ entry(%c : $ObjCClass):
// CHECK: [[DATA_ADDR:%.*]] = bitcast %swift.refcounted* %0 to [[DATA_TYPE]]*
// CHECK: [[X_ADDR:%.*]] = getelementptr inbounds [[DATA_TYPE]], [[DATA_TYPE]]* [[DATA_ADDR]], i32 0, i32 1
// CHECK: [[SELF:%.*]] = load %C13partial_apply9ObjCClass*, %C13partial_apply9ObjCClass** [[X_ADDR]], align 8
// CHECK: call void @rt_swift_retain(%swift.refcounted* %4)
// CHECK: [[CMD:%.*]] = load i8*, i8** @"\01L_selector(initFamily)", align 8
// CHECK: call {{.*}}@objc_retain{{.*}}(%{{.*}}* [[SELF]])
// CHECK: [[CMD:%.*]] = load i8*, i8** @"\01L_selector(fakeInitFamily)", align 8
// CHECK: [[I8PTRSELF:%.*]] = bitcast %C13partial_apply9ObjCClass* [[SELF]] to [[OPAQUE4:%.*]]*
// CHECK: call [[OPAQUE3:%.*]]* bitcast (void ()* @objc_msgSend to [[OPAQUE3]]* ([[OPAQUE4:%.*]]*, i8*)*)([[OPAQUE4]]* [[I8PTRSELF]], i8* [[CMD]])
// CHECK-NOT: release
// CHECK: call void @rt_swift_release(%swift.refcounted* %0)
// CHECK-NOT: release
// CHECK: ret void
// CHECK: }

sil @objc_partial_apply_consumes_self : $@convention(thin) ObjCClass -> @callee_owned () -> @owned ObjCClass {
entry(%c : $ObjCClass):
%m = class_method [volatile] %c : $ObjCClass, #ObjCClass.initFamily!1.foreign : (ObjCClass) -> () -> ObjCClass , $@convention(objc_method) (@owned ObjCClass) -> @owned ObjCClass
%m = class_method [volatile] %c : $ObjCClass, #ObjCClass.fakeInitFamily!1.foreign : (ObjCClass) -> () -> ObjCClass , $@convention(objc_method) (@owned ObjCClass) -> @owned ObjCClass
%p = partial_apply %m(%c) : $@convention(objc_method) (@owned ObjCClass) -> @owned ObjCClass
return %p : $@callee_owned () -> @owned ObjCClass
}
Expand Down
15 changes: 15 additions & 0 deletions test/PrintAsObjC/Inputs/arc-conventions.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
@import Foundation;
#import "swift.h"

int main() {
@autoreleasepool {
Test *test = [[Test alloc] init];
id result = [test initAllTheThings]; // CHECK: method called
NSCAssert(result != nil, @"failed to get a return value back");
result = [test initAllTheThings]; // CHECK: method called
NSCAssert(result != nil, @"failed to get a return value back");
#if !__has_feature(objc_arc)
[test release];
#endif
} // CHECK: deinitialized
}
26 changes: 26 additions & 0 deletions test/PrintAsObjC/arc-conventions.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// RUN: rm -rf %t && mkdir %t
// RUN: %target-build-swift -c %s -parse-as-library -force-single-frontend-invocation -o %t/swift.o -emit-objc-header-path %t/swift.h

// RUN: %target-clang -c -Weverything -Werror -Wno-unused-macros -Wno-incomplete-module -fobjc-arc -fmodules %S/Inputs/arc-conventions.m -o %t/main.o -I %t
// RUN: %target-build-swift %t/swift.o %t/main.o -o %t/main
// RUN: %target-run %t/main | FileCheck %S/Inputs/arc-conventions.m

// RUN: %target-clang -c -Weverything -Werror -Wno-unused-macros -Wno-incomplete-module -fno-objc-arc -fmodules %S/Inputs/arc-conventions.m -o %t/main.o -I %t
// RUN: %target-build-swift %t/swift.o %t/main.o -o %t/main
// RUN: %target-run %t/main | FileCheck %S/Inputs/arc-conventions.m

// REQUIRES: executable_test
// REQUIRES: objc_interop

import Foundation

public class Test: NSObject {
public func initAllTheThings() -> AnyObject {
print("method called")
return "initialized"
}

deinit {
print("deinitialized \(self)")
}
}
24 changes: 24 additions & 0 deletions test/PrintAsObjC/classes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ class NotObjC {}
// CHECK-NEXT: - (void)methodWithReservedParameterNames:(id _Nonnull)long_ protected:(id _Nonnull)protected_;
// CHECK-NEXT: - (void)honorRenames:(CustomName * _Nonnull)_;
// CHECK-NEXT: - (Methods * _Nullable __unsafe_unretained)unmanaged:(id _Nonnull __unsafe_unretained)_;
// CHECK-NEXT: - (void)initAllTheThings SWIFT_METHOD_FAMILY(none);
// CHECK-NEXT: - (void)initTheOtherThings SWIFT_METHOD_FAMILY(none);
// CHECK-NEXT: init
// CHECK-NEXT: @end
@objc class Methods {
Expand Down Expand Up @@ -215,6 +217,9 @@ class NotObjC {}
func honorRenames(_: ClassWithCustomName) {}

func unmanaged(_: Unmanaged<AnyObject>) -> Unmanaged<Methods>? { return nil }

func initAllTheThings() {}
@objc(initTheOtherThings) func setUpOtherThings() {}
}

typealias AliasForNSRect = NSRect
Expand Down Expand Up @@ -422,6 +427,14 @@ public class NonObjCClass { }
// CHECK-NEXT: + (BOOL)customGetterNameForPrivateSetter;
// CHECK-NEXT: SWIFT_CLASS_PROPERTY(@property (nonatomic, class, readonly) NSInteger sharedConstant;)
// CHECK-NEXT: + (NSInteger)sharedConstant;
// CHECK-NEXT: @property (nonatomic) NSInteger initContext;
// CHECK-NEXT: - (NSInteger)initContext SWIFT_METHOD_FAMILY(none);
// CHECK-NEXT: @property (nonatomic, readonly) NSInteger initContextRO;
// CHECK-NEXT: - (NSInteger)initContextRO SWIFT_METHOD_FAMILY(none);
// CHECK-NEXT: @property (nonatomic, getter=initGetter) BOOL getterIsInit;
// CHECK-NEXT: - (BOOL)initGetter SWIFT_METHOD_FAMILY(none);
// CHECK-NEXT: @property (nonatomic, setter=initSetter:) BOOL setterIsInit;
// CHECK-NEXT: - (void)initSetter:(BOOL)newValue SWIFT_METHOD_FAMILY(none);
// CHECK-NEXT: init
// CHECK-NEXT: @end
@objc class Properties {
Expand Down Expand Up @@ -515,6 +528,17 @@ public class NonObjCClass { }
@objc(customSetterNameForPrivateSetter:) set {}
}
static let sharedConstant = 2

var initContext = 4
var initContextRO: Int { return 4 }
var getterIsInit: Bool {
@objc(initGetter) get { return true }
set {}
}
var setterIsInit: Bool {
get { return true }
@objc(initSetter:) set {}
}
}

// CHECK-LABEL: @interface PropertiesOverridden
Expand Down
64 changes: 64 additions & 0 deletions test/SILGen/objc_thunks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@ class Hoozit : Gizmo {
// CHECK-NEXT: return [[RES]]
// CHECK-NEXT: }

// Override the normal family conventions to make this non-consuming and
// returning at +0.
func initFoo() -> Gizmo { return self }
// CHECK-LABEL: sil hidden [thunk] @_TToFC11objc_thunks6Hoozit7initFoo{{.*}} : $@convention(objc_method) (Hoozit) -> @autoreleased Gizmo
// CHECK: bb0([[THIS:%.*]] : $Hoozit):
// CHECK-NEXT: retain [[THIS]]
// CHECK-NEXT: // function_ref
// CHECK-NEXT: [[NATIVE:%.*]] = function_ref @_TFC11objc_thunks6Hoozit7initFoo{{.*}} : $@convention(method) (@guaranteed Hoozit) -> @owned Gizmo
// CHECK-NEXT: [[RES:%.*]] = apply [[NATIVE]]([[THIS]])
// CHECK-NEXT: release [[THIS]]
// CHECK-NEXT: return [[RES]]
// CHECK-NEXT: }

var typicalProperty: Gizmo
// -- getter
// CHECK-LABEL: sil hidden [transparent] [thunk] @_TToFC11objc_thunks6Hoozitg15typicalPropertyCSo5Gizmo : $@convention(objc_method) (Hoozit) -> @autoreleased Gizmo {
Expand Down Expand Up @@ -188,6 +201,57 @@ class Hoozit : Gizmo {
// CHECK-NEXT: return
// CHECK-NEXT: }

var initProperty: Gizmo
// -- getter
// CHECK-LABEL: sil hidden [transparent] [thunk] @_TToFC11objc_thunks6Hoozitg12initPropertyCSo5Gizmo : $@convention(objc_method) (Hoozit) -> @autoreleased Gizmo {
// CHECK: bb0([[THIS:%.*]] : $Hoozit):
// CHECK-NEXT: retain [[THIS]]
// CHECK-NEXT: // function_ref
// CHECK-NEXT: [[NATIVE:%.*]] = function_ref @_TFC11objc_thunks6Hoozitg12initPropertyCSo5Gizmo : $@convention(method) (@guaranteed Hoozit) -> @owned Gizmo
// CHECK-NEXT: [[RES:%.*]] = apply [[NATIVE]]([[THIS]])
// CHECK-NEXT: release [[THIS]]
// CHECK-NEXT: return [[RES]]
// CHECK-NEXT: }

// -- setter
// CHECK-LABEL: sil hidden [transparent] [thunk] @_TToFC11objc_thunks6Hoozits12initPropertyCSo5Gizmo : $@convention(objc_method) (Gizmo, Hoozit) -> () {
// CHECK: bb0([[VALUE:%.*]] : $Gizmo, [[THIS:%.*]] : $Hoozit):
// CHECK-NEXT: retain [[VALUE]]
// CHECK-NEXT: retain [[THIS]]
// CHECK-NEXT: // function_ref
// CHECK-NEXT: [[NATIVE:%.*]] = function_ref @_TFC11objc_thunks6Hoozits12initPropertyCSo5Gizmo : $@convention(method) (@owned Gizmo, @guaranteed Hoozit) -> ()
// CHECK-NEXT: apply [[NATIVE]]([[VALUE]], [[THIS]])
// CHECK-NEXT: release [[THIS]]
// CHECK-NEXT: return
// CHECK-NEXT: }

var propComputed: Gizmo {
@objc(initPropComputedGetter) get { return self }
@objc(initPropComputedSetter:) set {}
}
// -- getter
// CHECK-LABEL: sil hidden [thunk] @_TToFC11objc_thunks6Hoozitg12propComputedCSo5Gizmo : $@convention(objc_method) (Hoozit) -> @autoreleased Gizmo {
// CHECK: bb0([[THIS:%.*]] : $Hoozit):
// CHECK-NEXT: retain [[THIS]]
// CHECK-NEXT: // function_ref
// CHECK-NEXT: [[NATIVE:%.*]] = function_ref @_TFC11objc_thunks6Hoozitg12propComputedCSo5Gizmo : $@convention(method) (@guaranteed Hoozit) -> @owned Gizmo
// CHECK-NEXT: [[RES:%.*]] = apply [[NATIVE]]([[THIS]])
// CHECK-NEXT: release [[THIS]]
// CHECK-NEXT: return [[RES]]
// CHECK-NEXT: }

// -- setter
// CHECK-LABEL: sil hidden [thunk] @_TToFC11objc_thunks6Hoozits12propComputedCSo5Gizmo : $@convention(objc_method) (Gizmo, Hoozit) -> () {
// CHECK: bb0([[VALUE:%.*]] : $Gizmo, [[THIS:%.*]] : $Hoozit):
// CHECK-NEXT: retain [[VALUE]]
// CHECK-NEXT: retain [[THIS]]
// CHECK-NEXT: // function_ref
// CHECK-NEXT: [[NATIVE:%.*]] = function_ref @_TFC11objc_thunks6Hoozits12propComputedCSo5Gizmo : $@convention(method) (@owned Gizmo, @guaranteed Hoozit) -> ()
// CHECK-NEXT: apply [[NATIVE]]([[VALUE]], [[THIS]])
// CHECK-NEXT: release [[THIS]]
// CHECK-NEXT: return
// CHECK-NEXT: }

// Don't export generics to ObjC yet
func generic<T>(_ x: T) {}
// CHECK-NOT: sil hidden [thunk] @_TToFC11objc_thunks6Hoozit7generic{{.*}}
Expand Down

0 comments on commit e7fe0ab

Please sign in to comment.