Merge pull request #37840 from CodaFi/space-for-improvement

Add a Subtraction Optimization

Author: CodaFi

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -137,7 +137,7 @@ namespace {
           cache.insert(getType().getPointer());
 
           SmallVector<Space, 4> spaces;
-          decompose(DC, getType(), spaces);
+          decomposeDisjuncts(DC, getType(), {}, spaces);
           size_t acc = 0;
           for (auto &sp : spaces) {
             // Decomposed pattern spaces grow with the sum of the subspaces.
@@ -317,14 +317,14 @@ namespace {
 
           // (_ : Ty1) <= (_ : Ty2) iff D(Ty1) == D(Ty2)
           if (canDecompose(this->getType())) {
-            Space or1Space = decompose(DC, this->getType());
+            Space or1Space = decompose(DC, this->getType(), {});
             if (or1Space.isSubspace(other, DC)) {
               return true;
             }
           }
 
           if (canDecompose(other.getType())) {
-            Space or2Space = decompose(DC, other.getType());
+            Space or2Space = decompose(DC, other.getType(), {});
             return this->isSubspace(or2Space, DC);
           }
 
@@ -342,13 +342,13 @@ namespace {
           if (!canDecompose(this->getType())) {
             return false;
           }
-          Space or1Space = decompose(DC, this->getType());
+          Space or1Space = decompose(DC, this->getType(), {});
           return or1Space.isSubspace(other, DC);
         }
         PAIRCASE (SpaceKind::Type, SpaceKind::Constructor): {
           // (_ : Ty1) <= H(p1 | ... | pn) iff D(Ty1) <= H(p1 | ... | pn)
           if (canDecompose(this->getType())) {
-            Space or1Space = decompose(DC, this->getType());
+            Space or1Space = decompose(DC, this->getType(), {});
             return or1Space.isSubspace(other, DC);
           }
           // An undecomposable type is always larger than its constructor space.
@@ -466,7 +466,7 @@ namespace {
         }
         PAIRCASE (SpaceKind::Type, SpaceKind::Constructor): {
           if (canDecompose(this->getType())) {
-            auto decomposition = decompose(DC, this->getType());
+            auto decomposition = decompose(DC, this->getType(), {});
             return decomposition.minus(other, DC, minusCount);
           } else {
             return *this;
@@ -480,8 +480,38 @@ namespace {
           // decomposable.
           return *this;
 
+        PAIRCASE (SpaceKind::Type, SpaceKind::Disjunct): {
+          // Optimize for the common case of a type minus a disjunct of
+          // constructor subspaces. This form of subtraction is guaranteed to
+          // happen very early on, and we can eliminate a huge part of the
+          // pattern space by only decomposing the parts of the type space that
+          // aren't actually covered by the disjunction.
+          if (canDecompose(this->getType())) {
+            llvm::StringSet<> otherConstructors;
+            for (auto s : other.getSpaces()) {
+              // Filter for constructor spaces with no payloads.
+              if (s.getKind() != SpaceKind::Constructor) {
+                continue;
+              }
+
+              if (!s.getSpaces().empty()) {
+                continue;
+              }
+
+              otherConstructors.insert(s.Head.getBaseIdentifier().str());
+            }
+
+            auto decomposition = decompose(DC, this->getType(),
+                                           otherConstructors);
+            return decomposition.minus(other, DC, minusCount);
+          } else {
+            // If the type isn't decomposable then there's no way we can
+            // subtract from it. Report the total space as uncovered.
+            return *this;
+          }
+        }
+
         PAIRCASE (SpaceKind::Empty, SpaceKind::Disjunct):
-        PAIRCASE (SpaceKind::Type, SpaceKind::Disjunct):
         PAIRCASE (SpaceKind::Constructor, SpaceKind::Disjunct):
         PAIRCASE (SpaceKind::Disjunct, SpaceKind::Disjunct):
         PAIRCASE (SpaceKind::BooleanConstant, SpaceKind::Disjunct):
@@ -628,7 +658,7 @@ namespace {
           }
 
           if (canDecompose(other.getType())) {
-            auto decomposition = decompose(DC, other.getType());
+            auto decomposition = decompose(DC, other.getType(), {});
             return this->minus(decomposition, DC, minusCount);
           }
           return *this;
@@ -640,7 +670,7 @@ namespace {
 
         PAIRCASE (SpaceKind::Type, SpaceKind::BooleanConstant): {
           if (canDecompose(this->getType())) {
-            auto orSpace = decompose(DC, this->getType());
+            auto orSpace = decompose(DC, this->getType(), {});
             return orSpace.minus(other, DC, minusCount);
           } else {
             return *this;
@@ -778,9 +808,16 @@ namespace {
         }
       };
 
-      // Decompose a type into its component spaces.
-      static void decompose(const DeclContext *DC, Type tp,
-                            SmallVectorImpl<Space> &arr) {
+      // Decompose a type into its component spaces, ignoring any enum
+      // cases that have no payloads and are also in the `voidList`. Membership
+      // there means the space is guaranteed by the subtraction procedure to be
+      // covered, so there's no reason to include it. Note that this *only*
+      // works for constructor spaces with no payloads as these cannot be
+      // overloaded and there is no further recursive structure to subtract
+      // into.
+      static void decomposeDisjuncts(const DeclContext *DC, Type tp,
+                                     const llvm::StringSet<> &voidList,
+                                     SmallVectorImpl<Space> &arr) {
         assert(canDecompose(tp) && "Non-decomposable type?");
 
         if (tp->isBool()) {
@@ -797,6 +834,14 @@ namespace {
                   return Space();
                 }
 
+                // If we're guaranteed a match from a subtraction, don't include
+                // the space at all. See the `Type - Disjunct` case of
+                // subtraction for when this optimization applies.
+                if (!eed->hasAssociatedValues() &&
+                    voidList.contains(eed->getBaseIdentifier().str())) {
+                  return Space();
+                }
+
                 // .e(a: X, b: X)   -> (a: X, b: X)
                 // .f((a: X, b: X)) -> ((a: X, b: X)
                 auto eedTy = tp->getCanonicalType()->getTypeOfMember(
@@ -838,9 +883,11 @@ namespace {
         }
       }
 
-      static Space decompose(const DeclContext *DC, Type type) {
+      static Space decompose(const DeclContext *DC,
+                             Type type,
+                             const llvm::StringSet<> &voidList) {
         SmallVector<Space, 4> spaces;
-        decompose(DC, type, spaces);
+        decomposeDisjuncts(DC, type, voidList, spaces);
         return Space::forDisjunct(spaces);
       }
 
@@ -1034,7 +1081,7 @@ namespace {
       if (uncovered.getKind() == SpaceKind::Type) {
         if (Space::canDecompose(uncovered.getType())) {
           SmallVector<Space, 4> spaces;
-          Space::decompose(DC, uncovered.getType(), spaces);
+          Space::decomposeDisjuncts(DC, uncovered.getType(), {}, spaces);
           diagnoseMissingCases(RequiresDefault::No, Space::forDisjunct(spaces),
                                unknownCase);
         } else {

test/Sema/exhaustive_switch_huge.swift.gyb

@@ -0,0 +1,19 @@
+// RUN: %empty-directory(%t)
+// RUN: %gyb %s -o %t/exhaustive_switch_huge.swift
+// RUN: %target-swift-frontend -typecheck -verify %t/exhaustive_switch_huge.swift
+
+% case_limit = 10000
+
+// Make sure the exhaustiveness checker can check an unreasonable amount of
+// enum cases in a reasonable amount of time.
+enum E {
+% for i in range(case_limit):
+  case x${i}
+% end
+}
+
+switch E.x1 {
+% for i in range(case_limit):
+  case .x${i}  : break
+% end
+}