Skip to content

Commit 42ab996

Browse files
jrose-applejrose-apple
committed
[SILGen] Fix one more place that needs to check for enum exhaustivity
And add a bunch of tests, including one that runs into the verifier check added previously without this change.
1 parent ec8aa9f commit 42ab996

File tree

4 files changed

+296
-41
lines changed

4 files changed

+296
-41
lines changed

‎include/swift/AST/Decl.h

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3266,45 +3266,6 @@ class EnumDecl final : public NominalTypeDecl {
32663266
return std::distance(eltRange.begin(), eltRange.end());
32673267
}
32683268

3269-
/// If this is an enum with two cases, return the other case. Otherwise,
3270-
/// return nullptr.
3271-
EnumElementDecl *getOppositeBinaryDecl(EnumElementDecl *decl) const {
3272-
ElementRange range = getAllElements();
3273-
auto iter = range.begin();
3274-
if (iter == range.end())
3275-
return nullptr;
3276-
bool seenDecl = false;
3277-
EnumElementDecl *result = nullptr;
3278-
if (*iter == decl) {
3279-
seenDecl = true;
3280-
} else {
3281-
result = *iter;
3282-
}
3283-
3284-
++iter;
3285-
if (iter == range.end())
3286-
return nullptr;
3287-
if (seenDecl) {
3288-
assert(!result);
3289-
result = *iter;
3290-
} else {
3291-
if (decl != *iter)
3292-
return nullptr;
3293-
seenDecl = true;
3294-
}
3295-
++iter;
3296-
3297-
// If we reach this point, we saw the decl we were looking for and one other
3298-
// case. If we have any additional cases, then we do not have a binary enum.
3299-
if (iter != range.end())
3300-
return nullptr;
3301-
3302-
// This is always true since we have already returned earlier nullptr if we
3303-
// did not see the decl at all.
3304-
assert(seenDecl);
3305-
return result;
3306-
}
3307-
33083269
/// If this enum has a unique element, return it. A unique element can
33093270
/// either hold a value or not, and the existence of one unique element does
33103271
/// not imply the existence or non-existence of the opposite unique element.

‎lib/SILGen/SILGenDecl.cpp

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,52 @@ class EnumElementPatternInitialization : public RefutablePatternInitialization {
758758
};
759759
} // end anonymous namespace
760760

