Skip to content
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

[SR-7182] Ownership keyword removal from protocols breaks ObjC implementations #49730

Closed
MikeWeller opened this issue Mar 13, 2018 · 5 comments
Closed
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression swift 4.1

Comments

@MikeWeller
Copy link

MikeWeller commented Mar 13, 2018

Previous ID SR-7182
Radar rdar://problem/38418112
Original Reporter @MikeWeller
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, 4.1Regression
Assignee @DougGregor
Priority Medium

md5: 8ce845df43dbbfb365a932dd18c900d2

Issue Description:

(Reporting this as requested by Jordan Rose in https://forums.swift.org/t/ownership-keyword-removal-in-protocols-source-breaking-for-objc-implementations/10743)

SE-0176-0186: Remove ownership keyword support in protocols” (Pull Request, Proposal2, Discussion) made it a warning to specify weak on Swift properties (vars) in protocols.

In other words this now generates a warning:

protocol MyProtocol {  
    // 'weak' should not be applied to a property declaration in a protocol
    // and will be disallowed in future versions  
    weak var delegate: MyDelegate?  
} 

Instead, weak can be omitted from the protocol declaration, and Swift implementions of this protocol can redeclare the property as weak themselves:

class Impl: MyProtocol {  
    weak var delegate: MyDelegate?  
}

However, this is a problem for ObjC implementations (as of the Xcode 9.3 betas which brought in this warning) since the compiler complains if you redeclare the property and change the strong/weakness:

#import <MyFramework-Swift.h>  
      
@interface ImplObjC : NSObject <MyProtocol>  
      
// warning: 'retain (or strong)' attribute on property 'delegate' does
// not match the property inherited from 'MyProtocol'
// [-Wproperty-attribute-mismatch]  
@property (nonatomic, weak, nullable) id<MyDelegate> delegate;  
      
@end  

Alternatively in a private category:

@interface ImplObjC ()        

// error: illegal redeclaration of property in class extension 'ImplObjC'
// (attribute must be 'readwrite', while its primary must be 'readonly')  
@property (nonatomic, weak, nullable) id<MyDelegate> delegate;  
      
@end  

Alternatively synthesizing into a different ivar with __weak:

@interface ImplObjC ()  
{  
    // error: existing instance variable 'm_delegate' for strong property
    // 'delegate' may not be __weak  
    __weak id<MyDelegate> m_delegate;  
}  
@end  
      
@implementation ImplObjC  
      
@synthesize delegate = m_delegate;  
      
@end  

The only alternative that works is to override the setter and getter for the property and back it with a weak ivar/property. This is ugly and tedious.

Note that even when leaving weak on the Swift protocol property and ignoring the warning, the generated "-Swift.h" header still declares the property as strong:

@property (nonatomic, strong) id <MyDelegate> _Nullable delegate;

We build with “warnings as errors” for ObjC, so this is a source breaking change for our existing code. It doesn’t seem like it would be a good idea to ignore the Wproperty-attribute-mismatch warning. This is a problem for any existing ObjC code that implements Swift protocols like this.

Now, there are legitimate design discussions to be had over whether a delegate property belongs on an abstract protocol, but given how common the (weak) delegate pattern is in ObjC I believe there will be a significant amount of this kind of code in the wild (we ran into this at least).

@belkadan
Copy link
Contributor

belkadan commented Mar 13, 2018

@swift-ci create

@belkadan
Copy link
Contributor

belkadan commented Mar 13, 2018

Maybe the answer is to continue allowing the keywords without warning in @objc protocols.

@slavapestov
Copy link
Member

slavapestov commented Mar 15, 2018

@belkadan Would it also make sense to "implement" this change on the Clang side and ignore (or warn) on the keywords? After all, they don't mean anything in Objective-C either.

@belkadan
Copy link
Contributor

belkadan commented Mar 15, 2018

I can't remember if you can use @synthesize on something only declared in a protocol. If you can, then it does have meaning there as well.

@DougGregor
Copy link
Member

DougGregor commented Mar 15, 2018

#15274

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression swift 4.1
Projects
None yet
Development

No branches or pull requests

5 participants