We are already using this routine in other parts of TransferNonSendable to
ensure that we look through common insts that SILGen inserts that do not change
the actual underlying actor instance that we are using. In this case, I added
support for casts, optional formation, optional extraction, existential ref
initialization.
As an example of where this came up is the following test case where we fail to
look through an init_existential_ref.
```swift
public actor MyActor {
private var intDict: [Int: Int] = [:]
public func test() async {
await withTaskGroup(of: Void.self) { taskGroup in
for (_, _) in intDict {}
await taskGroup.waitForAll() // Isolation merge failure happens here
}
}
}
```
I also added the ability to at the SIL level actual test out this merge
condition using the analysis test runner. I used this to validate that this
functionality works as expected in a precise way.
rdar://130113744
Before we wouldn't print them in all situations and even more so a few of the
printing routines did not have it at all. This just adds a centralized
SILIsolationInfo::dumpOptions() method and then goes through all of the printing
helpers and changes them to use them as appropriate.
Given a function or a partial_apply with an isolated parameter, we do not know
immediately what the actual isolation is of the function or partial_apply since
we do not know which instance will be applied to the function or partial_apply.
In this commit, I introduce a new bit into SILIsolationInfo that tracks this
information upon construction and allows for it to merge with ownership that has
the appropriate type and a specific instance. Since the values that created the
two isolations, will be in the same region this should ensure that the value is
only ever in a flow sensitive manner in a region with only one actor instance
(since regions with isolations with differing actor instances are illegal).
Before this change in the following code, we would say that message is isolated to the actor instead of the global actor isolation of the actor's method:
```swift
class Message { ... }
actor MessageHolder {
@MainActor func hold(_ message: Message) { ... }
}
@MainActor
func sendMessage() async {
let messageHolder = MessageHolder()
let message = Message()
// We identified messageHolder.hold as being MessageHolder isolated
// instead of main actor isolated.
messageHolder.hold(message)
Task { @MainActor in print(message) }
}
```
I also used this as an opportunity to simplify the logic in this part of the
code. Specifically, I had made it so that multiple interesting cases were
handled in the same conditional statement in a manner that it made it hard to
know which cases were actually being handled and why it was correct. Now I split
that into two separate if statements with comments that make it clear what we
are actually trying to pattern match against.
rdar://130980933
Specifically:
1. We error now if one transfers an 'inout sending' parameter and does not
reinitialize it before the end of the function.
2. We error now if one merges an 'inout sending' parameter into an actor
isolated region and do not reinitialize it with a non-actor isolated value
before the end of the function.
rdar://126303739
Remove `deque` from files it isn't actually used in. Add it and `stack`
to files that it is - presumably they were previously transitively found
through other includes.
This was causing us to emit old style diagnostics in certain cases.
Just doing a walk through of the diagnostics and fixing issues while I
am here sprucing up sending diagnostics.
rdar://130915737
SILInstruction::clone doesn't know how to clone instructions that produce
the archetype uuid. SILCloner is equipped to handle such instructions.
Optimizations like LoopRotate use SILInstruction::clone and will be
incorrect for such instructions.
rdar://130047619
This corresponds to the parameter-passing convention of the Itanium C++
ABI, in which the argument is passed indirectly and possibly modified,
but not destroyed, by the callee.
@in_cxx is handled the same way as @in in callers and @in_guaranteed in
callees. OwnershipModelEliminator emits the call to destroy_addr that is
needed to destroy the argument in the caller.
rdar://122707697
The de-virtualizer utility didn't handle indirect error results when de-virtualizing class or actor methods.
This resulted in a missing argument for the indirect error result in the new try_apply instruction.
rdar://130545338
Otherwise, we will have differing isolation from other parameters since
the isolations will look different since one will have the .none value
as an instance and the other will not have one and instead will rely on
the AST isolation info. That is the correct behavior here since we do
not actually have an actor here.
I also removed some undefined behavior in the merging code. The way the
code should work is that we should check if the merge fails and in such
a case emit an unknown pattern error... instead of not checking
appropriately on the next iteration and hitting undefined behavior.
rdar://130396399
From the perspective of the IR, we are changing SILIsolationInfo such that
inferring an actor instance means looking at equivalence classes of values where
we consider operands to look through instructions to be equivalent to their dest
value. The result is that cases where the IR maybe puts in a copy_value or the
like, we consider the copy_value to have the same isolation info as using the
actor directly. This prevents a class of crashes due to merge failings. Example:
```swift
actor MyActor {
init() async {
init(ns: NonSendableKlass) async {
self.k = NonSendableKlass()
self.helper(ns)
}
func helper(_ newK: NonSendableKlass) {}
}
```
Incidently, we already had a failing test case from this behavior rather than
the one that was the original genesis. Specifically:
1. If a function's SILIsolationInfo is the same as the isolation info of a
SILValue, we assume that no transfer actually occurs.
2. Since we were taking too static of a view of actor instances when comparing,
we would think that a SILIsolationInfo of a #isolation parameter to as an
argument would be different than the ambient's function isolation which is also
that same one. So we would emit a transfer non transferrable error if we pass in
any parameters of the ambient function into another isolated function. Example:
```swift
actor Test {
@TaskLocal static var local: Int?
func withTaskLocal(isolation: isolated (any Actor)? = #isolation,
_ body: (consuming NonSendableValue, isolated (any Actor)?) -> Void) async {
Self.$local.withValue(12) {
// We used to get these errors here since we thought that body's isolation
// was different than the body's isolation.
//
// warning: sending 'body' risks causing data races
// note: actor-isolated 'body' is captured by a actor-isolated closure...
body(NonSendableValue(), isolation)
}
}
}
```
rdar://129400019
When devirtualizing witness method calls, it can happen that we need a cast between ABI compatible return types.
We were missing supporting type casts between nominal types which are ABI compatible.
This comes from whole-module reasoning of protocol conformances.
If a protocol only has a single conformance where the associated type (`ID`) is some concrete type (e.g. `Int`), then the devirtualizer knows that `p.get()` can only return an `Int`:
```
public struct X2<ID> {
let p: any P2<ID>
public func testit(i: ID, x: ID) -> S2<ID> {
return p.get(x: x)
}
}
```
and after devirtualizing the `get` function, its result must be cast from `Int` to `ID`.
The `layoutIsTypeDependent` utility is basically only used here to assert that this cast can only happen between layout compatible types.
rdar://129004015
This is usually the case. Some examples, where they layout is _not_ dependent:
```
struct S<T> {
var x: Int // no members which depend on T
}
struct S<T> {
var c: SomeClass<T> // a class reference does not depend on the layout of the class
}
```
Create two versions of the following functions:
isConsumedParameter
isGuaranteedParameter
SILParameterInfo::isConsumed
SILParameterInfo::isGuaranteed
SILArgumentConvention::isOwnedConvention
SILArgumentConvention::isGuaranteedConvention
These changes will be needed when we add a new convention for
non-trivial C++ types as the functions will return different answers
depending on whether they are called for the caller or the callee. This
commit doesn't change any functionality.
Cloning blocks might split CFG edges which can "convert" terminator result arguments to phi-arguments.
In this case a borrowed-from instruction must be inserted.
Fixes a SIL verifier crash caused by SimplifyCFG's jump threading.
rdar://129187525
This just means that I stopped treating it like an actor instance isolated
thing. This was fun to track down since ActorIsolation has a union in it that
was being misinterpreted, leading to memory corruption... my favorite! = ).
rdar://129256560
Now that the two known issues which resulted in invalid SIL being
provided to lifetime completion have been addressed, tighten up the
completion done on the availability boundary not to allow leaks.
Although I don't plan to bring over new assertions wholesale
into the current qualification branch, it's entirely possible
that various minor changes in main will use the new assertions;
having this basic support in the release branch will simplify that.
(This is why I'm adding the includes as a separate pass from
rewriting the individual assertions)
It indicates that the value's lifetime continues to at least this point.
The boundary formed by all consuming uses together with these
instructions will encompass all uses of the value.
Enhance the utility with the ability to end lifetimes of lexical values
at indicated instructions, overriding the usual behavior of maintaining
such lifetimes' previous endpoints (modulo non-deinit-barrier
instructions).
Parameterized `extendUnconsumedLiveness` on the ends of interest and the
action to take when visiting the extended boundary and named the
resulting function `visitExtendedUnconsumedBoundary`.
