Skip to content

[concurrency] fixes for conformance to async ObjC protocol requirements #35662

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

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

kavon
Copy link
Member

@kavon kavon commented Jan 29, 2021

This PR aims to fix and add more coverage for conformance checking of ObjC protocols that contain a requirement that can be conformed to in different ways by a Swift type: either as the ordinary sync version or an async version.

TODOs:

  • Fix incorrect diagnostic and then crash when typechecking an ObjC protocol with two optional methods that are projected into Swift with the same name, plus a Swift-side type that attempts to conform to that protocol using a sync witness (rdar://73326234).
  • Fix unusual note that claims a protocol's requirements are not satisfied (rdar://73641790). Will fix in a separate PR.
  • Fix a conformance serialization issue that might be related to the unusual note (rdar://73326224). Will fix in a separate PR.

a continuation of this work is in #35719

@kavon kavon force-pushed the objc-async-conformance-bugs branch from 9c21b4a to 884f2b5 Compare January 30, 2021 01:48
@kavon
Copy link
Member Author

kavon commented Jan 30, 2021

@swift-ci please smoke test macOS platform

@kavon kavon force-pushed the objc-async-conformance-bugs branch 3 times, most recently from f14a14d to fb1a268 Compare February 1, 2021 23:11
While a Swift protocol can have its async requirements satisfied
by a sync witness, this is not the case for ObjC protocols because
it is not just a calling convention difference.

Because every async ObjC function requirement in the protocol also
has a sibling that is sync, a sync witness might be trying
to conform to the sync version that takes a completion handler.

Without this change, we were seeing an issue where we would
consider both the async and sync versions of an ObjC requirement,
and accidentially choose the async version when the witness was
sync, and then raise an error about the typechecker's mistake.

Instead of raising an error, this change removes the ability for
the typechecker to even consider the errornous conformance.

resolves rdar://73326234
@kavon kavon force-pushed the objc-async-conformance-bugs branch from fb1a268 to ff54f3b Compare February 2, 2021 01:18
@kavon
Copy link
Member Author

kavon commented Feb 2, 2021

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Feb 2, 2021

Build failed
Swift Test OS X Platform
Git Sha - ff54f3b

@swift-ci
Copy link
Contributor

swift-ci commented Feb 2, 2021

Build failed
Swift Test Linux Platform
Git Sha - ff54f3b

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.

This looks great, thank you!

@DougGregor
Copy link
Member

@swift-ci please smoke test

@kavon kavon marked this pull request as ready for review February 2, 2021 20:20
@kavon kavon merged commit 9964b97 into swiftlang:main Feb 2, 2021
@kavon kavon deleted the objc-async-conformance-bugs branch February 2, 2021 20:21
@swift-ci
Copy link
Contributor

swift-ci commented Feb 2, 2021

Build failed
Swift Test OS X Platform
Git Sha - ff54f3b

@swift-ci
Copy link
Contributor

swift-ci commented Feb 2, 2021

Build failed
Swift Test Linux Platform
Git Sha - ff54f3b

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