This will ensure that the optimizer never eliminates the strong_retain. This
operation is meant to be unmanaged, we should respect the user's choice here
even in optimized builds.
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.
This is necessary since our func may want to emit conditional code with an early
exit, emitting unused cleanups from the current scope via the function
emitBranchAndCleanups(). If we have not yet created those cleanups, we will
introduce a leak along that path.
rdar://49990484
This prevents ManagedValue APIs on SILGenBuilder from by mistake accesing
non-conformance tracking APIs on SILBuilder.
NOTE: Eventually, we should make SILGenBuilder compose with SILGenSILBuilder and
then rename SILGenBuilder to ManagedValueBuilder. I did not do that in this PR
since it becomes very disruptive since there is still a lot of code in SILGen
that uses SILValue APIs and I have not found a concise way to write such
code. But this patch at least defines away this error which has bitten us
before.
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
Dynamic replacements are currently written in extensions as
extension ExtendedType {
@_dynamicReplacement(for: replacedFun())
func replacement() { }
}
The runtime implementation allows an implementation in the future where
dynamic replacements are gather in a scope and can be dynamically
enabled and disabled.
For example:
dynamic_extension_scope CollectionOfReplacements {
extension ExtentedType {
func replacedFun() {}
}
extension ExtentedType2 {
func replacedFun() {}
}
}
CollectionOfReplacements.enable()
CollectionOfReplacements.disable()
To make that work, enter appropriate scopes (ArgumentScopes and
FormalEvaluationScopes) at a bunch of places. But note that l-value
emission generally can't enter such a scope, so in generic routines
like emitOpenExistentialExpr we have to just assert that we're
already in a scope.
The problem here is that we were taking advantage of swift not-trashing the
original memory location after performing a load [take] to use load [take] as a
+0 load. This breaks the ownership verifier since the load [take] is never
actually destroyed. Instead this commit changes this code to use a load_borrow.
I found this when trying to enable ownership verification on
test/SILGen/indirect_enum.swift using some out of tree work that should have
fixed all of the ownership issues with SILGenPattern. Turns out I can just
enable ownership verification with just this change... so I did that as well in
this PR.
rdar://29791263
ConvertFunction and reabstraction thunks need this attribute. Otherwise,
there is no way to identify that withoutActuallyEscaping was used
to explicitly perform a conversion.
The destination of a [without_actually_escaping] conversion always has
an escaping function type. The source may have either an escaping or
@noescape function type. The conversion itself may be a nop, and there
is nothing distinctive about it. The thing that is special about these
conversions is that the source function type may have unboxed
captures. i.e. they have @inout_aliasable parameters. Exclusivity
requires that the compiler enforce a SIL data flow invariant that
nonescaping closures with unboxed captures can never be stored or
passed as an @escaping function argument. Adding this attribute allows
the compiler to enforce the invariant in general with an escape hatch
for withoutActuallyEscaping.
For now, the accessors have been underscored as `_read` and `_modify`.
I'll prepare an evolution proposal for this feature which should allow
us to remove the underscores or, y'know, rename them to `purple` and
`lettuce`.
`_read` accessors do not make any effort yet to avoid copying the
value being yielded. I'll work on it in follow-up patches.
Opaque accesses to properties and subscripts defined with `_modify`
accessors will use an inefficient `materializeForSet` pattern that
materializes the value to a temporary instead of accessing it in-place.
That will be fixed by migrating to `modify` over `materializeForSet`,
which is next up after the `read` optimizations.
SIL ownership verification doesn't pass yet for the test cases here
because of a general fault in SILGen where borrows can outlive their
borrowed value due to being cleaned up on the general cleanup stack
when the borrowed value is cleaned up on the formal-access stack.
Michael, Andy, and I discussed various ways to fix this, but it seems
clear to me that it's not in any way specific to coroutine accesses.
rdar://35399664
Introduced during the bring-up of the generics system in July, 2012,
Substitution (and SubstitutionList) has been completely superseded by
SubstitutionMap. R.I.P.
closure lifetimes.
SILGen will now unconditionally emit
%cvt = convert_escape_to_noescape [guaranteed] %op
instructions. The mandatory ClosureLifetimeFixup pass ensures that %op's
lifetime spans %cvt's uses.
The code in DefiniteInitialization that handled a subset of cases is
removed.
To mark when a user of it is known to escape the value. This happens
with materializeForSet arguments which are captured and used in the
write-back. This means we need to keep the context alive until after
the write-back.
Follow-up patches to fully replace the PostponedCleanup hack in SILGen
by a mandatory SIL transformation pass to guarantee the proper lifetime
will use this flag to be more conservative when extending the lifetime.
The problem:
%pa = partial_apply %f(%some_context)
%cvt = convert_escape_to_noescape [not_guaranteed] [escaped] %pa
%ptr = %materialize_for_set(..., %cvt)
... write_back
... // <-- %pa needs to be alive until after write_back
The main change here is that we no longer forward ownership through
unchecked_bitwise_cast. The reason why is that unchecked_bitwise_cast creates a
new "reference count" identity to the optimizer. This is reflected in the SIL
ownership model by unchecked_bitwise_cast returning a value with unowned
ownership. We really do not want to traffic in such values, so we copy the value
after the cast to ensure that result is valid.
Found while updating SILGen tests for +0.
rdar://34222540
This method was not distinguishing in between in_guaranteed and in
parameters. This would cause the curry thunk where this is used to not copy
in_guaranteed parameters before passing in the parameter to the
partial_apply. This can not affect +1 code since the curry thunk will always
have self at +1.
I also refactored code in:
1. SILGenPoly.
2. SILGenProlog.
3. SILGenConstructor.
to use this function instead of their own reimplementations of the same thing.
This should be NFC for +1 code and is tested by test updates when +0 is enabled.
rdar://34222540
Previously SwitchEnumBuilder was in SILGenBuilder.{h,cpp} and
SwitchEnumCaseFullExpr was in its own file. This really made no sense since:
1. The two classes are inherently related to each other so really should be
together in the source base.
2. SwitchEnumBuilder uses a SILGenBuilder, but is really a separate independent
concept/entity, so there really is no reason to keep it in SILGenBuilder.cpp.
Just a quick fix to eliminate something that was bugging me.
NFC.
This would cause us to not properly create cleanups on tuples, causing potential
memory leaks. Luckily, I verified that we previously did not call this anywhere
in the compiler by removing the method in question from SILGenBuilder and
performing a successful build of swift. So we do not need to worry about this
bug on 4.1/5.0. I think I added it just for completeness since we had other such
ManagedValue methods.
Caught by the sil ownership verifier while running the argument shuffle test
with +0 rvalues and the commit merged with this commit.
rdar://34222540
This eliminates a case where we were not moving a cleanup onto a forwarding case
and instead just jammed the original cleanup and the new value into 1 managed
value.
NFC, but helps the ownership verifier.
rdar://34222540
I was thinking about this last night. I think the reason that this difference
came up is that people saw the SILGenBuilder SILValue based conformance APIs and
copied their calling of SILBuilder SILValue APIs. In the general case this is
not safe, since in some cases we need to add conformances.
Noticed by inspection.
This commit does two different things:
1. Changes some code paths in SILGenConvert to use SILGenBuilder APIs.
2. It eliminates a few bugs in this path exposed by running the SILGen tests
with +0 enabled.
The bug specifically is that we are assuming in this code that the input value
is always at +1, so we unconditionally forward the input value and then create
destroys on the output. This is not correct in general (since we have guaranteed
values) and occurs all the time with +0 arguments enabled.
This is tested by running the SILGen tests with +0 arguments enabled.
rdar://34222540
