[Experiment] [Objective-C interoperability] Eliminate import of NSObjectProtocol. #14654

DougGregor · 2018-02-15T05:30:39Z

Stop importing the Objective-C protocol NSObject into Swift
programs, because it causes more harm than good. Specifically, with
this change:

The NSObjectProtocol protocol itself is not visible to Swift
programs.
NSObjectProtocol is dropped from the inheritance list of any
imported Objective-C protocol, e.g., UITableViewDelegate will not
mention NSObjectProtocol in its inherited protocols.
NSObjectProtocol is removed from the list of protocols to which an
imported Objective-C class conforms (e.g., the imported class
NSObject will no longer state its conformance to
NSObjectProtocol).
Qualified types in Objective-C will drop the NSObject protocol on
import, e.g., an Objective-C type id<NSCopying, NSObject> will be
imported as NSCopying rather than NSCopying & NSObjectProtocol. The type id<NSObject> will simply be imported
as AnyObject.
Introduce a deprecated type alias into the ObjectiveC module overlay
to ease transition:
```
public typealias NSObjectProtocol = AnyObject
```

Stop importing the Objective-C protocol `NSObject` into Swift programs, because it causes more harm than good. Specifically, with this change: * The `NSObjectProtocol` protocol itself is not visible to Swift programs. * `NSObjectProtocol` is dropped from the inheritance list of any imported Objective-C protocol, e.g., `UITableViewDelegate` will not mention `NSObjectProtocol` in its inherited protocols. * `NSObjectProtocol` is removed from the list of protocols to which an imported Objective-C class conforms (e.g., the imported class `NSObject` will no longer state its conformance to `NSObjectProtocol`). * Qualified types in Objective-C will drop the `NSObject` protocol on import, e.g., an Objective-C type `id<NSCopying, NSObject>` will be imported as `NSCopying` rather than `NSCopying & NSObjectProtocol`. The type `id<NSObject>` will simply be imported as `AnyObject`. * Introduce a deprecated type alias into the ObjectiveC module overlay to ease transition: ```swift public typealias NSObjectProtocol = AnyObject ```

DougGregor · 2018-02-15T05:30:54Z

@swift-ci please test

DougGregor · 2018-02-15T05:31:00Z

@swift-ci please test source compatibility

We were emitting an error on the "inherit from NSObjectProtocol" example when the Swift version was > 3; allow the specific use of the NSObjectProtocol deprecated typealias here. Thanks to Charlie Monroe for noticing the issue here!

DougGregor · 2018-02-15T06:27:51Z

@swift-ci please test

DougGregor · 2018-02-15T06:28:08Z

@swift-ci please test

swift-ci · 2018-02-15T08:02:28Z

Build failed
Swift Test OS X Platform
Git Sha - 8ec1b43

jrose-apple

Code mostly looks good. I'll go comment on the forums about the semantic changes.

jrose-apple · 2018-02-15T16:25:23Z

lib/ClangImporter/ImportDecl.cpp


+    // For NSObject, add the otherwise-hidden NSObject protocol. This is only
+    // used to import mirrored members.
+    // FIXME: Check for any class that conforms to the NSObject protocol?


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.

The principled thing would be to check whether it's a root class that adopts the protocol in Objective-C.

jrose-apple · 2018-02-15T16:26:52Z

lib/ClangImporter/ImportType.cpp

        importedType = ProtocolCompositionType::get(Impl.SwiftContext,
                                                    members,
-                                                    /*HasExplicitAnyObject=*/false);
+                                                    /*HasExplicitAnyObject=*/hasNSObjectProtocol);


This doesn't seem so important when members is known to contain at least one @objc protocol. Or does this handle the 0 case?

It's only really relevant for the 0 case; I can gate it on that.

FWIW, this is the part that forces us to import such things as AnyObject rather than Any; @jckarter was arguing that we should import as Any.

jrose-apple · 2018-02-15T16:28:36Z

test/ClangImporter/objc_parse.swift

-  @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 {


I think it'd be more interesting if this didn't subclass anything, but it probably doesn't really matter at this point.

jrose-apple · 2018-02-15T16:29:26Z

test/IDE/dump_swift_lookup_tables_objc.swift

 // CHECK:   NSObject:
 // CHECK-NEXT:     TU: NSObject
-// CHECK-NEXT:   NSObjectProtocol:
+// CHECK:   __NSObjectProtocol:


Where is the extra "Protocol" coming from? Does that kick in before the SwiftPrivate part?

jrose-apple · 2018-02-15T16:30:09Z

test/PrintAsObjC/classes.swift

 // CHECK-NEXT: - (nonnull instancetype)init OBJC_DESIGNATED_INITIALIZER;
 // CHECK-NEXT: @end
-@objc class ClassWithNSObjectProtocol : NSObjectProtocol {
+@objc class ClassWithNSObjectProtocol {


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.)

jrose-apple · 2018-02-15T16:31:04Z

test/PrintAsObjC/classes.swift

+// CHECK-LABEL: @protocol MyProtocol
 // CHECK-NEXT: @end
-@objc protocol MyProtocol : NSObjectProtocol {}
+@objc protocol MyProtocol {}


Similarly here, since we're testing printing.

jrose-apple · 2018-02-15T16:31:12Z

test/PrintAsObjC/classes.swift

  @IBOutlet var outletCollection: [Properties]!
  @IBOutlet var outletCollectionOptional: [ClassWithCustomName]? = []
  @IBOutlet var outletCollectionAnyObject: [AnyObject]?
-  @IBOutlet var outletCollectionProto: [NSObjectProtocol]?


shahmishal · 2020-10-01T06:58:13Z

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

How to change the base branch: (Link)
More detail about the branch update: (Link)

DougGregor requested a review from jrose-apple February 15, 2018 05:30

DougGregor changed the title ~~[Objective-C interoperability] Eliminate import of NSObjectProtocol.~~ [Experiment] [Objective-C interoperability] Eliminate import of NSObjectProtocol. Feb 15, 2018

DougGregor added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Feb 15, 2018

jrose-apple reviewed Feb 15, 2018

View reviewed changes

shahmishal closed this Oct 5, 2020

AnthonyLatsis removed the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Apr 18, 2023

Uh oh!

[Experiment] [Objective-C interoperability] Eliminate import of NSObjectProtocol. #14654

[Experiment] [Objective-C interoperability] Eliminate import of NSObjectProtocol. #14654

Conversation

DougGregor commented Feb 15, 2018

Uh oh!

DougGregor commented Feb 15, 2018

Uh oh!

DougGregor commented Feb 15, 2018

Uh oh!

DougGregor commented Feb 15, 2018

Uh oh!

DougGregor commented Feb 15, 2018

Uh oh!

swift-ci commented Feb 15, 2018

Uh oh!

jrose-apple left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shahmishal commented Oct 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants