Skip to content

[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

Conversation

DougGregor
Copy link
Member

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:

  • Fixes a bug in the Clang importer where we would fail to import both an 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".
  • Copes with conformance to protocols where the same Objective-C method has been imported with two different Swift requirements (a completion-handler one and an async one). Ensure that we don't treat one requirement as a "missing witness" if indeed the other requirement was satisfied.
  • Dial back @asyncHandler inference for witnesses in classes, because it causes source compatibility issues when combined with other aspects of the concurrency model.

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.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please smoke test macOS

@DougGregor DougGregor merged commit d21ff5c into swiftlang:main Oct 7, 2020
@DougGregor DougGregor deleted the concurrency-objc-compatibility branch October 7, 2020 02:43
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.

1 participant