We are creating/relying on a contract between the AST and SIL... that SILDeclRef
should accurately describe the method/accessor that a class_method is from. By
doing this we eliminate pattern matching on the AST which ties this code too
tightly to the AST and makes it brittle in the face of AST changes. This also
fixes an issue where we were not handling setters correctly.
I am doing this now since it is natural to fix it along side fixing the
ref_element_addr issue in the previous commit since they are effectively doing
the same thing.
rdar://153207557
The previous algorithm was doing an iterative forward data flow analysis
followed by a reverse data flow analysis. I suspect the history here is that
it was a reverse analysis, and that didn't really work for infinite loops,
and so complexity accumulated.
The new algorithm is quite straightforward and relies on the allocations
being properly jointly post-dominated, just not nested. We simply walk
forward through the blocks in consistent-with-dominance order, maintaining
the stack of active allocations and deferring deallocations that are
improperly nested until we deallocate the allocations above it. The only
real subtlety is that we have to delay walking into dead-end regions until
we've seen all of the edges into them, so that we can know whether we have
a coherent stack state in them. If the state is incoherent, we need to
remove any deallocations of previous allocations because we cannot talk
correctly about what's on top of the stack.
The reason I'm doing this, besides it just being a simpler and hopefully
faster algorithm, is that modeling some of the uses of the async stack
allocator properly requires builtins that cannot just be semantically
reordered. That should be somewhat easier to handle with the new approach,
although really (1) we should not have runtime functions that need this and
(2) we're going to need a conservatively-correct solution that's different
from this anyway because hoisting allocations is *also* limited in its own
way.
I've attached a rather pedantic proof of the correctness of the algorithm.
The thing that concerns me most about the rewritten pass is that it isn't
actually validating joint post-dominance on input, so if you give it bad
input, it might be a little mystifying to debug the verifier failures.
The caller is allowed to assume that the 'inout sending' parameters are not in
the same region on return so can be sent to different isolation domains safely.
To enforce that we have to ensure on return that the two are /actually/ not in
the same region.
rdar://138519484
The intent for `@inline(always)` is to act as an optimization control.
The user can rely on inlining to happen or the compiler will emit an error
message.
Because function values can be dynamic (closures, protocol/class lookup)
this guarantee can only be upheld for direct function references.
In cases where the optimizer can resolve dynamic function values the
attribute shall be respected.
rdar://148608854
I am going to be adding support to passes for emitting IsolationHistory behind a
flag. As part of this, we need to store the state of the partition that created
the error when the error is emitted. A partition stores heap memory so it makes
sense to make these types noncopyable types so we just move the heap memory
rather than copying it all over the place.
If a guaranteed value is used in a dead-end exit block and the enclosing value is _not_ destroyed in this block, we end up missing the enclosing value as phi-argument after duplicating the loop.
TODO: once we have complete lifetimes we can remove this check again.
rdar://159125605
We want 'inout sending' parameters to have the semantics that not only are they
disconnected on return from the function but additionally they are guaranteed to
be in their own disconnected region on return. This implies that we must emit
errors when an 'inout sending' parameter or any element that is in the same
region as the current value within an 'inout sending' parameter is
returned. This commit contains a new diagnostic for RegionIsolation that adds
specific logic for detecting and emitting errors in these situations.
To implement this, we introduce 3 new diagnostics with each individual
diagnostic being slightly different to reflect the various ways that this error
can come up in source:
* Returning 'inout sending' directly:
```swift
func returnInOutSendingDirectly(_ x: inout sending NonSendableKlass) -> NonSendableKlass {
return x // expected-warning {{cannot return 'inout sending' parameter 'x' from global function 'returnInOutSendingDirectly'}}
// expected-note @-1 {{returning 'x' risks concurrent access since caller assumes that 'x' and the result of global function 'returnInOutSendingDirectly' can be safely sent to different isolation domains}}
}
```
* Returning a value in the same region as an 'inout sending' parameter. E.x.:
```swift
func returnInOutSendingRegionVar(_ x: inout sending NonSendableKlass) -> NonSendableKlass {
var y = x
y = x
return y // expected-warning {{cannot return 'y' from global function 'returnInOutSendingRegionVar'}}
// expected-note @-1 {{returning 'y' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' and the result of global function 'returnInOutSendingRegionVar' can be safely sent to different isolation domains}}
}
```
* Returning the result of a function or computed property that is in the same
region as the 'inout parameter'.
```swift
func returnInOutSendingViaHelper(_ x: inout sending NonSendableKlass) -> NonSendableKlass {
let y = x
return useNonSendableKlassAndReturn(y) // expected-warning {{cannot return result of global function 'useNonSendableKlassAndReturn' from global function 'returnInOutSendingViaHelper'}}
// expected-note @-1 {{returning result of global function 'useNonSendableKlassAndReturn' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' and the result of global function 'returnInOutSendingViaHelper' can be safely sent to different isolation domains}}
}
```
Additionally, I had to introduce a specific variant for each of these
diagnostics for cases where due to us being in a method, we are actually in our
caller causing the 'inout sending' parameter to be in the same region as an
actor isolated value:
* Returning 'inout sending' directly:
```swift
extension MyActor {
func returnInOutSendingDirectly(_ x: inout sending NonSendableKlass) -> NonSendableKlass {
return x // expected-warning {{cannot return 'inout sending' parameter 'x' from instance method 'returnInOutSendingDirectly'}}
// expected-note @-1 {{returning 'x' risks concurrent access since caller assumes that 'x' is not actor-isolated and the result of instance method 'returnInOutSendingDirectly' is 'self'-isolated}}
}
}
```
* Returning a value in the same region as an 'inout sending' parameter. E.x.:
```swift
extension MyActor {
func returnInOutSendingRegionLet(_ x: inout sending NonSendableKlass) -> NonSendableKlass {
let y = x
return y // expected-warning {{cannot return 'y' from instance method 'returnInOutSendingRegionLet'}}
// expected-note @-1 {{returning 'y' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' is not actor-isolated and the result of instance method 'returnInOutSendingRegionLet' is 'self'-isolated}}
}
}
```
* Returning the result of a function or computed property that is in the same region as the 'inout parameter'.
```swift
extension MyActor {
func returnInOutSendingViaHelper(_ x: inout sending NonSendableKlass) -> NonSendableKlass {
let y = x
return useNonSendableKlassAndReturn(y) // expected-warning {{cannot return result of global function 'useNonSendableKlassAndReturn' from instance method 'returnInOutSendingViaHelper'; this is an error in the Swift 6 language mode}}
// expected-note @-1 {{returning result of global function 'useNonSendableKlassAndReturn' risks concurrent access to 'inout sending' parameter 'x' since the caller assumes that 'x' is not actor-isolated and the result of instance method 'returnInOutSendingViaHelper' is 'self'-isolated}}
}
}
```
To implement this, I used two different approaches depending on whether or not
the returned value was generic or not.
* Concrete
In the case where we had a concrete value, I was able to in simple cases emit
diagnostics based off of the values returned by the return inst. In cases where
we phied together results due to multiple results in the same function, we
determine which of the incoming phied values caused the error by grabbing the
exit partition information of each of the incoming value predecessors and seeing
if an InOutSendingAtFunctionExit would emit an error.
* Generic
In the case of generic code, it is a little more interesting since the result is
a value stored in an our parameter instead of being a value directly returned by
a return inst. To work around this, I use PrunedLiveness to determine the last
values stored into the out parameter in the function to avoid having to do a
full dataflow. Then I take the exit blocks where we assign each of those values
and run the same check as we do in the direct phi case to emit the appropriate
error.
rdar://152454571
The specific code path is the code used to emit errors if we assign or merge
into a sending result. I just left the code in tree in the short term to prevent
cherry-picking issues and since there wasn't a strong reason to do it at the
time. Now that we have more freedom, lets clean up this code!
Specifically the type checker to work around interface types not having
isolation introduces casts into the AST that enrich the AST with isolation
information. Part of that information is Sendable. This means that we can
sometimes lose due to conversions that a function is actually Sendable. To work
around this, we today suppress those errors when they are emitted (post 6.2, we
should just change their classification as being Sendable... but I don't want to
make that change now).
This change just makes the pattern matching for these conversions handle more
cases so that transfernonsendable_closureliterals_isolationinference.swift now
passes.
Centralize the logic for figuring out the conformances for the various
init_existential* instructions in a SILIsolationInfo static method, and
always go through that when handling "assign" semantics. This way, we
can use CONSTANT_TRANSLATION again for these instructions, or a simpler
decision process between Assign and LookThrough.
The actually undoes a small change made earlier when we stopped looking
through `init_existential_value` instructions. Now we do when there are
no isolated conformances.
Better match the style of SILIsolationInfo by moving the code for determining
SILIsolationInfo from conformances or dynamic casts to existentials into
static `getXYZ` methods on SILIsolationInfo.
Other than adding an assertion regarding disconnected regions, no
intended functionality change.
When we introduce isolation due to a (potential) isolated conformance,
keep track of the protocol to which the conformance could be
introduced. Use this information for two reasons:
1. Downgrade the error to a warning in Swift < 7, because we are newly
diagnosing these
2. Add a note indicating where the isolated conformance could be introduced.
Specifically in terms of printing, if NonisolatedNonsendingByDefault is enabled,
we print out things as nonisolated/task-isolated and @concurrent/@concurrent
task-isolated. If said feature is disabled, we print out things as
nonisolated(nonsending)/nonisolated(nonsending) task-isolated and
nonisolated/task-isolated. This ensures in the default case, diagnostics do not
change and we always print out things to match the expected meaning of
nonisolated depending on the mode.
I also updated the tests as appropriate/added some more tests/added to the
SendNonSendable education notes information about this.
I am doing this so that I can change how we emit the diagnostics just for
SendNonSendable depending on if NonisolatedNonsendingByDefault is enabled
without touching the rest of the compiler.
This does not actually change any of the actual output though.
This results in wrong argument/return calling conventions.
First, the method call must be specialized. Only then the call can be de-virtualized.
Usually, it's done in this order anyway, because the `class_method` instruction is located before the `apply`.
But when inlining functions, the order (in the worklist) can be the other way round.
Fixes a compiler crash.
rdar://154631438
OSSA lifetime canonicalization can take a very long time in certain
cases in which there are large basic blocks. to mitigate this, add logic
to skip walking the liveness boundary for extending liveness to dead
ends when there aren't any dead ends in the function.
Updates `DeadEndBlocks` with a new `isEmpty` method and cache to
determine if there are any dead-end blocks in a given function.
When the utility is used by the ConsumeOperatorCopyableValuesChecker,
the checker guarantees that the lifetime can end at the consumes, that
there are no uses after those consumes. In that circumstance, the
utility maintains liveness to those consumes and as far as possible
without introducing a copy everywhere else.
The lack of complete lifetimes has forced the utility to extend liveness
of values to dead-ends. That extension, however, is in tension with the
use that the checker is putting the utility to. If there is a dead-end
after a consume, liveness must not be maintained to that dead-end.
rdar://147586673
