Type annotations for instruction operands are omitted, e.g.
```
%3 = struct $S(%1, %2)
```
Operand types are redundant anyway and were only used for sanity checking in the SIL parser.
But: operand types _are_ printed if the definition of the operand value was not printed yet.
This happens:
* if the block with the definition appears after the block where the operand's instruction is located
* if a block or instruction is printed in isolation, e.g. in a debugger
The old behavior can be restored with `-Xllvm -sil-print-types`.
This option is added to many existing test files which check for operand types in their check-lines.
- While an opaque borrow access occurs to part of a value, the entire scope of
the access needs to be treated as a liveness range, so add the `EndAccess`es
to the liveness range.
- The SIL verifier may crash the compiler on SILGen-generated code when the
developer's source contains consume-during-borrow code patterns. Allow
`load_borrow` instructions to be marked `[unchecked]`, which suppresses
verifier checks until the move checker runs and gets a chance to properly
diagnose these errors.
Fixes rdar://124360175.
It works well enough now that it should be an acceptable replacement for both
borrowing and consuming switches that works in more correct situations than the
previous implementation. This does however expose a few known issues that I'll
try to fix in follow ups:
- overconsumes cause verifier errors instead of raising diagnostics (rdar://125381446)
- cases with multiple pattern labels aren't yet supported (rdar://125188955)
- copyable types with the `borrowing` or `consuming` modifiers should probably use
noncopyable pattern matching.
The `BorrowingSwitch` flag is still necessary to enable the surface-level syntax
changes (switches without `consume` and the `_borrowing` modifier, for instance).
Following https://github.com/apple/swift/pull/70333, do the same thing for
modify coroutines, marking the result so that we check uses of the result to
ensure it isn't consumed (without being reinitialized).
Mark the result of starting a read coroutine to be checked by the move-only checker, and then
update the pattern matching in the move checker itself so that it recognizes code patterns
involving yielding from and receiving yields from read coroutines. Teach move only diagnostics
to get the property name for an access through a read coroutine from the referenced declaration.
An immutable noncopyable capture borrows the captured value in-place and can't do anything
to modify it, and the may_assign_but_not_consume checking behaves badly with some code patterns
generated for resilient types when `self` is captured during a deinit. This change allows for
more accurate checking and fixes rdar://118427997.
I was originally hoping to reuse mark_must_check for multiple types of checkers.
In practice, this is not what happened... so giving it a name specifically to do
with non copyable types makes more sense and makes the code clearer.
Just a pure rename.
Also, the store_borrow work in the previous patch caused some additional issues
to crop up. I fixed them in this PR and added some tests in the process.
This prevents another type of copy of noncopyable value error.
I also as a small change, changed the tuple version to use a formal access
temporary since we are projecting a component out implying that the lifetime of
the temporary must end within the formal access. Otherwise, we cause the
lifetime of the temporary to outlive the access. This can be seen in the change
to read_accessor.swift where we used to extend the lifetime of the destroy_addr
outside of the coroutine access we are performing.
We want these to be borrowed in most cases and to create an appropriate onion
wrapping. Since we are doing this in more cases now, we fix a bunch of cases
where we used to be forced to insert a copy since a coroutine or access would
end too early.
The reason why we are doing this is that this combination of read/set forces the
compiler to emit a copy if we want to emit a modify operation.
The reason why we are forced to emit such a copy is that:
1. _read provides a guaranteed value in memory
2. performing a modify requires an owned value in memory.
This together implies that the only way we can do this is to copy from the _read
into temporary memory. But we have a noncopyable type so we can't do this.
rdar://112915525
The problem here is that the logic was conditionalized on all noncopyable
parameters that are borrowed as having the ValueOwnership::Shared flag set. This
is only true for user written parameters. Implicit noncopyable parameters like
self do not have ValueOwnership::Shared set upon them. We could potentially do
that in Sema, but Sema does not know what the proper convention of self is since
that information is in TypeLowering today.
With that in mind, conditionalize the logic here so we do the right thing.
rdar://112547982
this was staged in as a warning initially but it was intended to be
an error if it is not written so that we can move to a world where
these pattern matches are done as a borrowing operation instead.
rdar://110908714
Vars of such types should be given lexical `alloc_stack`s by
`AllocBoxToStack` which requires that the `alloc_box` insts formed for
them have an associated borrow scope which in turn requires that type
lowering for move only structs and enums have their lexical bits set.
rdar://110901430
The form of the AST changes slightly when a type has a read and a modify.
Specifically, we now have a load on the subscript and an inout_expr on the base.
I dealt with this by making the inout_expr something that when we look for
storage we look through and by tweaking the load lookthrough code.
Most of the time SILGen already emits these correctly without having extra
copies, but in certain situations SILGen will emit copies that we need the move
checker to eliminate (e.x.: when we generate a yield). An additional benefit is
that this also will catch places where the frontend makes a mistake.
This also removes a bunch of "copy of noncopyable" types error that showed up in
the implicit compiler generated modify.
I also added a bunch of tests that showed the behavior of subscripts/other accessors with the following combinations of semantics:
1. get only.
2. get/set.
3. get/modify.
4. read/set.
5. read/modify.
rdar://109746476
NOTE: This does not affect normal parameters since normal parameters that are
noncopyable never have default access semantics since the user is forced to
specify either borrow or consume. This is incontrast to implicit parameters like
the newValue of a setter.
rdar://109726282
Specifically, I changed emitRValueForDecl and SILGenProlog to do the right
thing. I also added some tests.
Some notes:
1. We currently consider using a copyable field of a move only address type to
be a consume of that type. I am going to fix that in the next commit to make it
easier to understand.
2. I am going to need to write more tests/flesh out the behavior more. I am sure
there is more here.
rdar://105106470
These are the same semantically, just the mangling is slightly different. The
benefit of doing this is that we are actually testing what we expect our users
to do.
rdar://108511703
The reason to do this is that:
1. Otherwise, we do not emit markers when someone attempts to consume the let.
We need the no_consume_or_assign to be there.
2. We need to insert assign_but_not_consuming so that DI can properly check lets
that are conditionally initialized and convert them to
initable_but_not_consuming.
I included a full definite_init SIL test that validates that we get the correct
codegen after DI in this case and emit the appropriate error as well.
rdar://108511534
The reason why I am doing this is that otherwise if one has a function that
takes both a guaranteed and an owned parameter, we will break OSSA invariants
since the load [take] will invalidate the load_borrow. So instead, we put in a
load_borrow knowing that the move checker will convert it to a load_borrow
assuming that the two pass exclusivity checking.
NOTE: Because of some missing functionality in subsequent tests, I had to
disable one test (moveonly_escaping_definite_initialization.swift) and also add
some checks for copy of noncopyable object errors. They will go away in the next
2 commits.
rdar://108510987
I ran into this while fixing the parent commit when attempting to add the
interpreter test in this commit into the aforementioned parent commit.
rdar://107974302
Code can only locally interact with a mutable memory location within a
formal access, and is only responsible for maintaining its invariants
during that access, so the move-only address checker does not need to,
and should not, observe operations that occur outside of the access
marked with the `mark_must_check` instruction. And for immutable
memory locations, although there are no explicit formal accesses, that's
because every access must be read-only, so although individual
accesses are not delimited, they are all compatible as far as
move-only checking is concerned. So we can back out the changes to SILGen
to re-project a memory location from its origin on every access, a
change which breaks invariants assumed by other SIL passes.
the main things still left behind the experimental flag(s) are
- move-only classes (guarded by MoveOnlyClasses feature)
- noimplicitcopy
- the _borrow operator
I also slightly changed the codegen around where we insert the mark_must_check.
Specifically, before we would emit the mark_must_check directly on the
ref_element_addr and then insert the access. This had the unfortunate effect
that we would hoist any destroy_addr that were actually needed out of the access
scope. Rather than do that, I now insert the mark_must_check on the access
itself. This results in the destroy_addr being within the scope (like the
mark_must_check itself).
rdar://105910066
Note that the changes from `load_borrow` to
`load [copy]` are for copyable types at the end
of an access chain. So far this patch just makes
more noncopyable things get borrowed when accessed,
like globals, which is what we want.
Some notes:
1. This ensures that if we capture them, we just capture the box by reference.
2. We are still using the old incorrect semantics for captures. I am doing this
so I can bring this up in separate easy to understand patches all of which
pass all of the moveonly tests.
3. Most of the test edits are due to small differences in error messages in
between the object and address checker.
4. I had to add a little support to the move only address checker for a small
pattern that doesn't occur with vars but do es occur for lets when we codegen
like this, specifically around enums. The pattern is we perform a load_borrow
and then copy_value and then use the result of the copy_value. Rather than fight
SILGen pattern I introduced a small canonicalization into the address checker which
transforms that pattern into a load [copy] + begin_borrow to restore the codegen
to a pattern the checker expects.
5. I left noimplicitcopy alone for now. But we should come back around and fix
it in a similar way. I just did not have time to do so.
This is the first slice of bringing up escaping closure support. The support is
based around introducing a new type of SILGen VarLoc: a VarLoc with a box and
without a value. Because the VarLoc only has a box, we have to in SILGen always
eagerly reproject out the address from the box. The reason why I am doing this
is that it makes it easy for the move checker to distinguish in between
different accesses to the box that we want to check separately. As such every
time that we open the box, we insert a mark_must_check
[assignable_but_not_consumable] on that project. If allocbox_to_stack manages to
determine that the box can be stack allocated, we eliminate all of the
mark_must_check and place a new mark_must_check [consumable_and_assignable] on
the alloc_stack. The end result is that we get the old model that we had before
and also can support escaping closures.
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.
