Skip to content

Commit 92b4185

Browse files
zoecarverzoecarver
committed
[SILGen] Make sure enum element is always cleaned up.
Makes the source enum value a "managed value" so that it is cleaned up after the data is extracted.
1 parent 1105843 commit 92b4185

File tree

6 files changed

+114
-7
lines changed

6 files changed

+114
-7
lines changed

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,9 +2048,12 @@ void PatternMatchEmission::emitEnumElementDispatch(
20482048
break;
20492049
}
20502050

2051+
CleanupCloner srcValueCleanupCloner(SGF, src.getFinalManagedValue());
2052+
20512053
// Emit the switch_enum_addr instruction.
2052-
SILValue srcValue = src.getFinalManagedValue().forward(SGF);
2053-
SGF.B.createSwitchEnumAddr(loc, srcValue, blocks.getDefaultBlock(),
2054+
ManagedValue srcValue =
2055+
ManagedValue::forUnmanaged(src.getFinalManagedValue().forward(SGF));
2056+
SGF.B.createSwitchEnumAddr(loc, srcValue.getValue(), blocks.getDefaultBlock(),
20542057
blocks.getCaseBlocks(), blocks.getCounts(),
20552058
defaultCaseCount);
20562059

@@ -2076,6 +2079,13 @@ void PatternMatchEmission::emitEnumElementDispatch(
20762079
hasElt = !eltTy.getASTType()->isVoid();
20772080
}
20782081

2082+
// If the element type is trivial, the enum won't be consumed. In this case,
2083+
// make sure we emit a cleanup for the enum itself (not the data, which is
2084+
// trivial).
2085+
if (!hasElt || eltTy.isTrivial(SGF.getFunction())) {
2086+
srcValueCleanupCloner.clone(srcValue.getValue());
2087+
}
2088+
20792089
ConsumableManagedValue eltCMV, origCMV;
20802090

20812091
// Empty cases. Try to avoid making an empty tuple value if it's
@@ -2124,8 +2134,8 @@ void PatternMatchEmission::emitEnumElementDispatch(
21242134
// nondestructively from an enum.
21252135
switch (eltConsumption) {
21262136
case CastConsumptionKind::TakeAlways:
2127-
eltValue =
2128-
SGF.B.createUncheckedTakeEnumDataAddr(loc, srcValue, elt, eltTy);
2137+
eltValue = SGF.B.createUncheckedTakeEnumDataAddr(
2138+
loc, srcValue.getValue(), elt, eltTy);
21292139
break;
21302140
case CastConsumptionKind::BorrowAlways:
21312141
// If we reach this point, we know that we have a loadable
@@ -2135,9 +2145,14 @@ void PatternMatchEmission::emitEnumElementDispatch(
21352145
// address only types do not support BorrowAlways.
21362146
llvm_unreachable("not allowed");
21372147
case CastConsumptionKind::CopyOnSuccess: {
2138-
auto copy = SGF.emitTemporaryAllocation(loc, srcValue->getType());
2139-
SGF.B.createCopyAddr(loc, srcValue, copy, IsNotTake, IsInitialization);
2140-
// We can always take from the copy.
2148+
auto copy = SGF.emitTemporaryAllocation(loc, srcValue.getType());
2149+
// If the element is trivial it won't be destroyed so we need to make
2150+
// sure we destroy the enum here.
2151+
if (eltTL->isTrivial())
2152+
copy = SGF.emitManagedBufferWithCleanup(copy).getValue();
2153+
SGF.B.createCopyAddr(loc, srcValue.getValue(), copy, IsNotTake,
2154+
IsInitialization);
2155+
// We can always take from the managedCopy.
21412156
eltConsumption = CastConsumptionKind::TakeAlways;
21422157
eltValue = SGF.B.createUncheckedTakeEnumDataAddr(loc, copy, elt, eltTy);
21432158
break;

‎test/Interpreter/switch.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,24 @@ SwitchTestSuite.test("Enum Initialization Leaks") {
282282
}
283283
}
284284

285+
SwitchTestSuite.test("Destroy all elements: protocol and trivial") {
286+
enum ProtocolAndTrivial {
287+
case a(P)
288+
case b(Int)
289+
case c(LifetimeTracked)
290+
}
291+
292+
func testEnumWithProtocolAndTrivial(_ e: ProtocolAndTrivial) -> Bool {
293+
switch (e) {
294+
case .a(_): return true
295+
case .b(_): return true
296+
case .c(_): return true
297+
}
298+
}
299+
300+
do {
301+
testEnumWithProtocolAndTrivial(.c(LifetimeTracked(0)))
302+
}
303+
}
304+
285305
runAllTests()

‎test/SILGen/destroy_enum_in_all_cases.swift

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %target-swift-emit-silgen -module-name switch %s > %t.sil
2+
// RUN: %target-sil-opt %t.sil -sil-combine -enable-sil-verify-all | %FileCheck %s
3+
4+
protocol P { }
5+
6+
enum ProtocolAndTrivial {
7+
case a(P)
8+
case b(Int)
9+
}
10+
11+
// This test is mainly checking the verifier doesn't error but, just in case,
12+
// there's also a filecheck test.
13+
// CHECK-LABEL: @$s6switch4testyyAA18ProtocolAndTrivialO_ADtF
14+
// CHECK: [[A1:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
15+
// CHECK: [[A2:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
16+
// CHECK: [[C1:%[0-9]+]] = alloc_stack $(ProtocolAndTrivial, ProtocolAndTrivial)
17+
18+
// Make sure we destroy both arg temp allocs.
19+
// CHECK: copy_addr [take] [[A1]]
20+
// CHECK: copy_addr [take] [[A2]]
21+
22+
// CHECK: [[CA:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 0
23+
// CHECK: [[CB:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 1
24+
25+
// CHECK: switch_enum_addr [[CA]] : $*ProtocolAndTrivial, case #ProtocolAndTrivial.a!enumelt: [[BBA:bb[0-9]+]], case #ProtocolAndTrivial.b!enumelt: [[BBB:bb[0-9]+]]
26+
27+
// We're only testing the trivial case.
28+
// CHECK: [[BBB]]:
29+
// CHECK: [[TMP:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
30+
// CHECK: switch_enum_addr [[CB]] : $*ProtocolAndTrivial, case #ProtocolAndTrivial.b!enumelt: [[BOTH_B:bb[0-9]+]], default [[DEFAULT:bb[0-9]+]]
31+
32+
// Make sure that we destroy everything.
33+
// CHECK: [[BOTH_B]]:
34+
// CHECK: destroy_addr [[CB]]
35+
// CHECK: destroy_addr [[TMP]]
36+
// CHECK: destroy_addr [[CA]]
37+
38+
// CHECK-LABEL: end sil function '$s6switch4testyyAA18ProtocolAndTrivialO_ADtF'
39+
func test(_ a : ProtocolAndTrivial, _ b : ProtocolAndTrivial) {
40+
switch ((a, b)) {
41+
case (.a(_), .a(_)): _ = ()
42+
case (.b(_), .b(_)): _ = ()
43+
default: _ = ()
44+
}
45+
}

‎test/SILGen/enum_resilience.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ import resilient_enum
1515
// CHECK-NEXT: [[METATYPE:%.+]] = value_metatype $@thick Medium.Type, [[BOX]] : $*Medium
1616
// CHECK-NEXT: switch_enum_addr [[BOX]] : $*Medium, case #Medium.Paper!enumelt: bb1, case #Medium.Canvas!enumelt: bb2, case #Medium.Pamphlet!enumelt: bb3, case #Medium.Postcard!enumelt: bb4, default bb5
1717
// CHECK: bb1:
18+
// CHECK-NEXT: destroy_addr [[BOX]]
1819
// CHECK-NEXT: dealloc_stack [[BOX]]
1920
// CHECK-NEXT: br bb6
2021
// CHECK: bb2:
22+
// CHECK-NEXT: destroy_addr [[BOX]]
2123
// CHECK-NEXT: dealloc_stack [[BOX]]
2224
// CHECK-NEXT: br bb6
2325
// CHECK: bb3:

‎test/SILGen/enum_resilience_testable.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
// CHECK-NEXT: [[METATYPE:%.+]] = value_metatype $@thick Medium.Type, [[BOX]] : $*Medium
2121
// CHECK-NEXT: switch_enum_addr [[BOX]] : $*Medium, case #Medium.Paper!enumelt: bb1, case #Medium.Canvas!enumelt: bb2, case #Medium.Pamphlet!enumelt: bb3, case #Medium.Postcard!enumelt: bb4, default bb5
2222
// CHECK: bb1:
23+
// CHECK-NEXT: destroy_addr [[BOX]]
2324
// CHECK-NEXT: dealloc_stack [[BOX]]
2425
// CHECK-NEXT: br bb6
2526
// CHECK: bb2:
27+
// CHECK-NEXT: destroy_addr [[BOX]]
2628
// CHECK-NEXT: dealloc_stack [[BOX]]
2729
// CHECK-NEXT: br bb6
2830
// CHECK: bb3:

‎test/SILGen/switch.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,3 +1605,26 @@ struct rdar49990484Struct {
16051605
}
16061606
}
16071607
}
1608+
1609+
enum ProtocolAndTrivial {
1610+
case a(P)
1611+
case b(Int)
1612+
}
1613+
1614+
// Make sure "e" is destroyed in both cases.
1615+
// CHECK-LABEL: sil hidden [ossa] @$s6switch30testEnumWithProtocolAndTrivialySbAA0efG0OF
1616+
// CHECK: bb0(
1617+
// CHECK: [[TMP:%[0-9]+]] = alloc_stack
1618+
// CHECK: bb1:
1619+
// CHECK: [[P:%[0-9]+]] = unchecked_take_enum_data_addr
1620+
// CHECK: destroy_addr [[P]]
1621+
// CHECK: bb2:
1622+
// CHECK: destroy_addr [[TMP]]
1623+
// CHECK-LABEL: end sil function '$s6switch30testEnumWithProtocolAndTrivialySbAA0efG0OF'
1624+
func testEnumWithProtocolAndTrivial(_ e: ProtocolAndTrivial) -> Bool {
1625+
switch (e) {
1626+
case .a(_): return true
1627+
case .b(_): return true
1628+
}
1629+
}
1630+

0 commit comments

Comments
 (0)