Previously we were handling this like we did not have any associated type. This
caused us to break the requirement in [ossa] that all switch_enum successors
associated with payloaded enums must have arguments. We still emit the empty
tuple value (or an undef tuple) if we do not have any associated type.
The ownership kind is Any for trivial types, or Owned otherwise, but
whether a type is trivial or not will soon depend on the resilience
expansion.
This means that a SILModule now uniques two SILUndefs per type instead
of one, and serialization uses two distinct sentinel IDs for this
purpose as well.
For now, the resilience expansion is not actually used here, so this
change is NFC, other than changing the module format.
There was a bug here where we were emitting a shared case if we had arguments.
That is why we needed the hacked in deletion of that block when we emitted
shared blocks. I was able to replace that with an assert to make sure that we do
not regress.
This additionally improved our code generation by eliminating a potential extra
copy when we had such a case. That is where the test updates come from.
NFC unless -enable-verify-exclusivity is on.
Boxed (indirect) enum loads are not formal accesses so they should not
have access markers. For a while, we used UnenforcedAccess in this
case to signal to the verifier that this is actually a safe load. But
that isn't needed. The verifier just ignores loads from unidentified
storage instead.
There are two code paths that had this UnenforcedAccess marker. One
was cleaned up, and the other was not. So finish the cleanup and add a
unit test for both SILGen code paths.
Today SILGenPattern maintains the following invariants:
1. All values passed into a switch must be either TakeAlways or BorrowAlways and
loadable input values will be loaded.
2. Loadable types passed to a subtree must be loaded.
3. TakeOnSuccess can only occur with address only types and only in subtrees.
4. A TakeOnSuccess value must go through "fake borrowing"
(i.e. forward/unforwarding) to ensure that along failing cases, we properly
re-enable the cleanup on the aggregate. This means that TakeOnSuccess can
never be handled as a loadable value with ownership enabled and that any
take_on_success value since the original cleanup on the parent value was
disabled must be at +1.
5. We use BorrowAlways instead of TakeOnSuccess for objects to express the
notion that the object is not owned by the sub-tree.
The bug in this tuple code occured at the a place in the code where we go from
an address only parent type to a loadable subtype via TakeOnSuccess. I was
trying to follow (5) and thus I converted the subvalue to use a {load_borrow,
BorrowAlways}. The problem with this is that I lost the cleanup from the tuple
subvalue since take_on_success is just simulating +0 and thus entails having a
cleanup for each of the underlying tuple values.
The fix here was to:
* Create a cleanup for the loadable subvalue leaving it in memory. This address
version of the subvalue we use for the purposes of unforwarding. This avoids
(4).
* load_borrow the subvalue and pass that off to the subtree. This ensures that
we respect (2), (3), (4).
* Unforward in the failure case the in memory subvalue.
This gives us both characteristics as well as in the future the possibility of
enforcing via the ownership verifier that the borrow ends before the
destroy_addr.
I also sprinkled some assertions liberally to maintain invariants.
rdar://47034816
Removes the _getBuiltinLogicValue intrinsic in favor of an open-coded
struct_extract in SIL. This removes Sema's last non-literal use of builtin
integer types and unblocks a bunch of cleanup.
This patch would be NFC, but it improves line information for conditional expression codegen.
In a previous commit, I banned in the verifier any SILValue from producing
ValueOwnershipKind::Any in preparation for this.
This change arises out of discussions in between John, Andy, and I around
ValueOwnershipKind::Trivial. The specific realization was that this ownership
kind was an unnecessary conflation of the a type system idea (triviality) with
an ownership idea (@any, an ownership kind that is compatible with any other
ownership kind at value merge points and can only create). This caused the
ownership model to have to contort to handle the non-payloaded or trivial cases
of non-trivial enums. This is unnecessary if we just eliminate the any case and
in the verifier separately verify that trivial => @any (notice that we do not
verify that @any => trivial).
NOTE: This is technically an NFC intended change since I am just replacing
Trivial with Any. That is why if you look at the tests you will see that I
actually did not need to update anything except removing some @trivial ownership
since @any ownership is represented without writing @any in the parsed sil.
rdar://46294760
This also exposed a bug where we use a lowered type as an AST type and
crash when emitting the '@unknown case' block. This block is unreachable
when switching over non-enum values, but its emitted anyway.
Fixes <https://bugs.swift.org/browse/SR-9159>, <rdar://problem/45962466>.
This commit allows the initial switch subject value to be emitted at +0 if we
can emit it that way. As you can imagine since we have +0 normal function
arguments this should tighten up a lot of code around switches on arguments. So
I got to delete a bunch of code in the tests. = ).
Some things to note:
1. If the switch is given a +1 value, we will still try to let it through at +1
until we hit a part of the decision tree where previously we would need to use
TakeOnSuccess. This means that +1 values that go through irrefutable patterns
like tuple splitting should still be emitted at +1.
2. If we are returned an address only type without a cleanup, we copy it and
pass it down at +1 like the old code.
3. I also elided the last ownership check in SILGenPattern in this commit. After
this, there is only 1 specialization for ownership in the swift compiler that is
in Apply emission. Thats my next target.
rdar://29791263
The `Stmt` and `Expr` classes had both `dump` and `print` methods that behaved similarly, making it unclear what each method was for. Following a conversation in https://forums.swift.org/t/unifying-printing-logic-in-astdumper/15995/6 the `dump` methods will be used to print the S-Expression-like ASTs, and the `print` methods will be used to print the more textual ASTPrinter-based representations. The `Stmt` and `Expr` classes seem to be where this distinction was more ambiguous. These changes should fix that ambiguity.
A few other classes also have `print` methods used to print straightforward representations that are neither the S-Expressions nor ASTPrinters. These were left as they are, as they don't cause the same ambiguity.
It should be noted that the ASTPrinter implementations themselves haven't yet been finished and aren't a part of these changes.
Fixes SR-8933.
Swift's uninhabited checking is conservative. An enum might not be found to be uninhabited, but all of its cases might. In that case, the switch can be empty even though the type isn't known to be uninhabited.
Fix an assertion that assumed there would always be at least one case for types that aren't known to be uninhabited.
The problem here was that if we had a tuple with mixed address only and loadable
components and we used take on success during pattern matching, we would load
the loadable component with a load [take] and then pass it as take on success.
Instead we load the loadable components of the tuple using a load_borrow and
make the cast consumption kind BorrowAlways.
This can not happen with enum element emission since enum element emission does
not support take_on_success so the problem can not occur there.
SR-9029
rdar://45345844
This is the enum element analogue of the tuple fixup in:
359eda52e5. Additionally as a nice fixup I can now
enable ownership verification on most of the switch code.
I ran into commit ordering issues with cleaning up the address only part of the
emitEnumElement so I included it in this commit. Specifically this was because I
realized that it was possible to get a copy_on_success with an object from this
code and I wanted to enforce the invariant that ConsumptionManagedValues only
have copy_on_success with addresses and borrow_always with objects. It was less
convoluted to just fix the address only code to fit that formulation rather than
shoe-horn the old code into the new form.
rdar://29791263
To do this the commit does a few things:
1. If we enter the tuple dispatch code with an address that is loadable, we
always immediately perform a load_borrow and change the consumable managed value
to BorrowAlways.
2. If we have a take_on_success object, we immediately borrow. We do not want to
do with TakeOnSuccess since we want to define away the need to unforward since
unforwarding recreates a destroy on the already invalidated parent tuple object.
Notice that this code still handles TakeAlways so in simple cases where we have
a +1 value, everything still works.
Since emitTupleDispatchWithOwnership now handles only objects, I renamed it to
emitTupleObjectDispatch.
rdar://29791263
This is really old code that I put in when I was still understanding
SILGenPattern. If we are emitting code in ownership form we will never have a
TakeOnSuccess consumption kind, so we should not forward in that case
"automagically".
rdar://29791263
The reason why the asserts were bogus is that even with SILOwnership enabled, we
will use the old pattern to emit address only code. In that case, the asserts
will fire on good behavior.
I also fixed a latent bug where when ownership was enabled we were treating
address only values like objects. Found via inspection.
rdar://29791263
This means that:
1. SILGenPattern always borrows the object before it emits a case.
2. Any cast with this cast has a +0 result.
NOTE: That one can not use this with address types (so we assert if you
pass this checked_cast_addr_br).
NOTE: Once we have opaque values, checked_cast_br of a guaranteed value will
lower to a copy + checked_cast_addr_br (assuming the operation is a consuming
cast). To make sure this does not become a problem in terms of performance, we
will need a pass that can transform SILGenPattern +0 cases to +1 cases. This is
something that we have talked about in the past and I think it is reasonable to
implement.
This is an incremental commit towards fixing SILGenPattern for ownership.
rdar://29791263
