-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Concurrency] Improve source compatibility with 'async' imports #34171
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
DougGregor
merged 7 commits into
swiftlang:main
from
DougGregor:concurrency-objc-compatibility
Oct 7, 2020
Merged
[Concurrency] Improve source compatibility with 'async' imports #34171
DougGregor
merged 7 commits into
swiftlang:main
from
DougGregor:concurrency-objc-compatibility
Oct 7, 2020
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The handling of "global" missing witnesses would diagnose a missing witness (showing "almost" matches) and then, later, provide the Fix-It to add the declaration if the user wants it. Delay the diagnostic part until later, when we add the missing witness. While here, correct an existing issue with memory corruption that occurs because we would diagnose some of these witnesses, then clear a "global" witness list, while still iterating through parts of that list.
The Clang importer was filtering out cases where the same declaration is imported twice under the same name, which can now happen when one is synchronous and one is asynchronous. This happens when, e.g., an Objective-C class provides both a completion-hander-based asynchronous version and a synchronous version, and the Swift names line up after the completion-handler parameter is dropped. Stop filtering these out. Overload resolution is capable of handling synchronous/asynchronous overloading based on context.
…selector. When concurrency is enabled, the same Objective-C method on a protocol can be imported as two different protocol requirements, both of which have the same selector. When performing conformance checking, only treat a given @objc protocol requirement as "unsatisfied" by a conformance if none of the requirements with the same Objective-C selector (+ instance/class designation) are satisfied.
…sses. Actor classes are by definition new code, so only perform inference of @asyncHandler from a protocol requirement to witnesses when those witnesses are within an actor class. This is partly because @asyncHandler might not have reasonable semantics outside of actor classes anywhere (where would you execute the detached task?), and partly because it is a source-compatibility problem due to the combination of... 1) Inferring @asyncHandler on imported Objective-C protocol methods for existing protocols, 2) Inferring @asyncHandler from those protocol methods to an existing method in a class that conforms to that protocol, and 3) Using an API that is now overloaded for both synchronous and asynchronous callers will end up preferring the asynchronous version, then fail due to a missing "await". The individual steps here are all reasonable, but the result is a source break, so back off on @asyncHandler inference for witnesses outside of actor classes. New code gets the more-asynchronous behavior for free.
With actor isolation checking for protocol witnesses moved out of the witness-matching phase, move the corresponding diagnostics from notes (that would have been on the "type does not conform" error) to freestanding errors.
@swift-ci please smoke test |
@swift-ci please test source compatibility |
@swift-ci please smoke test |
@swift-ci please smoke test Linux |
@swift-ci please smoke test |
@swift-ci please smoke test macOS |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Address a few source-compatibility issues introduced by importing completion-handler Objective-C methods
as '`async' alongside their completion handlers. This is a combination of outright bug fixes in the existing implementation, language rules for coping with the result of this transformation, and backing off slightly on concurrency-related inference rules:
async
function (from a completion handler) and a non-async
function that has the same signature, because we would filter one out as a "duplicate".async
one). Ensure that we don't treat one requirement as a "missing witness" if indeed the other requirement was satisfied.@asyncHandler
inference for witnesses in classes, because it causes source compatibility issues when combined with other aspects of the concurrency model.