`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
We're now double-diagnosing some things that are caught by both
SILGen and static enforcement; we can fix that later, but I want to
unblock this problem first.
The reason that this is being done is that:
1. SILGenFunction is passed around all throughout SILGen, including in between
APIs some of which call the SILGenFunction variable SGF and others that call it
gen.
2. Thus when one is debugging code in SILGen, one wastes time figuring out what
the variable name of SILGenFunction is in the current frame.
I did not do this by hand. I did this by:
1. Grepping for "SILGenFunction &gen".
2. By hand inspecting that the match was truly a SILGenFunction &gen site.
3. If so, use libclang tooling to rename the variable to SGF.
So I did not update any use sites.
Most of this involved sprinkling ValueOwnershipKind::Owned in many places. In
some of these places, I am sure I was too cavalier and I expect some of them to
be trivial. The verifier will help me to track those down.
On the other hand, I do expect there to be some places where we are willing to
accept guaranteed+trivial or owned+trivial. In those cases, I am going to
provide an aggregate ValueOwnershipKind that will then tell SILArgument that it
should disambiguate using the type. This will eliminate the ackwardness from
such code.
I am going to use a verifier to fix such cases.
This commit also begins the serialization of ValueOwnershipKind of arguments,
but does not implement parsing of value ownership kinds. That and undef are the
last places that we still use ValueOwnershipKind::Any.
rdar://29791263
We preserve the current behavior of assuming Any ownership always and use
default arguments to hide this change most of the time. There are asserts now in
the SILBasicBlock::{create,replace,insert}{PHI,Function}Argument to ensure that
the people can only create SILFunctionArguments in entry blocks and
SILPHIArguments in non-entry blocks. This will ensure that the code in tree
maintains the API distinction even if we are not using the full distinction in
between the two.
Once the verifier is finished being upstreamed, I am going to audit the
createPHIArgument cases for the proper ownership. This is b/c I will be able to
use the verifier to properly debug the code. At that point, I will also start
serializing/printing/parsing the ownershipkind of SILPHIArguments, but lets take
things one step at a time and move incrementally.
In the process, I also discovered a CSE bug. I am not sure how it ever worked.
Basically we replace an argument with a new argument type but return the uses of
the old argument to refer to the old argument instead of a new argument.
rdar://29671437
The RValue(ArrayRef<ManagedValue>, CanType) constructor was intended as a semi-private interface for building an RValue from a pre-exploded array of elements, but was (understandably) widely being misused as a general ManagedValue-to-RValue constructor, causing crashes when working with tuples in various contexts where RValue's methods expected them to be exploded. Make the constructor private and update most improper uses of it to use the exploding RValue constructor, or to use a new `RValue::withPreExplodedElements` static method that more explicitly communicates the intent of the constructor. Fixes rdar://problem/29500731.
