Skip to content

[Verifier] ObjC protocols may have unavailable requirements #6862

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

Conversation

jrose-apple
Copy link
Contributor

...which don't need to have witnesses.

I can't seem to come up with a reduced test case for this one, but it does fix a verifier failure in the larger project that triggered this, where it complained about NSCoder's encodeWithCoder(_:) method (which was renamed to encode(with:) in Swift 3).

rdar://problem/29744313

...which don't need to have witnesses.

I can't seem to come up with a reduced test case for this one, but it
does fix a verifier failure in the larger project that triggered this.

rdar://problem/29744313
@jrose-apple
Copy link
Contributor Author

@slavapestov, @DougGregor, any ideas how to test this? Or any ideas why this reduced case passed the verifier?

import Foundation

open class Test : NSObject, NSCoding {
  public override init() { fatalError() }
  public required init?(coder: NSCoder) { fatalError() }
  open func encode(with coder: NSCoder) { fatalError() }
}
import Lib
import Foundation

final class Sub: Test {}

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor Author

I guess that's also a review request, since we'll want to cherry-pick this to the 3.1 branch.

@slavapestov
Copy link
Contributor

Do non-objc protocols ever have unavailable requirements?

@jrose-apple
Copy link
Contributor Author

I imagine we'll start wanting them for renaming purposes, but in that case I think we probably still require a witness; it'll just be a forwarding witness in an unconstrained protocol extension.

@jrose-apple
Copy link
Contributor Author

…but currently they do not.

@jrose-apple
Copy link
Contributor Author

Known failure in Foundation (dependent commits merged out of order). Will restart testing later.

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@slavapestov
Copy link
Contributor

Oh, LGTM too

@jrose-apple jrose-apple merged commit 22b49d2 into swiftlang:master Jan 18, 2017
@jrose-apple jrose-apple deleted the objc-protocols-unavailable-requirements branch January 18, 2017 00:45
jrose-apple added a commit to jrose-apple/swift that referenced this pull request Jan 18, 2017
…ng#6862)

...which don't need to have witnesses.

I can't seem to come up with a reduced test case for this one, but it
does fix a verifier failure in the larger project that triggered this.

rdar://problem/29744313
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants