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)
When a `borrowing` or `consuming` parameter is captured by a closure,
we emit references to the binding within the closure as if it is non-implicitly
copyable, but we didn't mark the bindings inside the closure for move-only
checking to ensure the uses were correct, so improper consumes would go
undiagnosed and lead to assertion failures, compiler crashes, and/or
miscompiles. Fixes rdar://127382105
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.
Closure cycles were originally enforced "conservatively". Real code
does, however, use recursive local functions. And it's surprisingly
easy to create false exclusivity violations in those cases.
This is safe as long as we can rely on SILGen to keep captured
variables in boxes if the capture escapes in any closure that may call
other closures that capture the same variable.
Fixes https://github.com/apple/swift/issues/61404
Dynamic exclusivity checking bug with nested functions. #61404
Fixes rdar://102056143 (assertion failure due to exclusivity checker -
AccessEnforcementSelection should handle recursive local captures)
This fixes a commit from a few days ago where I meant to call
unique_ptr::reset() instead of unique_ptr::release() and
apparently didn't notice the compiler warning.
Handle recursive non-escaping local functions.
Previously, it was thought that recursion would force a closure to be
escaping. This is not necessarilly true.
Update AccessEnforcementSelection to conservatively handle closure cycles.
Fixes rdar://88726092 (Compiler hangs when building)
As a follow-up to reviewing the dynamically replaceable
implementation, document the place where this analysis will likely
crash in the future once we start optimizing non-escaping closure
captures.
I believe that these were in SILInstruction for historic reasons. This is a
separate API on top of SILInstruction so it makes sense to pull it out into its
own header.
A directly applied noescape closure that captures a local requires
dynamic enforcement if that local escaped prior to the direct
application. Previously this was only statically enforced, which could
miss conflicts.
It's unlikely that an actual conflict could occur, but can happen in
cases like this:
func noescapeConflict() {
var localVal = 0
let nestedModify = { localVal = 3 }
_ = {
takeInoutAndPerform(&localVal, closure: nestedModify)
}()
}
Code isn't normally written like this, but the general pattern of
passing an escaping closure as an argument to a noescape closure may
arise when using withoutActuallyEscaping. Even in that case, it's
highly unlikely that a real conflict would exist.
For this case to be handled correcly, we must also never allow a
closure passed to withoutActuallyEscaping to be SILGen's as a
non-escaping closure. Doing so would result in a non-escaping closure
to be passed as an argument to the withoutActuallyEscaping trailing
closure, which is also non-escaping. That directly violates the
Non-Escaping Recursion Restriction, but there would be no way to
diagnose the violation, creating a hole in the exclusivity model.
This is a module pass, so was processing a combination of raw and deserialized
canonical SIL. The analysis makes structural assumptions that only hold prior to
mandatory inlining.
Add more verification of the structural properties.
Teach the pass to skip over canonical SIL, which only works because closures
must be serialized/deserialized in the same module as their parent.
Fixes <rdar://38042781> [Repl] crash while running SILOptimizer
Until other bugs are fixed, this is still required in the short term after
mandatory inlining of debug libraries into an optimized executable.
Also, enable rerunning access marker elimination on deserialized functions,
because theoretically, the same problem could occur if we emit an external
function without inlining it.
Previously, this asserted on unexpected situations that the
optimizer couldn't handle.
It makes more sense now to handle these cases conservatively since we can't
catch them in early testing.
Fixes <rdar://problem/35402799> [4.1] Assertion failed: (user->getResults().empty())
Support for @noescape SILFunctionTypes.
These are the underlying SIL changes necessary to implement the new
closure capture ABI.
Note: This includes a change to function name mangling that
primarily affects reabstraction thunks.
The new ABI will allow stack allocation of non-escaping closures as a
simple optimization.
The new ABI, and the stack allocation optimization, also require
closure context to be @guaranteed. That will be implemented as the
next step.
Many SIL passes pattern match partial_apply sequences. These all
needed to be fixed to handle the convert_function that SILGen now
emits. The conversion is now needed whenever a function declaration,
which has an escaping type, is passed into a @NoEscape argument.
In addition to supporting new SIL patterns, some optimizations like
inlining and SIL combine are now stronger which could perturb some
benchmark results.
These underlying SIL changes should be merged now to avoid conflicting
with other work. Minor benchmark discrepancies can be investigated as part of
the stack-allocation work.
* Add a noescape attribute to SILFunctionType.
And set this attribute correctly when lowering formal function types to SILFunctionTypes based on @escaping.
This will allow stack allocation of closures, and unblock a related ABI change.
* Flip the polarity on @noescape on SILFunctionType and clarify that
we don't default it.
* Emit withoutActuallyEscaping using a convert_function instruction.
It might be better to use a specialized instruction here, but I'll leave that up to Andy.
Andy: And I'll leave that to Arnold who is implementing SIL support for guaranteed ownership of thick function types.
* Fix SILGen and SIL Parsing.
* Fix the LoadableByAddress pass.
* Fix ClosureSpecializer.
* Fix performance inliner constant propagation.
* Fix the PartialApplyCombiner.
* Adjust SILFunctionType for thunks.
* Add mangling for @noescape/@escaping.
* Fix test cases for @noescape attribute, mangling, convert_function, etc.
* Fix exclusivity test cases.
* Fix AccessEnforcement.
* Fix SILCombine of convert_function -> apply.
* Fix ObjC bridging thunks.
* Various MandatoryInlining fixes.
* Fix SILCombine optimizeApplyOfConvertFunction.
* Fix more test cases after merging (again).
* Fix ClosureSpecializer. Hande convert_function cloning.
Be conservative when combining convert_function. Most of our code doesn't know
how to deal with function type mismatches yet.
* Fix MandatoryInlining.
Be conservative with function conversion. The inliner does not yet know how to
cast arguments or convert between throwing forms.
* Fix PartialApplyCombiner.
introduce a common superclass, SILNode.
This is in preparation for allowing instructions to have multiple
results. It is also a somewhat more elegant representation for
instructions that have zero results. Instructions that are known
to have exactly one result inherit from a class, SingleValueInstruction,
that subclasses both ValueBase and SILInstruction. Some care must be
taken when working with SILNode pointers and testing for equality;
please see the comment on SILNode for more information.
A number of SIL passes needed to be updated in order to handle this
new distinction between SIL values and SIL instructions.
Note that the SIL parser is now stricter about not trying to assign
a result value from an instruction (like 'return' or 'strong_retain')
that does not produce any.
This analysis has a whitelist to ensure that we aren't missing any SIL
patterns and failing to enforce some cases.
There is a special case involving nested non-escaping closures being passed as a
block argument. Whitelist this very special case even though we don't enforce
it because the corresponding diagnostics pass also doesn't enforce it.
This uses the new ClosureScopeAnalysis to process parent functions before the
closures they reference. The analysis now tracks captured variables in addition
to immediately accessed variables. If the variable escapes before it's capture
is applied, then that closure uses dynamic enforcement for the variable.
<rdar://problem/32061282> [Exclusivity] Enforcement selection for noescape closure captures.
In AccessEnforcementSelection, treat passing a projection from a box to
a partial_apply as an escape to force using dynamic enforcement on the box.
This will now correctly use dynamic enforcement on variables that are taken
as inout and also captured by storage address in a closure:
var x = ...
x.mutatingMethod { ... use x ...}
but does pessimize some of our existing enforcement into dynamic since
access enforcement selection.
Ideally we would distinguish between escaping via an nonescaping closures
(which can only conflict with accesses that are in progress) and
escaping via escaping closures (which can conflict for any reachable code
after the escape)
This pass doesn't do anything useful downstream yet, so it's safe to temporarily
disable the assert.
Reenable after fixing:
<rdar://problem/31797132> [Exclusivity] lldb asserts in AccessEnforcementSelector
