This is a futile attempt to discourage future use of getType() by
giving it a "scary" name.
We want people to use getInterfaceType() like with the other decl kinds.
When a value's preferred existential representation is opaque, if the
value is not an address (which can only happen in opaque values mode),
emit open_existential_value rather than open_existential_addr.
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.
When setting a value at a key-path, in opaque values mode, don't create
a temporary and store the new value into it. That is necessary when
using lowered addresses because intrinsic's signature is
```
sil @swift_setAtWritableKeyPath : $@convention(thin) <τ_0_0, τ_0_1> (@inout τ_0_0, @guaranteed WritableKeyPath<τ_0_0, τ_0_1>, @in τ_0_1) -> ()
```
but is incorrect in opaque values mode where values are passed directly
to `@in` parameters.
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.
Instead of taking a setter type, let's switch over to a more general
`AccessorKind` which allows us to cover init accessors and simplify
`emitApplySetterToBase`.
This is a preliminary step towards enabling default initialization of
init accessor properties in user-defined initializers because this logic
would have to be shared by multiple places during SILGen.
Without this, we emit a copy of noncopyable type error since we do not insert a
mark_must_check on lazily initialized global initializers.
rdar://111402912
`Initialization` is stateful and not meant to be emitted into multiple times across different contexts.
If emitting into an initialization causes it to be split or aborted, that will carry over into
further uses of the initialization. This was happening during `if` and `switch` expression
emission, leading to miscompiles or compiler crashes. Fix this by saving only the buffer when
we prepare emission for a statement expression, and creating the initialization in the scope
where the expression for a branch actually gets emitted. Fixes rdar://112213253.
Implement "init accessor" component emission which is paired with
"init accessor" write strategy. Use `SILUndef` for a "setter" operand
of an "assign_or_init" instruction in cases when property with init
accessor doesn't have a setter. DI would detect re-initialization
attempts to produce diagnostics.
Instead of dealing with substitutions during raw SIL lowering,
let's produce a partial apply without argument to produce a
substituted reference that could be used by SILVerifier and
raw SIL lowering stages.
Reformatting everything now that we have `llvm` namespaces. I've
separated this from the main commit to help manage merge-conflicts and
for making it a bit easier to read the mega-patch.
This is phase-1 of switching from llvm::Optional to std::optional in the
next rebranch. llvm::Optional was removed from upstream LLVM, so we need
to migrate off rather soon. On Darwin, std::optional, and llvm::Optional
have the same layout, so we don't need to be as concerned about ABI
beyond the name mangling. `llvm::Optional` is only returned from one
function in
```
getStandardTypeSubst(StringRef TypeName,
bool allowConcurrencyManglings);
```
It's the return value, so it should not impact the mangling of the
function, and the layout is the same as `std::optional`, so it should be
mostly okay. This function doesn't appear to have users, and the ABI was
already broken 2 years ago for concurrency and no one seemed to notice
so this should be "okay".
I'm doing the migration incrementally so that folks working on main can
cherry-pick back to the release/5.9 branch. Once 5.9 is done and locked
away, then we can go through and finish the replacement. Since `None`
and `Optional` show up in contexts where they are not `llvm::None` and
`llvm::Optional`, I'm preparing the work now by going through and
removing the namespace unwrapping and making the `llvm` namespace
explicit. This should make it fairly mechanical to go through and
replace llvm::Optional with std::optional, and llvm::None with
std::nullopt. It's also a change that can be brought onto the
release/5.9 with minimal impact. This should be an NFC change.
We can probably avoid this copy in more circumstances, but make the change only for
noncopyable types for now, since that's the case where it's most semantically apparent.
rdar://109161396
A reference to an instance property in init accessor body
has to be remapped into an argument reference because all
of the properties from initialized/accesses lists are passed
to init accessors individually via arguments.
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.
Some notes:
1. I implemented this as a contextual keyword that can only apply directly to
lvalues. This ensures that we can still call functions called copy, define
variables named copy, etc. I added tests for both the c++ and swift-syntax based
parsers to validate this. So there shouldn't be any source breaks.
2. I did a little bit of type checker work to ensure that we do not treat
copy_expr's result as an lvalue. Otherwise, one could call mutating functions on
it or assign to it, which we do not want since the result of copy_value is
3. As expected, by creating a specific expr, I was able to have much greater
control of the SILGen codegen and thus eliminate extraneous copies and other
weirdness than if we used a function and had to go through SILGenApply.
rdar://101862423
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
Whenever we want to forward to a +1 value but don't need to destroy
the original memory, use isPlusOneOrTrivial.
This follows the existing naming scheme.
Fixes rdar://108001491 (SIL verification failed: Found mutating or
consuming use of an in_guaranteed parameter?!:
!ImmutableAddressUseVerifier().isMutatingOrConsuming(fArg))
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
Normally, if we project from a mutable class ivar or global variable, we'll
load a copy in a tight access scope and then project from the copy, in order to
minimize the potential for exclusivity violations while working with classes and
copyable values. However, this is undesirable when a value is move-only, since
copying is not allowed; borrowing the value in place is the expected and only possible
behavior. rdar://105794506
This patch replaces the stateful generation of SILScope information in
SILGenFunction with data derived from the ASTScope hierarchy, which should be
100% in sync with the scopes needed for local variables. The goal is to
eliminate the surprising effects that the stack of cleanup operations can have
on the current state of SILBuilder leading to a fully deterministic (in the
sense of: predictible by a human) association of SILDebugScopes with
SILInstructions. The patch also eliminates the need to many workarounds. There
are still some accomodations for several Sema transformation passes such as
ResultBuilders, which don't correctly update the source locations when moving
around nodes. If these were implemented as macros, this problem would disappear.
This necessary rewrite of the macro scope handling included in this patch also
adds proper support nested macro expansions.
This fixes
rdar://88274783
and either fixes or at least partially addresses the following:
rdar://89252827
rdar://105186946
rdar://105757810
rdar://105997826
rdar://105102288
Fixes a host of problems with stored type members with types dependent
on a type parameter pack; I ran into it specifically with vanishing
tuples.
Test to follow.
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.
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
The subexpression of a MaterializePackExpr (which is always a tuple value
currently) is emitted while preparing to emit a pack expansion expr, and its
elements are projected from within the dynamic pack loop. This means that a
materialized pack is only evaluated once, rather than being evaluated on
every iteration over the pack elements.
