Skip to content

Witness selection should pick exact matches for effect overloads #65840

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
May 12, 2023

Conversation

kavon
Copy link
Member

@kavon kavon commented May 10, 2023

You can overload a function based on its async-ness, and resolution is carried out based on async-ness of calling context.

But during protocol conformance checking, for an async requirement we were accidentally choosing the non-async overload instead of the asyncone. Theasyncone is the choice people would expect, since theasync` requirement is in essence the "context" that forwards to the underlying witness. This intended behavior is also inferred from:

#40088

The problem boiled down to a bad check when categorizing the witness matches prior to ranking them.

Resolves rdar://109135488 / #60318

TODO:

  • add tests

@kavon
Copy link
Member Author

kavon commented May 10, 2023

@swift-ci smoke test

@kavon kavon force-pushed the async-requirement-matching branch from 4e2d289 to a2b96cb Compare May 11, 2023 17:42
@kavon kavon marked this pull request as ready for review May 11, 2023 17:43
@kavon
Copy link
Member Author

kavon commented May 11, 2023

@swift-ci please test

@kavon kavon force-pushed the async-requirement-matching branch from a2b96cb to c64b987 Compare May 11, 2023 19:43
@kavon
Copy link
Member Author

kavon commented May 11, 2023

@swift-ci please test

You can overload a function based on its `async`-ness, and
resolution is carried out based on async-ness of calling
context.

But during protocol conformance checking, for an `async`
requirement we were accidentally choosing the non-`async
overload instead of the `async` one. The `async` one is
the choice people would expect, since the `async` requirement
is in essence the "context" that forwards to the underlying
witness. This intended behavior is also inferred from:

swiftlang#40088

The problem boiled down to a bad check when categorizing the
witness matches prior to ranking them.

Resolves rdar://109135488 / swiftlang#60318
@kavon kavon force-pushed the async-requirement-matching branch from c64b987 to 3ae2ce5 Compare May 12, 2023 18:04
@kavon
Copy link
Member Author

kavon commented May 12, 2023

@swift-ci please smoke test and merge

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