Replaces generic `expression is 'async' but is not marked with 'await`
diagnostic with a tailed one for cases where there is an access to an
actor-isolated value outside of its actor without `await` keyword.
This makes the diagnostics for async and sync contexts consistent
and actually identifies a problem instead of simply pointing out
the solution.
Resolves: rdar://151720646
This matches send non sendable but importantly also makes it clear that we are
talking about something that doesn't conform to the Sendable protocol which is
capitalized.
rdar://151802975
The logic here got confused over time. This simplifies the logic and ensures
that we do not send a value if it is in the same isolation domain as the callee.
The one interesting side effect of this is that in a few tests, due to the logic
being confused, we were emitting use-after-send errors for global actor isolated
values that were passed to a function that was global actor isolated to the same
actor and then used later locally. The error was sending 'X'-isolated a to
'X'-isolated function causes race against nonisolated local uses. In truth, this
error is misleading and the only error that we should be emitting in such a case
is the error about moving an isolated value into a non-isolated context (which
we already emit).
rdar://132932382
Instead, use the `%target-swift-5.1-abi-triple` substitution to compile the tests
for deployment to the minimum OS versions required for use of _Concurrency APIs.
Specifically:
I changed the main error message to focus on the closure and that the closure
is being accessed concurrently.
If we find that we captured a value that is the actual isolation source, we
emit that the capture is actually actor isolated.
If the captured value is in the same region as the isolated value but is not
isolated, we instead say that the value is accessible from *-isolated code or
code within the current task.
If we find multiple captures and we do not which is the actual value that was
in the same region before we formed the partial apply, we just emit a note on
the captures saying that the closure captures the value.
I changed the diagnostics from using the phrase "task-isolated" to use some
variant of accessible to code in the current task.
The idea is that in all situations we provide a breadcrumb that the user can
start investigating rather than just saying that the closure is "task-isolated".
From a preconcurrency perspective, I made it so that we apply the preconcurrency
behavior of all of the captures. This means that if one of the captures is
preconcurrency, we apply the preconcurrency restriction to the closure. This is
one step towards making it so that preconcurrency applies at the region level...
we just are not completely there yet.
rdar://133798044
wording.
Splitting up the diagnostic into separate diagnostics based on the reference
kind is easier for me to read. The wording of the error message now puts
the problem -- crossing an isolation boundary -- at the center of the message,
and attempts to clarify how the value crosses an isolation boundary. E.g. for
the witness diagnostics, the value crosses an isolation boundary when calling
the witness through the protocol requirement in generic code.
This change does not add any additional information to the diagnostics, but it'd
be valuable to show both the source and destination isolation.
to `Sendable`.
The unavailability check was not using the root conformance, which is where
the extension declaration with the unavailability attribute is for inherited
conformances, leading to bogus warnings about re-stating unchecked conformances
to `Sendable`.
When diagnosing a concurrency-unsafe global or static variable, provide
Fix-Its with specific guidance and advice. This is intended to aid the
workflow for folks enabling strict concurrency checking or Swift 6.
There are up to three Fix-Its attached to a diagnostic about
concurrency-unsafe global/static variables:
* convert 'global' to a 'let' constant to make the shared state
immutable, which replaces `var` with `let`
* restrict 'global' to the main actor if it will only be accessed from the
main thread, which adds `@MainActor`
* unsafely mark %0 as concurrency-safe if all accesses are protected
by an external synchronization mechanism, which adds `nonisolated(unsafe)`
I fretted over two things before deciding on this path:
1. For the second note, the reality is that any global actor will
suffice, but `@MainActor` is orders of magnitude more common than any
other global actor, so "common case convenience" wins over "precise
but less useful.
2. For the third note, `nonisolated(unsafe)` should only be used
sparingly, and surfacing it via Fix-It could cause overuse. However,
developers need to know about it, and this is how we do that. It comes
last in the list of notes (after the better options) and says "unsafe"
in not one but two places.
I added a disable flag -disable-region-based-isolation-with-strict-concurrency
so that we do not need to update the current tests. It is only available when
asserts are enabled to ensure users cannot use it.
rdar://125918028
This means that:
1. In test cases where minimal is the default (swift 5 without
-warn-concurrency), I added RUN lines for targeted, complete, and complete +
sns.
2. In test cases where complete is the default (swift 6, -warn-concurrency,
specified complete with -strict-concurrency), I added a send non-sendable run
line.
In each of these cases, I added additional expected-* lines as appropriate so
the tests can compile in each mode successfully.
Isolation checking for calls had two separate implementation places:
one that looked at the declaration being called (for member
declarations) and one that worked on the actual call expression. Unify
on the latter implementation, which is more general and has access to
the specific call arguments. Improve diagnostics here somewher so we
don't regress in that area.
This refactoring shouldn't change the actual semantics, but it makes
upcoming semantic changes easier.
