Although nonescaping closures are representationally trivial pointers to their
on-stack context, it is useful to model them as borrowing their captures, which
allows for checking correct use of move-only values across the closure, and
lets us model the lifetime dependence between a closure and its captures without
an ad-hoc web of `mark_dependence` instructions.
During ownership elimination, We eliminate copy/destroy_value instructions and
end the partial_apply's lifetime with an explicit dealloc_stack as before,
for compatibility with existing IRGen and non-OSSA aware passes.
As part of this I also had to change how we emit global_addr in
SILGenLValue. Specifically, only for noncopyable types, we no longer emit a
single global_addr at the beginning of the function (in a sense auto-CSEing) and
instead always emit a new global_addr for each access. The reason why we do this
is that otherwise, access base visitor will consider all accesses to the global
to be for the same single access. In contrast, by always emitting the
global_addr each time, we provide a new base for each access allowing us to emit
the diagnostics that we want to.
rdar://102794400
This fits the name of the check better. The reason I am doing this renaming is
b/c I am going to add a nonconsumable but assignable check for
global_addr/ref_element_addr/captures with var semantics.
This reflects better the true meaning of this check which is that a value marked
with this check cannot be consumed on its boundary at all (when performing
let/var checking) and cannot be assigned over when performing var checking.
I discovered when working with improving the debug output of the move only
address checker that I had a need for lightweight thing like
DebugVarCarryingInst but that only could vend a VarDecl (unlike
DebugVarCarryingInst which also can vend a SILDebugVariable). As an example,
this lets one write a high level API that uses the standard API to loop over a
bunch of instructions all that vend a VarDecl and construct a stringified path
component list.
rdar://105293841
The Swift Simplification pass can do more than the old MandatoryCombine pass: simplification of more instruction types and dead code elimination.
The result is a better -Onone performance while still keeping debug info consistent.
Currently following code patterns are simplified:
* `struct` -> `struct_extract`
* `enum` -> `unchecked_enum_data`
* `partial_apply` -> `apply`
* `br` to a 1:1 related block
* `cond_br` with a constant condition
* `isConcrete` and `is_same_metadata` builtins
More simplifications can be added in the future.
rdar://96708429
rdar://104562580
The reason why I am doing this is that:
1. There are sometimes copy_value on move only values loaded from memory that
the MoveOnlyAddressChecker needs to eliminate.
2. Previously, the move only address checker did not rewrite copy_value ->
explicit_copy_value if it failed in diagnostics (it did handle load [copy] and
copy_addr though). This could then cause the copy_value elimination verification
in the MoveOnlyObjectChecker to then fail. So this suggested that I needed to
move the verification from the object checkt to the address checker.
3. If we run the verification in the address checker, then the object checker
(which previously ran before the address checker) naturally needed to run
/before/ the address checker.
As mentioned previously, I am right now creating new Implementations for each
switch_enum argument. Once I get tests working I am going to refactor and reuse
the same implementation so I can reuse memory.
NOTE: This currently does not impact SILGen since I did not change SILGenPattern
to emit noncopyable switched upon values as guaranteed. Keep in mind that I am
not going to do this for no implicit copy though since no implicit copy values
cannot contain move only values so we do not need to work with destructures.
For now, I am creating new implementations every time... but once I get the
tests passing, I am going to improve the performance by reusing the same
Implementation for all computations so we can reuse memory.
This is in preparation for sinking the main computation of the pass into an
implementation local helper struct that can process multiple values. I am doing
this so we can use the same code to process switch_enum arguments and am using
the current implementation to make sure I don't break anything.
This will let the non-field sensitive version use a more performant
implementation internally. This is important since PrunedLiveBlocks is used in
the hot path when working with Ownership SSA, while the field sensitive version
is only used for certain diagnostics.
NOTE: I did not refactor PrunedLiveness to use the faster implementation... this
is just a quick pass over the code to prepare for that change.
Specifically, previously if we emitted an error we just dumped all of the
consuming uses. Now instead for each consuming use that needs a copy, we perform
a search for a specific boundary use (consuming or non-consuming) that is
reachable from the former and emit a specialized error for it. Thus we emit for
the two consuming case the normal consumed twice error, and now for
non-consuming errors we emit the "use after consume" error.
For those who are unaware, CanonicalizeOSSALifetime::canonicalizeValueLifetime()
is really a high level driver routine for the functionality of
CanonicalizeOSSALifetime that computes liveness and then rewrites copies using
boundary information. This change introduces splits the implementation of
canonicalizeValueLifetime into two parts: a first part called computeLiveness
and a second part called rewriteLifetimes. Internally canonicalizeValueLifetime
still just calls these two methods.
The reason why I am doing this is that it lets the move only object checker use
the raw liveness information computed before the rewriting mucks with the
analysis information. This information is used by the checker to compute the raw
liveness boundary of a value and use that information to determine the list of
consuming uses not on the boundary, consuming uses on the boundary, and
non-consuming uses on the boundary. This is then used by later parts of the
checker to emit our errors.
Some additional benefits of doing this are:
1. I was able to eliminate callbacks in the rewriting stage of
CanonicalOSSALifetimes which previously gave the checker this information.
2. Previously the move checker did not have access to the non-consuming boundary
uses causing us to always fail appropriately, but sadly not emit a note showing
the non-consuming use. I am going to wire this up in a subsequent commit.
The other change to the implementation of the move checker that this caused is
that I needed to add an extra diagnostic check for instructions that consume the
value twice or consume the value and use the value. The reason why this must be
done is that liveness does not distinguish in between different operands on the
same instruction meaning such an error would be lost.
The pass is attempting to diagnose any user-written code that appears
after the result of no-return function call. It has to skip any
instructions that happen after the no-return call but are associated
with it, such as `alloc_stack` to pass result of such call indirectly.
This helps to avoid extraneous diagnostics for synthesized code i.e.
a result builder with `buildExpression`:
```
static func buildExpression<T>(_ e: T) -> T { e }
```
The following example would produce extraneous warning:
```
switch <value> {
case ...
default: fatalError()
}
```
because it is translated into:
```
switch <value> {
case ...
default: {
var $__builderDefault = buildExpression(fatalError())
$__builderSwitch = buildEither(second: $__builderDefault)
}
}
```
In such cases all instructions that follow `fatalError()` call
are synthesized except to `alloc_stack $Never` which is passed
to `buildExpression`, that instruction is anchored on the
`fatalError()` itself which is where the diagnostic would point,
which is incorrect.
Resolves: rdar://104775183
The reason to do the first is to ensure that when I enable -sil-verify-all, we
do not have errors due to a copy_value of a move only type.
The reason to do the second thing is that:
1. If we have a move only type that is no implicit copy, I am in a subsequent
commit going to emit a type checker error saying that one cannot do this. When
we implement consuming/borrowing bindings/etc, we can make this looser if it
gets into the way.
2. If we have a copyable no implicit copy type, then any structural accesses
that we may want to do that would require a destructure must be to a copyable
type which is ok to copy as long as we do the unwrap from the first thing.
rdar://104929957
NOTE: The additional errors that are occuring in the move only object checker is
b/c I tweaked checkDestructureUsesOnBoundary so that when it detects an error it
continues instead of returns. This ensures that we get more that we emit errors
for multiple violations instead of just the first one.
rdar://104900171
The reason that I am doing this is I discovered that we emit worse quality
diagnostics since if we emit an error while running the borrow to destructure
transform, we want to do a complete cleanup to be safe (e.x.: converting
copy_value -> explicit_copy_value). This causes the object checker to then not
emit any additional diagnostics for other variables that were not impacted by
the BorrowToDestructureTransform, reducing the quality of diagnostics in a
significant way that isn't needed.
