The return pointer may point into the materialized base value, so if the base needs
materialization, ensure that materialization covers any futher projection of the
value.
Although I don't plan to bring over new assertions wholesale
into the current qualification branch, it's entirely possible
that various minor changes in main will use the new assertions;
having this basic support in the release branch will simplify that.
(This is why I'm adding the includes as a separate pass from
rewriting the individual assertions)
In address-lowered mode, to initialize tuple-typed memory in a single
step, tuple_addr_constructor must generally be used because it's not
possible to construct a tuple any of whose fields are address-only. In
opaque values mode, there is no problem constructing such a tuple. So
construct the tuple and then assign it into the tuple-typed memory; the
single instruction that initializes the memory will be the assign.
I also included changes to the rest of the SIL optimizer pipeline to ensure that
the part of the optimizer pipeline before we lower tuple_addr_constructor (which
is right after we run TransferNonSendable) work as before.
The reason why I am doing this is that this ensures that diagnostic passes can
tell the difference in between:
```
x = (a, b, c)
```
and
```
x.0 = a
x.1 = b
x.2 = c
```
This is important for things like TransferNonSendable where assigning over the
entire tuple element is treated differently from if one were to initialize it in
pieces using projections.
rdar://117880194
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.
Variable debug info is triggered by pattern bindings, however, inside a closure
capture list, this should be avoided by setting the appropriate flag in the
initializer object.
rdar://110329894
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))
In-memory values cannot be reused after deinitialization.
Add ManagedValue::isPlusOneOrTrivial() for situations where we want to
forward a value without destroying the original memory.
Fixes rdar://108001491 (SIL verification failed: Found mutating or
consuming use of an in_guaranteed parameter?!:
!ImmutableAddressUseVerifier().isMutatingOrConsuming(fArg))
Arguably, it would be more consistent with the current design to
figure out a representation for pack-expansion components so that
we can keep them around in whatever storage. But honestly, I
don't like the current design; I think the eager explosion is an
over-complicated mistake. So this is partly just me finding an
excuse to not extend that to more cases.
`getValue` -> `value`
`getValueOr` -> `value_or`
`hasValue` -> `has_value`
`map` -> `transform`
The old API will be deprecated in the rebranch.
To avoid merge conflicts, use the new API already in the main branch.
rdar://102362022
This makes it easier to understand conceptually why a ValueOwnershipKind with
Any ownership is invalid and also allowed me to explicitly document the lattice
that relates ownership constraints/value ownership kinds.
This is a large patch; I couldn't split it up further while still
keeping things working. There are four things being changed at
once here:
- Places that call SILType::isAddressOnly()/isLoadable() now call
the SILFunction overload and not the SILModule one.
- SILFunction's overloads of getTypeLowering() and getLoweredType()
now pass the function's resilience expansion down, instead of
hardcoding ResilienceExpansion::Minimal.
- Various other places with '// FIXME: Expansion' now use a better
resilience expansion.
- A few tests were updated to reflect SILGen's improved code
generation, and some new tests are added to cover more code paths
that previously were uncovered and only manifested themselves as
standard library build failures while I was working on this change.
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
The index type might be a one-element tuple, either with a label
or a vararg element. We have to deal with these cases explicitly
since getLoweredType() doesn't.
Fixes <rdar://problem/43237821>.
ManagedValue::ensurePlusOne is already enabled.
I also tightened up the implementation a little bit by:
1. Fixed a typo in ManagedValue::isPlueZero(). I forgot to negate a boolean
=/. Luckily, I have been using isPlusOne() instead of isPlusZero for all queries
I have needed so far.
2. I added code to ManagedValue/RValue so that if we have SILUndef, we do not
emit copies on SILUndef when we perform ensurePlusOne().
3. I changed isPlusOne() and isPlusZero() to return true for SILUndef. SILUndef
has "any" ownership so it is compatible with all forms of ownership.
rdar://34222540
This was found by the ownership verifier in the tuple shuffle test. The specific
problem was that we were performing an ensurePlusOne and forwarding that
value. The rvalue code was expecting this to be a "true" forward. Rather than
relying on the rvalue code's assumptions, this commit refactors the tuple
implosion code to just return an explicit ManagedValue with the proper cleanup
upon it.
Bug caught by the ownership verifier running on the argument shuffle SILGen test
with +0 enabled.
*NOTE* This is not a problem on any of the release branches, since ensurePlusOne
is a no-op on those branches (I just turned it on a couple of days ago).
rdar://34222540
We already have this requirement on RValues, so it is natural to extend it to
ManagedValue. The main reason why I am doing this though is to tighten up SILGen
asserts to help me catch places where we forward +0 arguments into memory
without a copy/cleanup.
rdar://34222540
In the code, we assume generally that a value that is "forwarded" into memory is
forwarded at +1. This isn't enforced and when I tried to enforce it at +0, I
quickly hit assertion failures. So rather than fix the myriad places, I am using
ensurePlusOne to handle such cases without needing to change a bunch of the
code.
rdar://34222540
This helper function returns *this if the value is already at +1. Otherwise, if
the value is a +0 value, we copy the value. Since I am using this initially for
some experiments, I am hiding it behind the enable guaranteed normal arguments
flag.
rdar://34222540
I am currently changing Callee to use an ArgumentSource instead of a SILValue to
represent self. Due to uncurrying/etc in certain cases, we need to communicate
that a value needs to be borrowed later before use. To do that in a clean way, I
am introducing a new form of ArgumentSource::Kind, DelayedBorrowedRValue.
This type of ArgumentSource can only be produced by calling
ArgumentSource::delayedBorrow(...). Such an argument source acts like a normal
RValue, except when you perform asKnownRValue, a borrow is performed before
returning the known rvalue.
Once uncurrying is ripped out/redone, this code can be removed in favor of just
using a borrowed ArgumentSource.
rdar://33358110
Today, SILGenFunction::emitRValue assumes the caller will create any cleanup
scopes that are needed to cleanup side-effects relating to the rvalue
evaluation. The API also provides the ability for the caller to specify that a
+0 rvalues is an "ok" result. The API then tries to produce a +0 rvalue and
returns a +1 rvalue otherwise. These two properties create conflicting
requirements on the caller since the caller does not know whether or not it
should create a scope (if a +1 rvalue will be returned) or not (if a +0 rvalue
would be returned).
The key issue here is the optionality of returning a +0 rvalue. This change
begins to resolve this difference by creating two separate APIs that guarantee
to the caller whether or not a +0 or a +1 rvalue is returned and also creates
local scopes for the caller as appropriate. So by using these APIs, the caller
knows that the +0 or +1 rvalue that is returned has properly been put into the
caller scope. So the caller no longer needs to create its own scopes anymore.
emitPlusOneRValue is emitRValue except that it scopes the rvalue emission and
then *pushes* the produced rvalue through the scope. emitPlusZeroRValue is
currently a stub implementation that just calls emitPlusOneRValue and then
borrows the resulting +1 RValue in the outer scope, creating the +0 RValue that
was requested by the caller.
rdar://33358110
This is already an RValue invariant that used to be enforced upon RValue
construction. We put in a hack to work around a bug where that was not occuring
and changed RValue constructors to instead load stored objects when they needed
to. But the problem is that since then we have added more constructors that
provide other manners to create such an invalid RValue.
I added verification to many parts of RValue and exposed an additional verify
method that we can invoke at the end of emitRValue() eventually to verify our
invariants. This will give me the comfort to make that assumption in other parts
of SILGen without worry.
I also performed a small amount of cleanup of RValue construction.
rdar://33358110
This can only come up if you have a tuple that is part address only and part
non-trivial, but loadable. Previously, the non-trivial value would be taken but
no cleanup would be created. This would trip the ownership verifier. Now we
properly model this via a load_borrow.
*NOTE* Due to us modeling a take as not-copying a value, after ownership is
stripped we have the same underlying singular load, so this is not a bug, but a
semantic violation of the model (i.e. taking a +0 self value).
rdar://33358110
I already in a previous commit forced all rvalues to have consistent cleanups
and consistent value ownership kinds. Now that we know all constructed RValues
are consistent, we can safely query that information.
rdar://33358110
This means that all non-trivial ManagedValues must:
1. All have a cleanup or all not have a cleanup.
2. Have the same ValueOwnershipKind.
Semantic SIL requires this of tuple values. So it makes sense to catch this bug
as early as possible.
rdar://33358110
