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
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.
If an instruction references the DynamicSelfType by calling a
static member with `Self.foo()`, we consider this a type-dependent
use of `self`. This means that at runtime we may need to load the
isa pointer, but we don't need to touch any other protected state
from the instance.
Therefore, we can skip type-dependent uses in the analysis to
avoid false positives in this case.
An existing test case already exercised the overly-conservative
behavior, so I just updated it to not expect an error.
Fixes rdar://129676769.
When SE-0412 (strict concurrency for global variables) is enabled, each
global or static mutable variable will be diagnosed if it isn't
explicitly on a global actor or `nonisolated(unsafe)`.
Suppress diagnostics for references to such global variables when they
are in the same module as the declaration of the global variable
itself. While these diagnostics are technically correct, they are not
strictly necessary since we've already diagnosed the global variable
itself (with more actionable advice), and they tend to pile on the
developer in a manner that is not helpful.
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.
Instance properties of non-sendable types cannot safely be
accessed within deinitializers. Make sure we respect `@preconcurrency`
when diagnosing these.
All non-trivial values which correspond to `VarDecl`s must be marked
appropriately so that they can be found by the relevant checker. Here,
values with none ownership are marked.
Init accessor properties should be handled specifically because
they do not reference underlying storage directly via `ref_element_addr`
instructions when 'self' is fully initialized.
Init accessor properties cannot escape self and are only allowed
to initialized and access a set of properties declared in their
`@storageRestrictions(...)` attribute.
Resolves: https://github.com/apple/swift/issues/70550
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.
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
The flow-isolation pass was not respecting the new strict-concurrency checking mode.
Since the Sendable diagnostics in these deinits are very noisy, I'm moving them to only
be emitted in 'complete' mode. The reason why they're so noisy is that any class that
inherits from a `@MainActor`-constrained class will have these diagnostics emitted when
trying to access its own `@MainActor`-isolated members.
This is needed, even during the `deinit`, because multiple instances of a `@MainActor`-isolated
class might have stored properties that refer to the same state.
This change specifically avoids emitting these diagnostics even in 'targeted' mode because
I'd like to take more time to reconsider the ergonomics of these deinits.
resolves rdar://94699928
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
When emitting the note to point out what introduced
nonisolation, the code building the message assumed that
`DeclContext::getDecl` will not return null, when it can
if it's a closure expression.
fixes rdar://88776902
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 is a combination of fixes:
- inject hops in self-isolated delegating actor initializers
after self becomes initialized.
- fix sendable and isolation for convenience inits
- fix bug in distributed actor inits that I introduced when
implementing flow-isolation.
- fix / add test coverage.
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.
