Weaken some type checks for @preconcurrency decls #41386
Merged
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
In an async context, trying to pass a non-`@Sendable` function to an `@Sendable` parameter or trying to assign a `@MainActor` method to a non-`@MainActor`-typed variable were hard errors. We now think that this a mistake for `@preconcurrency` APIs in Swift 5 mode, as it hinders retroactive adoption of `@Sendable` and `@MainActor` by libraries. This PR weakens these errors to warnings *only* when the decl which contains the attribute in its type signature is `@preconcurrency` and *only* when in Swift 5 mode (with or without -warn-concurrency). For non-`@preconcurrency` decls, it is still an error. Fixes <rdar://88703266>.
|
@swift-ci smoke test |
DougGregor
reviewed
Feb 15, 2022
There was a problem hiding this comment.
This looks great, and is very much how @preconcurrency should work. There another place in isolation checking where we likely want to downgrade from an error to a warning when calling a @preconcurrency function that has a @Sendable parameter, which is actor-isolation checking. For example, I've seen this issue crop up:
@MainActor class C {
func f() { }
func g() {
someCompletionHandlerAPI {
f() // when completion handler was non-Sendable, this is okay
// now that completion handler is Sendable, it's an error. can we downgrade to a warning?
}
}
}If, for instance, an error is emitted as a warning instead, the verifier now detects this and emits a single diagnostic saying that the warning was found but had the wrong kind, instead of emitting one diagnostic saying the error was missing and another saying the warning was unexpected. In theory there are some edge cases we could handle better by doing two separate passes—one to detect exact expectation matches and remove them, another to detect near-misses and diagnose them—but in practice, I think the text + diagnostic location is likely to be unique enough to keep this from being a problem. (I would hesitate to do wrong-line diagnostics in the same pass like this, though.)
When a closure is not properly actor-isolated, but we know that we inferred its isolation from a `@preconcurrency` declaration, we now emit the errors as warnings in Swift 5 mode to avoid breaking source compatibility if the isolation was added retroactively.
|
@swift-ci smoke test |
|
macOS failure looks unrelated. @swift-ci smoke test macOS platform |
kavon
reviewed
Feb 25, 2022
| @@ -86,7 +90,7 @@ func testCallsWithAsync() async { | |||
| onMainActorAlways() // expected-error{{expression is 'async' but is not marked with 'await'}} | |||
| // expected-note@-1{{calls to global function 'onMainActorAlways()' from outside of its actor context are implicitly asynchronous}} | |||
|
|
|||
| let _: () -> Void = onMainActorAlways // expected-error{{converting function value of type '@MainActor () -> ()' to '() -> Void' loses global actor 'MainActor'}} | |||
| let _: () -> Void = onMainActorAlways // expected-warning{{converting function value of type '@MainActor () -> ()' to '() -> Void' loses global actor 'MainActor'}} | |||
There was a problem hiding this comment.
I wonder if we should also allow the assignment of a non-Sendable closure to a property whose type requires that it be sendable, if the property is a member of a pre-concurrency type / module?
There was a problem hiding this comment.
This already seems to work on main for Swift 5 without preconcurrency:
struct hello {
var clos: (@Sendable () -> Void)? = nil
}
func asdf(f: @escaping () -> Void) -> hello {
var h = hello()
h.clos = f // warning: assigning non-sendable parameter 'f' to a @Sendable closure
}
kavon
approved these changes
Feb 25, 2022
|
@swift-ci smoke test and merge |
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.
In an async context, trying to pass a non-
@Sendablefunction to an@Sendableparameter or trying to assign a@MainActormethod to a non-@MainActor-typed variable were hard errors. We now think that this a mistake for@preconcurrencyAPIs in Swift 5 mode, as it hinders retroactive adoption of@Sendableand@MainActorby libraries.This PR weakens these errors to warnings only when the decl which contains the attribute in its type signature is
@preconcurrencyand only when in Swift 5 mode (with or without -warn-concurrency). For non-@preconcurrencydecls, it is still an error.Fixes rdar://88703266.