Merge pull request swiftlang#21230 from CodaFi/casein

Lift The "Large Space Heuristic" For Real This Time

Author: CodaFi

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -115,11 +115,6 @@ namespace {
       Identifier Head;
       std::forward_list<Space> Spaces;
 
-      // NB: This constant is arbitrary.  Anecdotally, the Space Engine is
-      // capable of efficiently handling Spaces of around size 200, but it would
-      // potentially push an enormous fixit on the user.
-      static const size_t MAX_SPACE_SIZE = 128;
-
       size_t computeSize(TypeChecker &TC, const DeclContext *DC,
                          SmallPtrSetImpl<TypeBase *> &cache) const {
         switch (getKind()) {
@@ -250,10 +245,6 @@ namespace {
         return computeSize(TC, DC, cache);
       }
 
-      static size_t getMaximumSize() {
-        return MAX_SPACE_SIZE;
-      }
-
       bool isEmpty() const { return getKind() == SpaceKind::Empty; }
 
       bool isAllowedButNotRequired() const {
@@ -895,103 +886,6 @@ namespace {
       }
     }
 
-    /// Estimate how big is the search space that exhaustivity
-    /// checker needs to cover, based on the total space and information
-    /// from the `switch` statement itself. Some of the easy situations
-    /// like `case .foo(let bar)` don't really contribute to the complexity
-    /// of the search so their sub-space sizes could be excluded from
-    /// consideration.
-    ///
-    /// \param total The total space to check.
-    /// \param covered The space covered by the `case` statements in the switch.
-    ///
-    /// \returns The size of the search space exhastivity checker has to check.
-    size_t estimateSearchSpaceSize(const Space &total, const Space &covered) {
-      switch (PairSwitch(total.getKind(), covered.getKind())) {
-      PAIRCASE(SpaceKind::Type, SpaceKind::Type): {
-        return total.getType()->isEqual(covered.getType())
-                    ? 0
-                    : total.getSize(TC, DC);
-      }
-      PAIRCASE(SpaceKind::Type, SpaceKind::Disjunct):
-      PAIRCASE(SpaceKind::Type, SpaceKind::Constructor): {
-        if (!Space::canDecompose(total.getType(), DC))
-          break;
-
-        auto decomposition = Space::decompose(TC, DC, total.getType());
-        return estimateSearchSpaceSize(decomposition, covered);
-      }
-
-      PAIRCASE(SpaceKind::Disjunct, SpaceKind::Disjunct):
-      PAIRCASE(SpaceKind::Disjunct, SpaceKind::Constructor): {
-        auto &spaces = total.getSpaces();
-        return std::accumulate(spaces.begin(), spaces.end(), 0,
-                               [&](size_t totalSize, const Space &space) {
-                                 return totalSize + estimateSearchSpaceSize(
-                                                        space, covered);
-                               });
-      }
-
-      // Search space size computation is not commutative, because it
-      // tries to check if space on right-hand side is covering any
-      // portion of the "total" space on the left.
-      PAIRCASE(SpaceKind::Constructor, SpaceKind::Disjunct): {
-        for (const auto &space : covered.getSpaces()) {
-          // enum E { case foo }
-          // func bar(_ lhs: E, _ rhs: E) {
-          //   switch (lhs, rhs) {
-          //     case (_, _): break
-          // }
-          if (total == space)
-            return 0;
-
-          if (!space.isSubspace(total, TC, DC))
-            continue;
-
-          if (estimateSearchSpaceSize(total, space) == 0)
-            return 0;
-        }
-        break;
-      }
-
-      PAIRCASE(SpaceKind::Constructor, SpaceKind::Constructor): {
-        if (total.getHead() != covered.getHead())
-          break;
-
-        auto &lhs = total.getSpaces();
-        auto &rhs = covered.getSpaces();
-
-        if (std::distance(lhs.begin(), lhs.end()) !=
-            std::distance(rhs.begin(), rhs.end()))
-          return total.getSize(TC, DC);
-
-        auto i = lhs.begin();
-        auto j = rhs.begin();
-
-        size_t totalSize = 0;
-        for (; i != lhs.end() && j != rhs.end(); ++i, ++j) {
-          // The only light-weight checking we can do
-          // is when sub-spaces on both sides are types
-          // otherwise we'd have to decompose, which
-          // is too heavy, so let's just return total
-          // space size if such situation is detected.
-          if (i->getKind() != SpaceKind::Type ||
-              j->getKind() != SpaceKind::Type)
-            return total.getSize(TC, DC);
-
-          totalSize += estimateSearchSpaceSize(*i, *j);
-        }
-
-        return totalSize;
-      }
-
-      default:
-        break;
-      }
-
-      return total.getSize(TC, DC);
-    }
-
     void checkExhaustiveness(bool limitedChecking) {
       // If the type of the scrutinee is uninhabited, we're already dead.
       // Allow any well-typed patterns through.
@@ -1008,7 +902,6 @@ namespace {
         return;
       }
 
-      bool sawRedundantPattern = false;
       const CaseStmt *unknownCase = nullptr;
       SmallVector<Space, 4> spaces;
       for (auto *caseBlock : Switch->getCases()) {
@@ -1032,8 +925,6 @@ namespace {
 
           if (!projection.isEmpty() &&
               projection.isSubspace(Space::forDisjunct(spaces), TC, DC)) {
-            sawRedundantPattern |= true;
-
             TC.diagnose(caseItem.getStartLoc(),
                           diag::redundant_particular_case)
               .highlight(caseItem.getSourceRange());
@@ -1058,18 +949,11 @@ namespace {
       Space totalSpace = Space::forType(subjectType, Identifier());
       Space coveredSpace = Space::forDisjunct(spaces);
 
-      const size_t searchSpaceSizeEstimate =
-          estimateSearchSpaceSize(totalSpace, coveredSpace);
-      if (searchSpaceSizeEstimate > Space::getMaximumSize()) {
-        diagnoseCannotCheck(sawRedundantPattern, totalSpace, coveredSpace,
-                            unknownCase);
-        return;
-      }
       unsigned minusCount = 0;
       auto diff = totalSpace.minus(coveredSpace, TC, DC, &minusCount);
       if (!diff) {
-        diagnoseCannotCheck(sawRedundantPattern, totalSpace, coveredSpace,
-                            unknownCase);
+        diagnoseMissingCases(RequiresDefault::SpaceTooLarge, Space(),
+                             unknownCase);
         return;
       }
 
@@ -1115,26 +999,7 @@ namespace {
       UncoveredSwitch,
       SpaceTooLarge,
     };
-    
-    void diagnoseCannotCheck(const bool sawRedundantPattern,
-                             const Space &totalSpace,
-                             const Space &coveredSpace,
-                             const CaseStmt *unknownCase) {
-      // Because the space is large or the check is too slow,
-      // we have to extend the size
-      // heuristic to compensate for actually exhaustively pattern matching
-      // over enormous spaces.  In this case, if the covered space covers
-      // as much as the total space, and there were no duplicates, then we
-      // can assume the user did the right thing and that they don't need
-      // a 'default' to be inserted.
-      // FIXME: Do something sensible for non-frozen enums.
-      if (!sawRedundantPattern &&
-          coveredSpace.getSize(TC, DC) >= totalSpace.getSize(TC, DC))
-        return;
-      diagnoseMissingCases(RequiresDefault::SpaceTooLarge, Space(),
-                           unknownCase);
-    }
-    
+
     void diagnoseMissingCases(RequiresDefault defaultReason, Space uncovered,
                               const CaseStmt *unknownCase = nullptr) {
       SourceLoc startLoc = Switch->getStartLoc();

test/Compatibility/exhaustive_switch.swift

@@ -441,8 +441,8 @@ enum ContainsOverlyLargeEnum {
 }
 
 func quiteBigEnough() -> Bool {
-  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{the compiler is unable to check that this switch is exhaustive in reasonable time}}
-  // expected-note@-1 {{do you want to add a default clause?}}
+  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
+  // expected-note@-1 132 {{add missing case:}}
   case (.case0, .case0): return true
   case (.case1, .case1): return true
   case (.case2, .case2): return true
@@ -457,8 +457,8 @@ func quiteBigEnough() -> Bool {
   case (.case11, .case11): return true
   }
 
-  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{the compiler is unable to check that this switch is exhaustive in reasonable time}}
-  // expected-note@-1 {{do you want to add a default clause?}}
+  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
+  // expected-note@-1 {{add missing case: '(.case11, _)'}}
   case (.case0, _): return true
   case (.case1, _): return true
   case (.case2, _): return true
@@ -472,7 +472,8 @@ func quiteBigEnough() -> Bool {
   case (.case10, _): return true
   }
 
-  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{the compiler is unable to check that this switch is exhaustive in reasonable time}}
+  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-warning {{switch must be exhaustive}}
+  // expected-note@-1 {{add missing case: '(.case11, _)'}}
   case (.case0, _): return true
   case (.case1, _): return true
   case (.case2, _): return true
@@ -484,7 +485,7 @@ func quiteBigEnough() -> Bool {
   case (.case8, _): return true
   case (.case9, _): return true
   case (.case10, _): return true
-  @unknown default: return false // expected-note {{remove '@unknown' to handle remaining values}} {{3-12=}}
+  @unknown default: return false
   }