-
Couldn't load subscription status.
- Fork 10.6k
[Experiment] [Objective-C interoperability] Eliminate import of NSObjectProtocol. #14654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -926,17 +926,6 @@ namespace { | |
| SmallVector<clang::ObjCProtocolDecl *, 4> protocols{ | ||
| type->qual_begin(), type->qual_end() | ||
| }; | ||
| auto *nsObjectProto = | ||
| Impl.getNSObjectProtocolType()->getAnyNominal(); | ||
| if (!nsObjectProto) { | ||
| // Input is malformed | ||
| return {}; | ||
| } | ||
| auto *clangProto = | ||
| cast<clang::ObjCProtocolDecl>(nsObjectProto->getClangDecl()); | ||
| protocols.push_back( | ||
| const_cast<clang::ObjCProtocolDecl *>(clangProto)); | ||
|
|
||
| clang::ASTContext &clangCtx = Impl.getClangASTContext(); | ||
| clang::QualType protosOnlyType = | ||
| clangCtx.getObjCObjectType(clangCtx.ObjCBuiltinIdTy, | ||
|
|
@@ -1037,8 +1026,15 @@ namespace { | |
| if (!importedType->isAnyObject()) | ||
| members.push_back(importedType); | ||
|
|
||
| bool hasNSObjectProtocol = false; | ||
| for (auto cp = type->qual_begin(), cpEnd = type->qual_end(); | ||
| cp != cpEnd; ++cp) { | ||
| // Never import the NSObject protocol in a type. | ||
| if (isNSObjectProtocol(*cp)) { | ||
| hasNSObjectProtocol = true; | ||
| continue; | ||
| } | ||
|
|
||
| auto proto = castIgnoringCompatibilityAlias<ProtocolDecl>( | ||
| Impl.importDecl(*cp, Impl.CurrentVersion)); | ||
| if (!proto) | ||
|
|
@@ -1049,7 +1045,7 @@ namespace { | |
|
|
||
| importedType = ProtocolCompositionType::get(Impl.SwiftContext, | ||
| members, | ||
| /*HasExplicitAnyObject=*/false); | ||
| /*HasExplicitAnyObject=*/hasNSObjectProtocol); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem so important when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's only really relevant for the 0 case; I can gate it on that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, this is the part that forces us to import such things as |
||
| } | ||
|
|
||
| // Class or Class<P> maps to an existential metatype. | ||
|
|
@@ -2473,3 +2469,10 @@ Type ClangImporter::Implementation::getNSCopyingType() { | |
| Type ClangImporter::Implementation::getNSObjectProtocolType() { | ||
| return getNamedProtocolType(*this, "NSObject"); | ||
| } | ||
|
|
||
| bool swift::importer::isNSObjectProtocol(const clang::Decl *decl) { | ||
| if (auto objcProto = dyn_cast_or_null<clang::ObjCProtocolDecl>(decl)) | ||
| return objcProto->getName() == "NSObject"; | ||
| return false; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,7 +57,7 @@ func instanceMethods(_ b: B) { | |
|
|
||
| // Instance methods with keyword components | ||
| var obj = NSObject() | ||
| var prot = NSObjectProtocol.self | ||
| var prot = NSCoding.self | ||
| b.`protocol`(prot, hasThing:obj) | ||
| b.doThing(obj, protocol: prot) | ||
| } | ||
|
|
@@ -311,10 +311,10 @@ func ivars(_ hive: Hive) { | |
| hive.queen.description() // expected-error{{value of type 'Hive' has no member 'queen'}} | ||
| } | ||
|
|
||
| class NSObjectable : NSObjectProtocol { | ||
| @objc var description : String { return "" } | ||
| @objc(conformsToProtocol:) func conforms(to _: Protocol) -> Bool { return false } | ||
| @objc(isKindOfClass:) func isKind(of aClass: AnyClass) -> Bool { return false } | ||
| class NSObjectable : NSObject { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'd be more interesting if this didn't subclass anything, but it probably doesn't really matter at this point. |
||
| @objc override var description : String { return "" } | ||
| @objc(conformsToProtocol:) override func conforms(to _: Protocol) -> Bool { return false } | ||
| @objc(isKindOfClass:) override func isKind(of aClass: AnyClass) -> Bool { return false } | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -553,17 +553,15 @@ func testStrangeSelectors(obj: StrangeSelectors) { | |
|
|
||
| func testProtocolQualified(_ obj: CopyableNSObject, cell: CopyableSomeCell, | ||
| plainObj: NSObject, plainCell: SomeCell) { | ||
| _ = obj as NSObject // expected-error {{'CopyableNSObject' (aka 'NSCopying & NSObjectProtocol') is not convertible to 'NSObject'; did you mean to use 'as!' to force downcast?}} {{11-13=as!}} | ||
| _ = obj as NSObjectProtocol | ||
| _ = obj as NSObject // expected-error {{'CopyableNSObject' (aka 'NSCopying') is not convertible to 'NSObject'; did you mean to use 'as!' to force downcast?}} {{11-13=as!}} | ||
| _ = obj as NSCopying | ||
| _ = obj as SomeCell // expected-error {{'CopyableNSObject' (aka 'NSCopying & NSObjectProtocol') is not convertible to 'SomeCell'; did you mean to use 'as!' to force downcast?}} {{11-13=as!}} | ||
| _ = obj as SomeCell // expected-error {{'CopyableNSObject' (aka 'NSCopying') is not convertible to 'SomeCell'; did you mean to use 'as!' to force downcast?}} {{11-13=as!}} | ||
|
|
||
| _ = cell as NSObject | ||
| _ = cell as NSObjectProtocol | ||
| _ = cell as NSCopying // expected-error {{'CopyableSomeCell' (aka 'SomeCell') is not convertible to 'NSCopying'; did you mean to use 'as!' to force downcast?}} {{12-14=as!}} | ||
| _ = cell as SomeCell | ||
|
|
||
| _ = plainObj as CopyableNSObject // expected-error {{'NSObject' is not convertible to 'CopyableNSObject' (aka 'NSCopying & NSObjectProtocol'); did you mean to use 'as!' to force downcast?}} {{16-18=as!}} | ||
| _ = plainObj as CopyableNSObject // expected-error {{'NSObject' is not convertible to 'CopyableNSObject' (aka 'NSCopying'); did you mean to use 'as!' to force downcast?}} {{16-18=as!}} | ||
| _ = plainCell as CopyableSomeCell // FIXME: This is not really typesafe. | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // RUN: %target-typecheck-verify-swift -swift-version 3 | ||
| // RUN: %target-typecheck-verify-swift -swift-version 4 | ||
| // REQUIRES: objc_interop | ||
|
|
||
| import ObjectiveC | ||
| import Foundation | ||
|
|
||
| class X: NSObjectProtocol { } // expected-warning{{'NSObjectProtocol' is deprecated: all classes implicitly conform to the 'NSObject' protocol}} | ||
| // expected-note@-1{{use 'AnyObject' instead}} | ||
| // expected-warning@-2{{conformance of class 'X' to 'AnyObject' is redundant}} | ||
|
|
||
| protocol P: NSObjectProtocol { } // expected-warning{{'NSObjectProtocol' is deprecated: all classes implicitly conform to the 'NSObject' protocol}} | ||
| // expected-note@-1{{use 'AnyObject' instead}} | ||
|
|
||
| func composition(_: NSCoding & NSObjectProtocol) { } // expected-warning{{'NSObjectProtocol' is deprecated: all classes implicitly conform to the 'NSObject' protocol}} | ||
| // expected-note@-1{{use 'AnyObject' instead}} | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,11 +14,12 @@ | |
| // CHECK-NOT: lookup table | ||
| // CHECK: NSObject: | ||
| // CHECK-NEXT: TU: NSObject | ||
| // CHECK-NEXT: NSObjectProtocol: | ||
| // CHECK: __NSObjectProtocol: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is the extra "Protocol" coming from? Does that kick in before the SwiftPrivate part? |
||
| // CHECK-NEXT: TU: NSObject | ||
| // CHECK: responds: | ||
| // CHECK-NEXT: -[NSObject respondsToSelector:] | ||
|
|
||
|
|
||
| // CHECK-LABEL: <<Bridging header lookup table>> | ||
| // CHECK-NEXT: Base name -> entry mappings: | ||
| // CHECK-NEXT: CCItem: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,13 +89,13 @@ class ClassWithCustomName2 {} | |
| class ClassWithCustomNameSub : ClassWithCustomName {} | ||
|
|
||
|
|
||
| // CHECK-LABEL: @interface ClassWithNSObjectProtocol <NSObject> | ||
| // CHECK-LABEL: @interface ClassWithNSObjectProtocol | ||
| // CHECK-NEXT: @property (nonatomic, readonly, copy) NSString * _Nonnull description; | ||
| // CHECK-NEXT: - (BOOL)conformsToProtocol:(Protocol * _Nonnull)_ SWIFT_WARN_UNUSED_RESULT; | ||
| // CHECK-NEXT: - (BOOL)isKindOfClass:(Class _Nonnull)aClass SWIFT_WARN_UNUSED_RESULT; | ||
| // CHECK-NEXT: - (nonnull instancetype)init OBJC_DESIGNATED_INITIALIZER; | ||
| // CHECK-NEXT: @end | ||
| @objc class ClassWithNSObjectProtocol : NSObjectProtocol { | ||
| @objc class ClassWithNSObjectProtocol { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is no longer testing the same thing; please have it conform to something else. (Preferably something where the Objective-C name doesn't match the Swift name.) |
||
| @objc var description: String { return "me" } | ||
| @objc(conformsToProtocol:) | ||
| func conforms(to _: Protocol) -> Bool { return false } | ||
|
|
@@ -355,9 +355,9 @@ typealias AliasForNSRect = NSRect | |
| // NEGATIVE-NOT: @interface NSObject | ||
| class MyObject : NSObject {} | ||
|
|
||
| // CHECK-LABEL: @protocol MyProtocol <NSObject> | ||
| // CHECK-LABEL: @protocol MyProtocol | ||
| // CHECK-NEXT: @end | ||
| @objc protocol MyProtocol : NSObjectProtocol {} | ||
| @objc protocol MyProtocol {} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly here, since we're testing printing. |
||
|
|
||
| // CHECK-LABEL: @protocol MyProtocolMetaOnly; | ||
| // CHECK-LABEL: @interface MyProtocolMetaCheck | ||
|
|
@@ -521,7 +521,6 @@ public class NonObjCClass { } | |
| // CHECK-NEXT: @property (nonatomic, copy) IBOutletCollection(Properties) NSArray<Properties *> * _Null_unspecified outletCollection; | ||
| // CHECK-NEXT: @property (nonatomic, copy) IBOutletCollection(CustomName) NSArray<CustomName *> * _Nullable outletCollectionOptional; | ||
| // CHECK-NEXT: @property (nonatomic, copy) IBOutletCollection(id) NSArray * _Nullable outletCollectionAnyObject; | ||
| // CHECK-NEXT: @property (nonatomic, copy) IBOutletCollection(id) NSArray<id <NSObject>> * _Nullable outletCollectionProto; | ||
| // CHECK-NEXT: SWIFT_CLASS_PROPERTY(@property (nonatomic, class, readonly) NSInteger staticInt;) | ||
| // CHECK-NEXT: + (NSInteger)staticInt SWIFT_WARN_UNUSED_RESULT; | ||
| // CHECK-NEXT: SWIFT_CLASS_PROPERTY(@property (nonatomic, class, copy) NSString * _Nonnull staticString;) | ||
|
|
@@ -614,7 +613,6 @@ public class NonObjCClass { } | |
| @IBOutlet var outletCollection: [Properties]! | ||
| @IBOutlet var outletCollectionOptional: [ClassWithCustomName]? = [] | ||
| @IBOutlet var outletCollectionAnyObject: [AnyObject]? | ||
| @IBOutlet var outletCollectionProto: [NSObjectProtocol]? | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here. |
||
|
|
||
| @objc static let staticInt = 2 | ||
| @objc static var staticString = "Hello" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe NSProxy is the only other one in the Apple SDKs. We are absolutely terrible with it but if you wanted to avoid touching it maybe you could make the condition about root classes instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The principled thing would be to check whether it's a root class that adopts the protocol in Objective-C.