Skip to content

Commit 7d09aee

Browse files
committed
ClosureLifetimeFixup: Handle undef partial_apply arguments gracefully
We have to handle undef partial_apply arguments to handle the following source gracefully during the diagnosis pipeline. ``` class TestUndefined { private var stringList: [String]! func dontCrash(strings: [String]) { assert(stringList.allSatisfy({ $0 == stringList.first!})) let stringList = strings.filter({ $0 == "a" }) } } ``` rdar://57893008
1 parent 2025068 commit 7d09aee

File tree

3 files changed

+41
-2
lines changed

3 files changed

+41
-2
lines changed

lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,9 @@ static bool tryRewriteToPartialApplyStack(
458458
saveDeleteInst(convertOrPartialApply);
459459
saveDeleteInst(origPA);
460460

461+
// Only insert destroys for defined partial_apply arguments.
462+
auto isDefined = [](SILValue arg) -> bool { return !isa<SILUndef>(arg); };
463+
461464
// Insert destroys of arguments after the apply and the dealloc_stack.
462465
if (auto *apply = dyn_cast<ApplyInst>(singleApplyUser)) {
463466
auto insertPt = std::next(SILBasicBlock::iterator(apply));
@@ -466,12 +469,12 @@ static bool tryRewriteToPartialApplyStack(
466469
return true;
467470
SILBuilderWithScope b3(insertPt);
468471
b3.createDeallocStack(loc, newPA);
469-
insertDestroyOfCapturedArguments(newPA, b3);
472+
insertDestroyOfCapturedArguments(newPA, b3, isDefined);
470473
} else if (auto *tai = dyn_cast<TryApplyInst>(singleApplyUser)) {
471474
for (auto *succBB : tai->getSuccessorBlocks()) {
472475
SILBuilderWithScope b3(succBB->begin());
473476
b3.createDeallocStack(loc, newPA);
474-
insertDestroyOfCapturedArguments(newPA, b3);
477+
insertDestroyOfCapturedArguments(newPA, b3, isDefined);
475478
}
476479
} else {
477480
llvm_unreachable("Unknown FullApplySite instruction kind");

test/SILOptimizer/closure-lifetime-fixup.sil

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,3 +155,26 @@ bb1:
155155
%86 = tuple ()
156156
return %86 : $()
157157
}
158+
159+
sil [ossa] @closureImpl : $@convention(thin) (@guaranteed Klass, @guaranteed Klass) -> Bool
160+
sil [ossa] @useClosure : $@convention(thin) (@noescape @callee_guaranteed () -> Bool) -> ()
161+
162+
// Don't crash.
163+
// CHECK-LABEL: sil hidden [ossa] @testUndefined
164+
// CHECK: [[PA:%.*]] = partial_apply [callee_guaranteed] [on_stack]
165+
// CHECK: mark_dependence
166+
// CHECK: mark_dependence
167+
// CHECK: dealloc_stack [[PA]] : $@noescape @callee_guaranteed () -> Bool
168+
// CHECK: destroy_value
169+
sil hidden [ossa] @testUndefined : $@convention(method) (@guaranteed Klass, @guaranteed Klass) -> () {
170+
bb0(%0 : @guaranteed $Klass, %1 : @guaranteed $Klass):
171+
%4 = function_ref @closureImpl : $@convention(thin) (@guaranteed Klass, @guaranteed Klass) -> Bool
172+
%5 = copy_value %1 : $Klass
173+
%6 = partial_apply [callee_guaranteed] %4(%5, undef) : $@convention(thin) (@guaranteed Klass, @guaranteed Klass) -> Bool
174+
%7 = convert_escape_to_noescape [not_guaranteed] %6 : $@callee_guaranteed () -> Bool to $@noescape @callee_guaranteed () -> Bool
175+
%21 = function_ref @useClosure : $@convention(thin) (@noescape @callee_guaranteed () -> Bool) -> ()
176+
%22 = apply %21(%7) : $@convention(thin) (@noescape @callee_guaranteed () -> Bool) -> ()
177+
destroy_value %6 : $@callee_guaranteed () -> Bool
178+
%42 = tuple ()
179+
return %42 : $()
180+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: not %target-swift-frontend %s -sil-verify-all -c 2>&1 | %FileCheck %s
2+
3+
// Report the error but don't crash.
4+
// CHECK: error: closure captures 'stringList' before it is declared
5+
6+
class TestUndefined {
7+
private var stringList: [String]!
8+
9+
func dontCrash(strings: [String]) {
10+
assert(stringList.allSatisfy({ $0 == stringList.first!}))
11+
let stringList = strings.filter({ $0 == "a" })
12+
}
13+
}

0 commit comments

Comments
 (0)