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.
Remarks are intended to be enabled via eg. `-R...`, where as
`(add|remove)_predates_concurrency_import` is a diagnostic that's always
output without any `-R` flag. Move it to a warning instead.
Resolves rdar://114207080.
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.
It's ok to drop the global-actor qualifier `@G` from a function's type if:
- the cast is happening in a context isolated to global-actor `G`
- the function value will not be `@Sendable`
- the function value is not `async`
It's primarily safe to drop the attribute because we're already in the
same isolation domain. So it's OK to simply drop the global-actor
if we prevent the value from later leaving that isolation domain.
This means we no longer need to warn about code like this:
```
@MainActor func doIt(_ x: [Int], _ f: @MainActor (Int) -> ()) {
x.forEach(f)
// warning: converting function value of type '@MainActor (Int) -> ()' to '(Int) throws -> Void' loses global actor 'MainActor'
}
```
NOTE: this implementation is a bit gross in that the constraint solver
might emit false warnings about casts it introduced that are actually
safe. This is mainly because closure isolation is only fully determined
after constraint solving. See the FIXME's for more details.
resolves rdar://94462333
Instead of the `warning` Boolean threaded through the solver's
diagnostics, thread `DiagnosticBehavior` to be used as the behavior
limit. Use this for concurrency checking (specifically dropped
`@Sendable` and dropped global actors) so the solver gets more control
over these diagnostics.
This change restores the diagnostics to a usable state after the prior
change, which introduced extra noise. The only change from existing
beavior is that dropping a global actor from a function type is now
always a warning in Swift < 6. This is partly intentional, because
there are some places where dropping the global actor is well-formed.
This is possible because actors do not support inheritance. There
is one specific exception to that rule, which is that an actor
can inherit from `NSObject` just to support ObjC interop.
This means an actor is effectively a final class.
resolves rdar://87568153
`isConcurrencyChecked()` was being used as a proxy for
`-warn-concurrency` that didn't account for Swift 6. Replace
checks against it within the current module with checks against the
strict concurrency level, which subsumes the Swift 6 check and can
account for the difference between "limited" and "on".
`isConcurrencyChecked()` is used now used exclusively to mean "treat a
missing Sendable conformance as an explicitly-non-Sendable type".
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.
In the replacement of the escaping-use restriction with
flow-isolation, I hadn't accounted for all of the situations
where the isolation changes would break backwards compatability
with Swift 5.5 programs. The escaping-use restriction permitted
a lot of very unsafe things with warnings that it would become
an error in Swift 6.
With the introduction of flow-isolation, it was a bit tricky to
get the right warnings back in place, while not unnessecarily
warning about property accesses that might actually be OK. There
is a very careful coordination between the type-checker and
the flow-isolation pass.
While I had done these downgrades for deinits, I also needed to
do them for inits as well, because member accesses to isolated
methods within actor initializer were still getting rejected
as an error. This patch should be pretty solid now.
fixes rdar://90595278
There were some tests that relied on the top-level code not being an
asynchronous context to emit certain error messages. Now that it is,
those tests weren't emitting the expected error message.
In other cases, the issue was that they were trying to initialize a
global variable and weren't really using top-level code as top-level
code, so adding `-parse-as-library` was sufficient for the testing
purposes.
To fix the objc_async test, parsing as a library was nearly sufficient.
Unfortunately, the little `if #available` trick that I was using stopped
working since it relied on being in top-level code. So that we emit the
unavailableFromAsync error message, I had to set the availability on
everything correctly because we can't just disable availability
checking.
During actor isolation inference, we would unconditionally choose the
isolation of the overridden decl (when, say, there is no attribute on the decl).
The overridden decl is identified with `getOverriddenDecl`.
This works mostly fine, but initializers have some unusual overridden decls
returned by that method. For example, in the following code:
```swift
@objc class PictureFrame: NSObject {
init(size: Int) { }
}
@MainActor
class FooFrame: PictureFrame {
init() {
super.init(size: 0)
}
}
```
that method claims that `FooFrame.init()` overrides `PictureFrame.init()`, when
it really does not! So, if we were to unconditionally take the isolation from
`PictureFrame.init()` (and thus `NSObject.init()`), then we'd infer that
`FooFrame.init()` has unspecified isolation, despite the nominal it resides in
being marked as `MainActor`. This is in essence the problem in SR-15694, where
placing the isolation directly on the initializer fixes this issue.
If `FooFrame.init()` really does override, then why can it be `MainActor`? Well,
we have a rule in one part of the type-checker saying that if an ObjC-imported
decl has unspecified isolation, then overriding it with isolation is permitted.
But the other part of the type-checker dealing with the isolation inference was
not permitting that.
So, this patch unifies how actor-isolation inference is conducted to reuse that
same logic. In addition, the inference now effectively says that, if the decl
has no inferred isolation, or the inferred isolation is invalid according to the
overriding rules, then we use the isolation of the overridden decl. This
preserves the old behavior of the inference, while also fixing this issue with
ObjC declarations by expanding where isolation is allowed.
For example, as a consequence of this change, in the following code:
```swift
@MainActor
class FooFrame: NotIsolatedPictureFrame {
override func rotate() {
mainActorFn()
}
}
```
if `NotIsolatedPictureFrame` is a plain-old not-isolated class imported from
Objective-C, then `rotate` will now have `MainActor` isolation, instead of
having unspecified isolation like it was inferred to have previously. This
helps make things consistent, because `rotate` is allowed to be `@MainActor` if
it were explicitly marked as such.
resolves rdar://87217618 / SR-15694
With the clarification in SE-0338, revise the definition of "may
execute concurrently with" to always be false when we're accessing a
value from a same-actor-isolated inner context.
Fixes rdar://82004833.
This capability was available in Swift 5.5, but with flow-isolation
it's not safe to do so in general. When I initially implemented
flow-isolation, I intended for it to not break too much existing
code, and emit warnings about things changing in Swift 6. But
I missed this case, where we have just a simple method call in
a deinit, which is likely to be common:
```swift
actor A {
func cleanup() { ... }
deinit {
cleanup()
}
}
```
Instead of rejecting that call to `cleanup`, we now warn that it's
not going to be allowed in Swift 6, because `cleanup` is isolated
and the deinit is not.
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.
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
Flow-isolation is a diagnostic SIL pass that finds
unsafe accesses to properties in initializers and
deinitializers that cannot gain isolation to otherwise
protect those accesses from concurrent modifications.
See SE-327 for more details about how and why it exists.
This commit includes changes and features like:
- The removal of the escaping-use restriction
- Flow-isolation that works properly with `defer` statements
- Flow-isolation with an emphasis on helpful diagnostics.
It also includes known issues like:
- Local / nonescaping functions are not analyzed by
flow-isolation, despite it being technically possible.
The main challenge in supporting it efficiently is that
such functions do not have a single exit-point, like
a `defer`. In particular, arbitrary functions can throw
so there are points where nonisolation should _not_ flow
out of the function at a call-site in the initializer, etc.
- The implementation of the flow-isolation pass is not
particularly memory efficient; it relies on BitDataflow
even though the particular flow problem is simple.
So, a more efficient implementation would be specialized for
this particular problem, etc.
There are also some changes to the Swift language itself: defer
will respect its context when deciding its property access kind.
Previously, a defer in an initializer would always access a stored
property through its accessor methods, instead of doing so directly
like its enclosing function might. This inconsistency is unfortunate,
so for Swift 6+ we make this consistent. For Swift 5, only a defer
in a function that is a member of the following kinds of types
will gain this consistency:
- an actor type
- any nominal type that is actor-isolated, excluding UnsafeGlobalActor.
These types are still rather new, so there is much less of a chance of
breaking expected behaviors around defer. In particular, the danger is
that users are relying on the behavior of defer triggering a property
observer within an init or deinit, when it would not be triggering it
without the defer.
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.
Rather than tacking the "add `@_predatesConcurrecy` to import"
diagnostic on to the prior diagnostic as a note, make it its own
remark. Then, ensure that we only emit this remark once per source
file per imported module, so we're not overwhelming the user.
When a non-isolated requirement is witnessed by an actor-isolated
witness, we are crossing into the actor. Ensure that we perform
Sendable checking across the actor boundary here.
Fixes the rest of rdar://80424675.