761+
/// If \p elt belongs to an enum that has exactly two cases and that can be
762+
/// exhaustively switched, return the other case. Otherwise, return nullptr.
763+
static EnumElementDecl *getOppositeBinaryDecl(const SILGenFunction &SGF,
764+
const EnumElementDecl *elt) {
765+
const EnumDecl *enumDecl = elt->getParentEnum();
766+
if (!enumDecl->isEffectivelyExhaustive(SGF.SGM.SwiftModule,
767+
SGF.F.getResilienceExpansion())) {
768+
return nullptr;
769+
}
770+
771+
EnumDecl::ElementRange range = enumDecl->getAllElements();
772+
auto iter = range.begin();
773+
if (iter == range.end())
774+
return nullptr;
775+
bool seenDecl = false;
776+
EnumElementDecl *result = nullptr;
777+
if (*iter == elt) {
778+
seenDecl = true;
779+
} else {
780+
result = *iter;
781+
}
782+
783+
++iter;
784+
if (iter == range.end())
785+
return nullptr;
786+
if (seenDecl) {
787+
assert(!result);
788+
result = *iter;
789+
} else {
790+
if (elt != *iter)
791+
return nullptr;
792+
seenDecl = true;
793+
}
794+
++iter;
795+
796+
// If we reach this point, we saw the decl we were looking for and one other
797+
// case. If we have any additional cases, then we do not have a binary enum.
798+
if (iter != range.end())
799+
return nullptr;
800+
801+
// This is always true since we have already returned earlier nullptr if we
802+
// did not see the decl at all.
803+
assert(seenDecl);
804+
return result;
805+
}
806+
761807
void EnumElementPatternInitialization::emitEnumMatch(
762808
ManagedValue value, EnumElementDecl *eltDecl, Initialization *subInit,
763809
JumpDest failureDest, SILLocation loc, SILGenFunction &SGF) {
@@ -793,8 +839,7 @@ void EnumElementPatternInitialization::emitEnumMatch(
793839
// If we have a binary enum, do not emit a true default case. This ensures
794840
// that we do not emit a destroy_value on a .None.
795841
bool inferredBinaryEnum = false;
796-
auto *enumDecl = value.getType().getEnumOrBoundGenericEnum();
797-
if (auto *otherDecl = enumDecl->getOppositeBinaryDecl(eltDecl)) {
842+
if (auto *otherDecl = getOppositeBinaryDecl(SGF, eltDecl)) {
798843
inferredBinaryEnum = true;
799844
switchBuilder.addCase(otherDecl, defaultBlock, nullptr, handler);
800845
} else {

‎test/SILOptimizer/Inputs/switch_enum_objc.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Even though these are marked "closed", Swift shouldn't trust it.
2+
3+
enum Alpha {
4+
AlphaA __attribute__((swift_name("a"))),
5+
AlphaB __attribute__((swift_name("b"))),
6+
AlphaC __attribute__((swift_name("c"))),
7+
AlphaD __attribute__((swift_name("d"))),
8+
AlphaE __attribute__((swift_name("e")))
9+
} __attribute__((enum_extensibility(closed)));
10+
11+
enum Coin {
12+
CoinHeads,
13+
CoinTails
14+
} __attribute__((enum_extensibility(closed)));

‎test/SILOptimizer/switch_enum_objc.swift

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
// RUN: %target-swift-frontend -emit-sil -O -primary-file %s -import-objc-header %S/Inputs/switch_enum_objc.h | %FileCheck %s
2+
// RUN: %target-swift-frontend -emit-sil -Osize -primary-file %s -import-objc-header %S/Inputs/switch_enum_objc.h | %FileCheck %s
3+
4+
@inline(never)
5+
func action0() {}
6+
7+
@inline(never)
8+
func action1(_: Int) {}
9+
10+
@inline(never)
11+
func action2(_: Int, _: Int) {}
12+
13+
@inline(never)
14+
func action3(_: Int, _: Int, _: Int) {}
15+
16+
@inline(never)
17+
func action4(_: Int, _: Int, _: Int, _: Int) {}
18+
19+
20+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc14testImperativeyySo5AlphaVF
21+
func testImperative(_ letter: Alpha) {
22+
// CHECK: switch_enum %0 : $Alpha, case #Alpha.a!enumelt: bb1, case #Alpha.b!enumelt: bb2, case #Alpha.c!enumelt: bb3, case #Alpha.d!enumelt: bb4, case #Alpha.e!enumelt: bb5, default bb6
23+
switch letter {
24+
case .a:
25+
action0()
26+
case .b:
27+
action1(0)
28+
case .c:
29+
action2(0, 0)
30+
case .d:
31+
action3(0, 0, 0)
32+
case .e:
33+
action4(0, 0, 0, 0)
34+
}
35+
// CHECK: bb6:
36+
// CHECK: function_ref @$Ss32_diagnoseUnexpectedEnumCaseValue
37+
} // CHECK: end sil function '$S16switch_enum_objc14testImperativeyySo5AlphaVF'
38+
39+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc27testImperativeDefaultMiddleyySo5AlphaVF
40+
func testImperativeDefaultMiddle(_ letter: Alpha) {
41+
// CHECK: switch_enum %0 : $Alpha, case #Alpha.a!enumelt: bb1, case #Alpha.b!enumelt: bb2, case #Alpha.d!enumelt: bb3, case #Alpha.e!enumelt: bb4, default bb5
42+
switch letter {
43+
case .a:
44+
action0()
45+
case .b:
46+
action1(0)
47+
// case .c:
48+
case .d:
49+
action2(0, 0)
50+
case .e:
51+
action3(0, 0, 0)
52+
default:
53+
// CHECK: bb5:
54+
// CHECK: function_ref @$S16switch_enum_objc7action4
55+
action4(0, 0, 0, 0)
56+
}
57+
} // CHECK: end sil function '$S16switch_enum_objc27testImperativeDefaultMiddleyySo5AlphaVF'
58+
59+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc24testImperativeDefaultEndyySo5AlphaVF
60+
func testImperativeDefaultEnd(_ letter: Alpha) {
61+
// CHECK: switch_enum %0 : $Alpha, case #Alpha.a!enumelt: bb1, case #Alpha.b!enumelt: bb2, case #Alpha.c!enumelt: bb3, case #Alpha.d!enumelt: bb4, default bb5
62+
switch letter {
63+
case .a:
64+
action0()
65+
case .b:
66+
action1(0)
67+
case .c:
68+
action2(0, 0)
69+
case .d:
70+
action3(0, 0, 0)
71+
// case .e:
72+
default:
73+
// CHECK: bb5:
74+
// CHECK: function_ref @$S16switch_enum_objc7action4
75+
action4(0, 0, 0, 0)
76+
}
77+
} // CHECK: end sil function '$S16switch_enum_objc24testImperativeDefaultEndyySo5AlphaVF'
78+
79+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc26testImperativeDefaultMultiyySo5AlphaVF
80+
func testImperativeDefaultMulti(_ letter: Alpha) {
81+
// CHECK: switch_enum %0 : $Alpha, case #Alpha.a!enumelt: bb1, case #Alpha.b!enumelt: bb2, case #Alpha.d!enumelt: bb3, default bb4
82+
switch letter {
83+
case .a:
84+
action0()
85+
case .b:
86+
action1(0)
87+
// case .c:
88+
case .d:
89+
action2(0, 0)
90+
// case .e:
91+
default:
92+
// CHECK: bb4:
93+
// CHECK: function_ref @$S16switch_enum_objc7action3
94+
action3(0, 0, 0)
95+
}
96+
} // CHECK: end sil function '$S16switch_enum_objc26testImperativeDefaultMultiyySo5AlphaVF'
97+
98+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc14testFunctionalySiSo5AlphaVF
99+
func testFunctional(_ letter: Alpha) -> Int {
100+
// This one can't be converted to select_enum because of the generated trap.
101+
// CHECK: switch_enum %0 : $Alpha, case #Alpha.a!enumelt: bb1, case #Alpha.b!enumelt: bb2, case #Alpha.c!enumelt: bb3, case #Alpha.d!enumelt: bb4, case #Alpha.e!enumelt: bb5, default bb6
102+
switch letter {
103+
case .a:
104+
return 3
105+
case .b:
106+
return 5
107+
case .c:
108+
return 8
109+
case .d:
110+
return 13
111+
case .e:
112+
return 21
113+
}
114+
// CHECK: bb6:
115+
// CHECK: function_ref @$Ss32_diagnoseUnexpectedEnumCaseValue
116+
} // CHECK: end sil function '$S16switch_enum_objc14testFunctionalySiSo5AlphaVF'
117+
118+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc27testFunctionalDefaultMiddleySiSo5AlphaVF
119+
func testFunctionalDefaultMiddle(_ letter: Alpha) -> Int {
120+
// CHECK: [[THREE:%.+]] = integer_literal ${{.+}}, 3
121+
// CHECK: [[FIVE:%.+]] = integer_literal ${{.+}}, 5
122+
// CHECK: [[EIGHT:%.+]] = integer_literal ${{.+}}, 8
123+
// CHECK: [[THIRTEEN:%.+]] = integer_literal ${{.+}}, 13
124+
// CHECK: [[TWENTY_ONE:%.+]] = integer_literal ${{.+}}, 21
125+
// CHECK: = select_enum %0 : $Alpha, case #Alpha.a!enumelt: [[THREE]], case #Alpha.b!enumelt: [[FIVE]], case #Alpha.d!enumelt: [[EIGHT]], case #Alpha.e!enumelt: [[THIRTEEN]], default [[TWENTY_ONE]] :
126+
switch letter {
127+
case .a:
128+
return 3
129+
case .b:
130+
return 5
131+
// case .c:
132+
case .d:
133+
return 8
134+
case .e:
135+
return 13
136+
default:
137+
return 21
138+
}
139+
} // CHECK: end sil function '$S16switch_enum_objc27testFunctionalDefaultMiddleySiSo5AlphaVF'
140+
141+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc24testFunctionalDefaultEndySiSo5AlphaVF
142+
func testFunctionalDefaultEnd(_ letter: Alpha) -> Int {
143+
// CHECK: [[THREE:%.+]] = integer_literal ${{.+}}, 3
144+
// CHECK: [[FIVE:%.+]] = integer_literal ${{.+}}, 5
145+
// CHECK: [[EIGHT:%.+]] = integer_literal ${{.+}}, 8
146+
// CHECK: [[THIRTEEN:%.+]] = integer_literal ${{.+}}, 13
147+
// CHECK: [[TWENTY_ONE:%.+]] = integer_literal ${{.+}}, 21
148+
// CHECK: = select_enum %0 : $Alpha, case #Alpha.a!enumelt: [[THREE]], case #Alpha.b!enumelt: [[FIVE]], case #Alpha.c!enumelt: [[EIGHT]], case #Alpha.d!enumelt: [[THIRTEEN]], default [[TWENTY_ONE]] :
149+
switch letter {
150+
case .a:
151+
return 3
152+
case .b:
153+
return 5
154+
case .c:
155+
return 8
156+
case .d:
157+
return 13
158+
// case .e:
159+
default:
160+
return 21
161+
}
162+
} // CHECK: end sil function '$S16switch_enum_objc24testFunctionalDefaultEndySiSo5AlphaVF'
163+
164+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc26testFunctionalDefaultMultiySiSo5AlphaVF
165+
func testFunctionalDefaultMulti(_ letter: Alpha) -> Int {
166+
// CHECK: [[THREE:%.+]] = integer_literal ${{.+}}, 3
167+
// CHECK: [[FIVE:%.+]] = integer_literal ${{.+}}, 5
168+
// CHECK: [[EIGHT:%.+]] = integer_literal ${{.+}}, 8
169+
// CHECK: [[THIRTEEN:%.+]] = integer_literal ${{.+}}, 13
170+
// CHECK: = select_enum %0 : $Alpha, case #Alpha.a!enumelt: [[THREE]], case #Alpha.b!enumelt: [[FIVE]], case #Alpha.d!enumelt: [[EIGHT]], default [[THIRTEEN]] :
171+
switch letter {
172+
case .a:
173+
return 3
174+
case .b:
175+
return 5
176+
// case .c:
177+
case .d:
178+
return 8
179+
// case .e:
180+
default:
181+
return 13
182+
}
183+
} // CHECK: end sil function '$S16switch_enum_objc26testFunctionalDefaultMultiySiSo5AlphaVF'
184+
185+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc19testImperativeHeadsyySo4CoinVF
186+
func testImperativeHeads(_ coin: Coin) {
187+
// CHECK: switch_enum %0 : $Coin, case #Coin.heads!enumelt: bb2, default bb1
188+
// CHECK: bb1:
189+
// CHECK: function_ref @$S16switch_enum_objc7action1
190+
// CHECK: bb2:
191+
// CHECK: function_ref @$S16switch_enum_objc7action0
192+
if case .heads = coin {
193+
action0()
194+
} else {
195+
action1(0)
196+
}
197+
} // CHECK: end sil function '$S16switch_enum_objc19testImperativeHeadsyySo4CoinVF'
198+
199+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc19testImperativeTailsyySo4CoinVF
200+
func testImperativeTails(_ coin: Coin) {
201+
// CHECK: switch_enum %0 : $Coin, case #Coin.tails!enumelt: bb2, default bb1
202+
// CHECK: bb1:
203+
// CHECK: function_ref @$S16switch_enum_objc7action1
204+
// CHECK: bb2:
205+
// CHECK: function_ref @$S16switch_enum_objc7action0
206+
if case .tails = coin {
207+
action0()
208+
} else {
209+
action1(0)
210+
}
211+
} // CHECK: end sil function '$S16switch_enum_objc19testImperativeTailsyySo4CoinVF'
212+
213+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc19testFunctionalHeadsySiSo4CoinVF
214+
func testFunctionalHeads(_ coin: Coin) -> Int {
215+
// CHECK: [[FIVE:%.+]] = integer_literal ${{.+}}, 5000
216+
// CHECK: [[NINE:%.+]] = integer_literal ${{.+}}, 9001
217+
// CHECK: = select_enum %0 : $Coin, case #Coin.heads!enumelt: [[FIVE]], default [[NINE]]
218+
if case .heads = coin {
219+
return 5000
220+
} else {
221+
return 9001
222+
}
223+
} // CHECK: end sil function '$S16switch_enum_objc19testFunctionalHeadsySiSo4CoinVF'
224+
225+
// CHECK-LABEL: sil hidden @$S16switch_enum_objc19testFunctionalTailsySiSo4CoinVF
226+
func testFunctionalTails(_ coin: Coin) -> Int {
227+
// CHECK: [[FIVE:%.+]] = integer_literal ${{.+}}, 5000
228+
// CHECK: [[NINE:%.+]] = integer_literal ${{.+}}, 9001
229+
// CHECK: = select_enum %0 : $Coin, case #Coin.tails!enumelt: [[FIVE]], default [[NINE]]
230+
if case .tails = coin {
231+
return 5000
232+
} else {
233+
return 9001
234+
}
235+
} // CHECK: end sil function '$S16switch_enum_objc19testFunctionalTailsySiSo4CoinVF'

0 commit comments

Comments
 (0)