Skip to content

Commit 2c417ec

Browse files
committed
Add a Subtraction Optimization
When subtraction encounters a large type space and a disjunction - as it is bound to do when the exhaustiveness checker first submits the initial pattern space to it - chaos ensues. The subtraction breaks down like this: T - (C1 | C2 | C3 | ...) = (T1 | T2 | T3 | ...) - C1 | (T1 | T2 | T3 | ...) - C2 | (T1 | T2 | T3 | ...) - C3 | ... For large types, this quickly trips the evaluation limit in the space engine. The repeated decompositions are only necessary if Cn is a constructor space with a payload. If it's a simple constructor space, we know that the subtraction is for that constructor space is always going to succeed anyways. So, avoid creating this explosion of disjuncts by "subtracting en-masse". We gather all of the Cn that do not have payloads, then when decompositing T we intentionally void out any matching Cn. Thus the subtraction becomes T - (C1 | C2 | C3 | ...) = () In the case when C1, C2, ..., Cn have no payload. Otherwise it becomes T - (C1 | C2 | C3 | ...) = (T1 | T2 | T3 | ...) - Ck(c1, c2, ...) | ... Which is, barring any further clever optimizations, the best we can do for now. This allows enormous enums on the order of 10^5 cases to typecheck in a "reasonable" amount of time. Resolves rdar://59129547
1 parent 3602f1a commit 2c417ec

File tree

2 files changed

+81
-15
lines changed

2 files changed

+81
-15
lines changed

lib/Sema/TypeCheckSwitchStmt.cpp

