Skip to content

Commit 2b78e16

Browse files
committed
[SpaceEngine] Improve total search space size estimation
In some situations it is possible to determine that a particular associated value is completely covered by a given `switch` statement before running main checker algorithm, so let's not include sizes of such cases into "total" search space size estimate. Resolves: rdar://problem/40400251
1 parent 131b860 commit 2b78e16

File tree

2 files changed

+142
-7
lines changed

2 files changed

+142
-7
lines changed

lib/Sema/TypeCheckSwitchStmt.cpp

Lines changed: 105 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,7 +1105,104 @@ namespace {
11051105
return false;
11061106
}
11071107
}
1108-
1108+
1109+
/// Estimate how big is the search space that exhaustivity
1110+
/// checker needs to cover, based on the total space and information
1111+
/// from the `switch` statement itself. Some of the easy situations
1112+
/// like `case .foo(let bar)` don't really contribute to the complexity
1113+
/// of the search so their sub-space sizes could be excluded from
1114+
/// consideration.
1115+
///
1116+
/// \param total The total space to check.
1117+
/// \param covered The space covered by the `case` statements in the switch.
1118+
///
1119+
/// \returns The size of the search space exhastivity checker has to check.
1120+
size_t estimateSearchSpaceSize(const Space &total, const Space &covered) {
1121+
switch (PairSwitch(total.getKind(), covered.getKind())) {
1122+
PAIRCASE(SpaceKind::Type, SpaceKind::Type): {
1123+
return total.getType()->isEqual(covered.getType())
1124+
? 0
1125+
: total.getSize(TC, DC);
1126+
}
1127+
PAIRCASE(SpaceKind::Type, SpaceKind::Disjunct):
1128+
PAIRCASE(SpaceKind::Type, SpaceKind::Constructor): {
1129+
if (!Space::canDecompose(total.getType(), DC))
1130+
break;
1131+
1132+
auto decomposition = Space::decompose(TC, DC, total.getType());
1133+
return estimateSearchSpaceSize(decomposition, covered);
1134+
}
1135+
1136+
PAIRCASE(SpaceKind::Disjunct, SpaceKind::Disjunct):
1137+
PAIRCASE(SpaceKind::Disjunct, SpaceKind::Constructor): {
1138+
auto &spaces = total.getSpaces();
1139+
return std::accumulate(spaces.begin(), spaces.end(), 0,
1140+
[&](size_t totalSize, const Space &space) {
1141+
return totalSize + estimateSearchSpaceSize(
1142+
space, covered);
1143+
});
1144+
}
1145+
1146+
// Search space size computation is not commutative, because it
1147+
// tries to check if space on right-hand side is covering any
1148+
// portion of the "total" space on the left.
1149+
PAIRCASE(SpaceKind::Constructor, SpaceKind::Disjunct): {
1150+
for (const auto &space : covered.getSpaces()) {
1151+
// enum E { case foo }
1152+
// func bar(_ lhs: E, _ rhs: E) {
1153+
// switch (lhs, rhs) {
1154+
// case (_, _): break
1155+
// }
1156+
if (total == space)
1157+
return 0;
1158+
1159+
if (!space.isSubspace(total, TC, DC))
1160+
continue;
1161+
1162+
if (estimateSearchSpaceSize(total, space) == 0)
1163+
return 0;
1164+
}
1165+
break;
1166+
}
1167+
1168+
PAIRCASE(SpaceKind::Constructor, SpaceKind::Constructor): {
1169+
if (total.getHead() != covered.getHead())
1170+
break;
1171+
1172+
auto &lhs = total.getSpaces();
1173+
auto &rhs = covered.getSpaces();
1174+
1175+
if (std::distance(lhs.begin(), lhs.end()) !=
1176+
std::distance(rhs.begin(), rhs.end()))
1177+
return total.getSize(TC, DC);
1178+
1179+
auto i = lhs.begin();
1180+
auto j = rhs.begin();
1181+
1182+
size_t totalSize = 0;
1183+
for (; i != lhs.end() && j != rhs.end(); ++i, ++j) {
1184+
// The only light-weight checking we can do
1185+
// is when sub-spaces on both sides are types
1186+
// otherwise we'd have to decompose, which
1187+
// is too heavy, so let's just return total
1188+
// space size if such situation is detected.
1189+
if (i->getKind() != SpaceKind::Type ||
1190+
j->getKind() != SpaceKind::Type)
1191+
return total.getSize(TC, DC);
1192+
1193+
totalSize += estimateSearchSpaceSize(*i, *j);
1194+
}
1195+
1196+
return totalSize;
1197+
}
1198+
1199+
default:
1200+
break;
1201+
}
1202+
1203+
return total.getSize(TC, DC);
1204+
}
1205+
11091206
void checkExhaustiveness(bool limitedChecking) {
11101207
// If the type of the scrutinee is uninhabited, we're already dead.
11111208
// Allow any well-typed patterns through.
@@ -1174,16 +1271,17 @@ namespace {
11741271
Space totalSpace = Space::forType(subjectType, Identifier());
11751272
Space coveredSpace = Space::forDisjunct(spaces);
11761273

1177-
const size_t totalSpaceSize = totalSpace.getSize(TC, DC);
1178-
if (totalSpaceSize > Space::getMaximumSize()) {
1179-
diagnoseCannotCheck(sawRedundantPattern, totalSpaceSize, coveredSpace,
1274+
const size_t searchSpaceSizeEstimate =
1275+
estimateSearchSpaceSize(totalSpace, coveredSpace);
1276+
if (searchSpaceSizeEstimate > Space::getMaximumSize()) {
1277+
diagnoseCannotCheck(sawRedundantPattern, totalSpace, coveredSpace,
11801278
unknownCase);
11811279
return;
11821280
}
11831281
unsigned minusCount = 0;
11841282
auto diff = totalSpace.minus(coveredSpace, TC, DC, &minusCount);
11851283
if (!diff) {
1186-
diagnoseCannotCheck(sawRedundantPattern, totalSpaceSize, coveredSpace,
1284+
diagnoseCannotCheck(sawRedundantPattern, totalSpace, coveredSpace,
11871285
unknownCase);
11881286
return;
11891287
}
@@ -1234,7 +1332,7 @@ namespace {
12341332
};
12351333

12361334
void diagnoseCannotCheck(const bool sawRedundantPattern,
1237-
const size_t totalSpaceSize,
1335+
const Space &totalSpace,
12381336
const Space &coveredSpace,
12391337
const CaseStmt *unknownCase) {
12401338
// Because the space is large or the check is too slow,
@@ -1246,7 +1344,7 @@ namespace {
12461344
// a 'default' to be inserted.
12471345
// FIXME: Do something sensible for non-frozen enums.
12481346
if (!sawRedundantPattern &&
1249-
coveredSpace.getSize(TC, DC) >= totalSpaceSize)
1347+
coveredSpace.getSize(TC, DC) >= totalSpace.getSize(TC, DC))
12501348
return;
12511349
diagnoseMissingCases(RequiresDefault::SpaceTooLarge, Space(),
12521350
unknownCase);

test/stmt/rdar40400251.swift

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
enum E1 {
4+
case v1
5+
case v2
6+
case v3
7+
case v4
8+
case v5
9+
case v6
10+
indirect case v7(E1)
11+
}
12+
13+
enum E2 {
14+
case foo((E1, E1, E1)) // total size of this case is 7 ^ 3 + 1
15+
case bar(E1)
16+
case baz
17+
}
18+
19+
func foo(_ e: E2) {
20+
switch e {
21+
// expected-error@-1 {{switch must be exhaustive}}
22+
// expected-note@-2 {{add missing case: '.bar(_)'}}
23+
24+
case .foo(let tuple): break // expected-warning {{immutable value 'tuple' was never used; consider replacing with '_' or removing it}}
25+
case .baz: break
26+
}
27+
}
28+
29+
func bar(_ e: E2) {
30+
switch e {
31+
// expected-error@-1 {{switch must be exhaustive}}
32+
// expected-note@-2 {{add missing case: '.bar(_)'}}
33+
// expected-note@-3 {{add missing case: '.baz'}}
34+
35+
case .foo((_, _, _)): break
36+
}
37+
}

0 commit comments

Comments
 (0)