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
A protocol conformance can be ill-formed due to isolation mismatches
between witnesses and requirements, or with associated conformances.
Previously, such failures would be emitted as a number of separate
errors (downgraded to warnings in Swift 5), one for each witness and
potentially an extra for associated conformances. The rest was a
potential flood of diagnostics that was hard to sort through.
Collect all of the isolation-related problems for a given conformance
together and produce a single error (downgraded to a warning when
appropriate) that describes the overall issue. That error will have up
to three notes suggesting specific courses of action:
* Isolating the conformance (when the experimental feature is enabled)
* Marking the witnesses as 'nonisolated' where needed
*
The diagnostic also has notes to point out the witnesses/associated
conformances that have isolation problems. There is a new educational
note that also describes these options.
We give the same treatment to missing 'distributed' on witnesses to a
distributed protocol.
When diagnosing an isolation mismatch between a requirement and witness,
we would produce notes on the requirement itself suggesting the addition of
`async`. This is almost never what you want to do, and is often so far
away from the actual conforming type as to be useless. Remove this note,
and the non-function fallback that just points at the requirement, because
they are unhelpful.
This is staging for a rework of the way we deal with conformance-level
actor isolation problems.
`x declared here` is not helpful and clear enough, especially when there
are other notes attached. Swap it for a new note that says
`requirement x declared here`.
Find all the usages of `--enable-experimental-feature` or
`--enable-upcoming-feature` in the tests and replace some of the
`REQUIRES: asserts` to use `REQUIRES: swift-feature-Foo` instead, which
should correctly apply to depending on the asserts/noasserts mode of the
toolchain for each feature.
Remove some comments that talked about enabling asserts since they don't
apply anymore (but I might had miss some).
All this was done with an automated script, so some formatting weirdness
might happen, but I hope I fixed most of those.
There might be some tests that were `REQUIRES: asserts` that might run
in `noasserts` toolchains now. This will normally be because their
feature went from experimental to upcoming/base and the tests were not
updated.
When diagnosing a case where an actor-isolated witness cannot satisfy
a non-isolated requirement, also suggest that the conformance could be
annotated with `@preconcurrency`.
property for IsolatedDefaultValues.
For property wrappers and init accesors, skip property initializers that are
subsumed, e.g. by an init accessor or a backing property wrapper initializer,
and always consider the subsuming initializer to determine whether compiler
synthesized initializers should have `nonisolated` applied.
This change also lessens the source break of SE-0411 by still emitting
member initializers in implicit constructors when the initializer violates
actor isolation to preserve the behavior of existing code when concurrency
diagnostics are downgraded to warnings in Swift 5 mode.
actor isolation.
Adding global actor isolation via subclassing admits data races because
actor-isolated types are Sendable while nonisolated classes are not (unless
otherwise annotated), so this allowed bypassing Sendable checking.
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.
Reimplement the final client of ActorIsolationRestriction, conformance
isolation checking, to base it on the new "actor reference" logic.
Centralize the diagnostics emission so we have a single place where we
emit the primary diagnostic (which is heavily customized based on
actor isolation/distributed/etc.) and any relevant notes to make
adjustments to the witness and/or requirement, e.g., adding
'distributed', 'async', 'throws', etc. Improve the diagnostics
slightly by providing Fix-Its when suggesting that we add "async"
and/or "throws".
With the last client of ActorIsolationRestriction gone, remove it
entirely.
Start collapsing the several implementations of actor isolation checking
into a single place that determines what it means to reference a declaration
from a given context, potentially supplying an instance for an actor. This
is partly cleanup, and partly staging for the implementation of the
Sendable restrictions introduced in SE-0338. The result of this check
falls into one of three categories:
* Reference occurs within the same concurrency domain (actor/task)
* Reference leaves an actor context to a nonisolated context (SE-0338)
* Reference enters the context of the actor, which might require a
combination of implicit async, implicit throws, and a "distributed" check.
Throughout this change I've sought to maintain the existing semantics,
even where I believe they are incorrect. The changes to the test cases
are not semantic changes, but reflect the unification of some
diagnostic paths that changed the diagnostic text but not when or how
those diagnostics are produced. Additionally, SE-0338 has not yet been
implemented, although this refactoring makes it easier to implement
SE-0338.
Use this new actor isolation checking scheme to implement the most
common actor-isolation check, which occurs when accessing a member of
an instance.
This effectively reverts 6823744779
The blanket removal of isolation in default-value expressions had
unintented consequences for important workflows. It's still
a problem that needs to be addressed, but we need to be more precise
about the problematic situations.
This patch delays the removal of redundant isolation for inferred
global-actor isolation to Swift 6 too, since we only warn about it
changing in Swift 5. Otherwise, only isolation that is a byproduct
of inference no longer needs an await, which will probably confuse
people.
This change is with respect to SE-327, which argues that the
non-static stored properties of ordinary structs do not need
global-actor isolation.
If a struct is a property-wrapper, then global-actor isolation
still applies to the `wrappedValue`, even if it's a stored property.
This is needed in order to support the propagation of global-actor
isolation through the wrapper, even when the programmer has opted
to use a stored property instead of a computed one for the
`wrappedValue`. Since this propagation is a useful pattern, I think
this exception is reasonable.
In Swift 5 mode, I only warned about the global-actor attribute
becoming unnecessary in the future, yet I was still returning
None for the global-actor attribute checking request. This led
to the attribute remaining unidentified, but also not removed.
During module serialization, this problem was manifesting as
emitting a typeless attribute, which when deserialized would
trigger a segfault.
As part of SE-327, global-actor isolation applied to
the instance-stored properties of a value type do
not require any isolation, since there is no way to
create a race on access to that storage.
https://github.com/apple/swift-evolution/blob/main/proposals/0327-actor-initializers.md#removing-redundant-isolation
This change turns global-actor annotations on such
properties into an error in Swift 6+, and a warning
in Swift 5 and earlier.
In addition, inference for global-actor isolation
no longer applies global-actor isolation to such
properties. Since this latter change only results
in warnings in existing Swift 5 code, about a now
superflous 'await', this change will happen in
Swift 5+.
Fixes rdar://87568381
Introduce the `@preconcurrency` attribute name for `@_predatesConcurrency`,
which has been the favored name in the pitch thread so far. Retain the
old name for now to help smooth migration.
It's possible to create an impossible set of constraints for
instance-member stored properties of a type. For example:
@MainActor func getStatus() -> Int { /* ... */ }
@PIDActor func genPID() -> ProcessID { /* ... */ }
class Process {
@MainActor var status: Int = getStatus()
@PIDActor var pid: ProcessID = genPID()
init() {} // Problem: what is the isolation of this init?
}
We cannot satisfy the isolation of the initilizing expressions,
which demand that genStatus and genPID are run with isolation
from a non-async designated initializer, which is not possible.
This patch changes the isolation for those initializer expressions
for instance members, saying that the isolation is unspecified.
fixes rdar://84225474
The first attempt to do this was in
https://github.com/apple/swift/pull/40652
But, I implemented that as a hard source break, since the isolation
was changed in a way that an error diagnostic would be emitted.
This commit reimplements the change more gently, as a warning for
Swift 5 users.