+62-15
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ namespace {
137137
cache.insert(getType().getPointer());
138138

139139
SmallVector<Space, 4> spaces;
140-
decompose(DC, getType(), spaces);
140+
decomposeDisjuncts(DC, getType(), {}, spaces);
141141
size_t acc = 0;
142142
for (auto &sp : spaces) {
143143
// Decomposed pattern spaces grow with the sum of the subspaces.
@@ -317,14 +317,14 @@ namespace {
317317

318318
// (_ : Ty1) <= (_ : Ty2) iff D(Ty1) == D(Ty2)
319319
if (canDecompose(this->getType())) {
320-
Space or1Space = decompose(DC, this->getType());
320+
Space or1Space = decompose(DC, this->getType(), {});
321321
if (or1Space.isSubspace(other, DC)) {
322322
return true;
323323
}
324324
}
325325

326326
if (canDecompose(other.getType())) {
327-
Space or2Space = decompose(DC, other.getType());
327+
Space or2Space = decompose(DC, other.getType(), {});
328328
return this->isSubspace(or2Space, DC);
329329
}
330330

@@ -342,13 +342,13 @@ namespace {
342342
if (!canDecompose(this->getType())) {
343343
return false;
344344
}
345-
Space or1Space = decompose(DC, this->getType());
345+
Space or1Space = decompose(DC, this->getType(), {});
346346
return or1Space.isSubspace(other, DC);
347347
}
348348
PAIRCASE (SpaceKind::Type, SpaceKind::Constructor): {
349349
// (_ : Ty1) <= H(p1 | ... | pn) iff D(Ty1) <= H(p1 | ... | pn)
350350
if (canDecompose(this->getType())) {
351-
Space or1Space = decompose(DC, this->getType());
351+
Space or1Space = decompose(DC, this->getType(), {});
352352
return or1Space.isSubspace(other, DC);
353353
}
354354
// An undecomposable type is always larger than its constructor space.
@@ -466,7 +466,7 @@ namespace {
466466
}
467467
PAIRCASE (SpaceKind::Type, SpaceKind::Constructor): {
468468
if (canDecompose(this->getType())) {
469-
auto decomposition = decompose(DC, this->getType());
469+
auto decomposition = decompose(DC, this->getType(), {});
470470
return decomposition.minus(other, DC, minusCount);
471471
} else {
472472
return *this;
@@ -480,8 +480,38 @@ namespace {
480480
// decomposable.
481481
return *this;
482482

483+
PAIRCASE (SpaceKind::Type, SpaceKind::Disjunct): {
484+
// Optimize for the common case of a type minus a disjunct of
485+
// constructor subspaces. This form of subtraction is guaranteed to
486+
// happen very early on, and we can eliminate a huge part of the
487+
// pattern space by only decomposing the parts of the type space that
488+
// aren't actually covered by the disjunction.
489+
if (canDecompose(this->getType())) {
490+
llvm::StringSet<> otherConstructors;
491+
for (auto s : other.getSpaces()) {
492+
// Filter for constructor spaces with no payloads.
493+
if (s.getKind() != SpaceKind::Constructor) {
494+
continue;
495+
}
496+
497+
if (!s.getSpaces().empty()) {
498+
continue;
499+
}
500+
501+
otherConstructors.insert(s.Head.getBaseIdentifier().str());
502+
}
503+
504+
auto decomposition = decompose(DC, this->getType(),
505+
otherConstructors);
506+
return decomposition.minus(other, DC, minusCount);
507+
} else {
508+
// If the type isn't decomposable then there's no way we can
509+
// subtract from it. Report the total space as uncovered.
510+
return *this;
511+
}
512+
}
513+
483514
PAIRCASE (SpaceKind::Empty, SpaceKind::Disjunct):
484-
PAIRCASE (SpaceKind::Type, SpaceKind::Disjunct):
485515
PAIRCASE (SpaceKind::Constructor, SpaceKind::Disjunct):
486516
PAIRCASE (SpaceKind::Disjunct, SpaceKind::Disjunct):
487517
PAIRCASE (SpaceKind::BooleanConstant, SpaceKind::Disjunct):
@@ -628,7 +658,7 @@ namespace {
628658
}
629659

630660
if (canDecompose(other.getType())) {
631-
auto decomposition = decompose(DC, other.getType());
661+
auto decomposition = decompose(DC, other.getType(), {});
632662
return this->minus(decomposition, DC, minusCount);
633663
}
634664
return *this;
@@ -640,7 +670,7 @@ namespace {
640670

641671
PAIRCASE (SpaceKind::Type, SpaceKind::BooleanConstant): {
642672
if (canDecompose(this->getType())) {
643-
auto orSpace = decompose(DC, this->getType());
673+
auto orSpace = decompose(DC, this->getType(), {});
644674
return orSpace.minus(other, DC, minusCount);
645675
} else {
646676
return *this;
@@ -778,9 +808,16 @@ namespace {
778808
}
779809
};
780810

781-
// Decompose a type into its component spaces.
782-
static void decompose(const DeclContext *DC, Type tp,
783-
SmallVectorImpl<Space> &arr) {
811+
// Decompose a type into its component spaces, ignoring any enum
812+
// cases that have no payloads and are also in the `voidList`. Membership
813+
// there means the space is guaranteed by the subtraction procedure to be
814+
// covered, so there's no reason to include it. Note that this *only*
815+
// works for constructor spaces with no payloads as these cannot be
816+
// overloaded and there is no further recursive structure to subtract
817+
// into.
818+
static void decomposeDisjuncts(const DeclContext *DC, Type tp,
819+
const llvm::StringSet<> &voidList,
820+
SmallVectorImpl<Space> &arr) {
784821
assert(canDecompose(tp) && "Non-decomposable type?");
785822

786823
if (tp->isBool()) {
@@ -797,6 +834,14 @@ namespace {
797834
return Space();
798835
}
799836

837+
// If we're guaranteed a match from a subtraction, don't include
838+
// the space at all. See the `Type - Disjunct` case of
839+
// subtraction for when this optimization applies.
840+
if (!eed->hasAssociatedValues() &&
841+
voidList.contains(eed->getBaseIdentifier().str())) {
842+
return Space();
843+
}
844+
800845
// .e(a: X, b: X) -> (a: X, b: X)
801846
// .f((a: X, b: X)) -> ((a: X, b: X)
802847
auto eedTy = tp->getCanonicalType()->getTypeOfMember(
@@ -838,9 +883,11 @@ namespace {
838883
}
839884
}
840885

841-
static Space decompose(const DeclContext *DC, Type type) {
886+
static Space decompose(const DeclContext *DC,
887+
Type type,
888+
const llvm::StringSet<> &voidList) {
842889
SmallVector<Space, 4> spaces;
843-
decompose(DC, type, spaces);
890+
decomposeDisjuncts(DC, type, voidList, spaces);
844891
return Space::forDisjunct(spaces);
845892
}
846893

@@ -1034,7 +1081,7 @@ namespace {
10341081
if (uncovered.getKind() == SpaceKind::Type) {
10351082
if (Space::canDecompose(uncovered.getType())) {
10361083
SmallVector<Space, 4> spaces;
1037-
Space::decompose(DC, uncovered.getType(), spaces);
1084+
Space::decomposeDisjuncts(DC, uncovered.getType(), {}, spaces);
10381085
diagnoseMissingCases(RequiresDefault::No, Space::forDisjunct(spaces),
10391086
unknownCase);
10401087
} else {
+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %gyb %s -o %t/exhaustive_switch_huge.swift
3+
// RUN: %target-swift-frontend -typecheck -verify %t/exhaustive_switch_huge.swift
4+
5+
% case_limit = 10000
6+
7+
// Make sure the exhaustiveness checker can check an unreasonable amount of
8+
// enum cases in a reasonable amount of time.
9+
enum E {
10+
% for i in range(case_limit):
11+
case x${i}
12+
% end
13+
}
14+
15+
switch E.x1 {
16+
% for i in range(case_limit):
17+
case .x${i} : break
18+
% end
19+
}

0 commit comments

Comments
 (0)