Revert "Stricter enforcement of the "large space" heuristic"

Author: xedin

include/swift/AST/DiagnosticsSIL.def

@@ -244,7 +244,7 @@ WARNING(unreachable_code_after_stmt,none,
         "code after '%select{return|break|continue|throw}0' will never "
         "be executed", (unsigned))
 WARNING(unreachable_case,none,
-        "case will never be executed", ())
+        "%select{case|default}0 will never be executed", (bool))
 WARNING(switch_on_a_constant,none,
         "switch condition evaluates to a constant", ())
 NOTE(unreachable_code_note,none, "will never be executed", ())

include/swift/AST/DiagnosticsSema.def

@@ -3858,11 +3858,6 @@ WARNING(redundant_particular_literal_case,none,
 NOTE(redundant_particular_literal_case_here,none,
      "first occurrence of identical literal pattern is here", ())
 
-ERROR(cannot_prove_exhaustive_switch,none,
-      "analysis of uncovered switch statement is too complex to perform in a "
-      "reasonable amount of time; "
-      "insert a 'default' clause to cover this switch statement", ())
-
 // HACK: Downgrades the above to warnings if any of the cases is marked
 // @_downgrade_exhaustivity_check.
 WARNING(non_exhaustive_switch_warn_swift3,none, "switch must be exhaustive", ())

lib/SILGen/SILGenPattern.cpp

@@ -1031,8 +1031,7 @@ void PatternMatchEmission::emitDispatch(ClauseMatrix &clauses, ArgArray args,
         } else {
           Loc = clauses[firstRow].getCasePattern()->getStartLoc();
         }
-        if (!isDefault)
-          SGF.SGM.diagnose(Loc, diag::unreachable_case);
+        SGF.SGM.diagnose(Loc, diag::unreachable_case, isDefault);
       }
     }
   }

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -169,7 +169,6 @@ namespace {
         }
         }
       }
-
 
     public:
       explicit Space(Type T, Identifier NameForPrinting)
@@ -202,29 +201,6 @@ namespace {
         return computeSize(TC, cache);
       }
 
-      // Walk one level deep into the space to return whether it is
-      // composed entirely of irrefutable patterns - these are quick to check
-      // regardless of the size of the total type space.
-      bool isAllIrrefutable() const {
-        switch (getKind()) {
-        case SpaceKind::Empty:
-        case SpaceKind::Type:
-          return true;
-        case SpaceKind::BooleanConstant:
-          return false;
-        case SpaceKind::Constructor:
-          return llvm::all_of(getSpaces(), [](const Space &sp) {
-            return sp.getKind() == SpaceKind::Type
-                || sp.getKind() == SpaceKind::Empty;
-          });
-        case SpaceKind::Disjunct: {
-          return llvm::all_of(getSpaces(), [](const Space &sp) {
-            return sp.isAllIrrefutable();
-          });
-        }
-        }
-      }
-
       static size_t getMaximumSize() {
         return MAX_SPACE_SIZE;
       }
@@ -1035,16 +1011,14 @@ namespace {
       if (subjectType && subjectType->isStructurallyUninhabited()) {
         return;
       }
-
-      // Reject switch statements with empty blocks.
-      if (limitedChecking && Switch->getCases().empty()) {
-        SpaceEngine::diagnoseMissingCases(TC, Switch,
-                                          RequiresDefault::EmptySwitchBody,
-                                          SpaceEngine::Space());
-      }
-
-      // If the switch body fails to typecheck, end analysis here.
+      
       if (limitedChecking) {
+        // Reject switch statements with empty blocks.
+        if (Switch->getCases().empty()) {
+          SpaceEngine::diagnoseMissingCases(TC, Switch,
+                                            /*justNeedsDefault*/true,
+                                            SpaceEngine::Space());
+        }
         return;
       }
 
@@ -1089,18 +1063,23 @@ namespace {
 
       Space totalSpace(subjectType, Identifier());
       Space coveredSpace(spaces);
-
       size_t totalSpaceSize = totalSpace.getSize(TC);
-      if (totalSpaceSize > Space::getMaximumSize() && !coveredSpace.isAllIrrefutable()) {
-        // Because the space is large, fall back to requiring 'default'.
-        //
-        // FIXME: Explore ways of reducing runtime of this analysis or doing
-        // partial analysis to recover this case.
-        diagnoseMissingCases(TC, Switch,
-                             RequiresDefault::SpaceTooLarge, Space());
+      if (totalSpaceSize > Space::getMaximumSize()) {
+        // Because the space is large, 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.
+        if (!sawRedundantPattern
+            && coveredSpace.getSize(TC) >= totalSpaceSize) {
+          return;
+        }
+
+        diagnoseMissingCases(TC, Switch, /*justNeedsDefault*/true, Space());
         return;
       }
-
+      
       auto uncovered = totalSpace.minus(coveredSpace, TC).simplify(TC);
       if (uncovered.isEmpty()) {
         return;
@@ -1113,12 +1092,11 @@ namespace {
         if (Space::canDecompose(uncovered.getType())) {
           SmallVector<Space, 4> spaces;
           Space::decompose(TC, uncovered.getType(), spaces);
-          diagnoseMissingCases(TC, Switch, RequiresDefault::No, Space(spaces));
+          diagnoseMissingCases(TC, Switch,
+                               /*justNeedsDefault*/ false, Space(spaces));
         } else {
-          diagnoseMissingCases(TC, Switch, Switch->getCases().empty()
-                                            ? RequiresDefault::EmptySwitchBody
-                                            : RequiresDefault::UncoveredSwitch,
-                               Space());
+          diagnoseMissingCases(TC, Switch,
+                               /*justNeedsDefault*/ true, Space());
         }
         return;
       }
@@ -1129,7 +1107,7 @@ namespace {
         uncovered = Space(spaces);
       }
 
-      diagnoseMissingCases(TC, Switch, RequiresDefault::No, uncovered,
+      diagnoseMissingCases(TC, Switch, /*justNeedsDefault*/ false, uncovered,
                            sawDowngradablePattern);
     }
 
@@ -1161,15 +1139,8 @@ namespace {
       }
     }
 
-    enum class RequiresDefault {
-      No,
-      EmptySwitchBody,
-      UncoveredSwitch,
-      SpaceTooLarge,
-    };
-
     static void diagnoseMissingCases(TypeChecker &TC, const SwitchStmt *SS,
-                                     RequiresDefault defaultReason,
+                                     bool justNeedsDefault,
                                      Space uncovered,
                                      bool sawDowngradablePattern = false) {
       SourceLoc startLoc = SS->getStartLoc();
@@ -1178,30 +1149,20 @@ namespace {
       llvm::SmallString<128> buffer;
       llvm::raw_svector_ostream OS(buffer);
 
-      switch (defaultReason) {
-      case RequiresDefault::EmptySwitchBody: {
-        OS << tok::kw_default << ":\n" << placeholder << "\n";
-        TC.diagnose(startLoc, diag::empty_switch_stmt)
-          .fixItInsert(endLoc, buffer.str());
-      }
-        return;
-      case RequiresDefault::UncoveredSwitch: {
+      bool InEditor = TC.Context.LangOpts.DiagnosticsEditorMode;
+
+      if (justNeedsDefault) {
         OS << tok::kw_default << ":\n" << placeholder << "\n";
-        TC.diagnose(startLoc, diag::non_exhaustive_switch);
-        TC.diagnose(startLoc, diag::missing_several_cases, uncovered.isEmpty())
-          .fixItInsert(endLoc, buffer.str());
-      }
-        return;
-      case RequiresDefault::SpaceTooLarge: {
-        OS << tok::kw_default << ":\n" << "<#fatalError()#>" << "\n";
-        TC.diagnose(startLoc,  diag::cannot_prove_exhaustive_switch);
-        TC.diagnose(startLoc, diag::missing_several_cases, uncovered.isEmpty())
-          .fixItInsert(endLoc, buffer.str());
-      }
+        if (SS->getCases().empty()) {
+          TC.Context.Diags.diagnose(startLoc, diag::empty_switch_stmt)
+             .fixItInsert(endLoc, buffer.str());
+        } else {
+          TC.Context.Diags.diagnose(startLoc, diag::non_exhaustive_switch);
+          TC.Context.Diags.diagnose(startLoc, diag::missing_several_cases,
+                                    uncovered.isEmpty()).fixItInsert(endLoc,
+                                                              buffer.str());
+        }
         return;
-      case RequiresDefault::No:
-        // Break out to diagnose below.
-        break;
       }
 
       // If there's nothing else to diagnose, bail.
@@ -1228,7 +1189,7 @@ namespace {
       //
       // missing case '(.none, .some(_))'
       // missing case '(.some(_), .none)'
-      if (TC.Context.LangOpts.DiagnosticsEditorMode) {
+      if (InEditor) {
         buffer.clear();
         SmallVector<Space, 8> emittedSpaces;
         for (auto &uncoveredSpace : uncovered.getSpaces()) {
@@ -1251,7 +1212,7 @@ namespace {
         TC.diagnose(startLoc, diag::missing_several_cases, false)
           .fixItInsert(endLoc, buffer.str());
       } else {
-        TC.diagnose(startLoc, mainDiagType);
+        TC.Context.Diags.diagnose(startLoc, mainDiagType);
 
         SmallVector<Space, 8> emittedSpaces;
         for (auto &uncoveredSpace : uncovered.getSpaces()) {

test/SILGen/unreachable_code.swift

@@ -106,7 +106,7 @@ func testUnreachableCase5(a : Tree) {
   switch a {
   case _:
     break
-  default:  
+  default:  // expected-warning {{default will never be executed}}
     return
   }
 }

test/SILOptimizer/unreachable_code.swift

@@ -208,7 +208,7 @@ class r20097963MyClass {
       str = "A"
     case .B:
       str = "B"
-    default: 
+    default:    // expected-warning {{default will never be executed}}
       str = "unknown"  // Should not be rejected.
     }
     return str

test/Sema/exhaustive_switch.swift

@@ -444,7 +444,7 @@ enum ContainsOverlyLargeEnum {
 }
 
 func quiteBigEnough() -> Bool {
-  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}}
+  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
   // expected-note@-1 {{do you want to add a default clause?}}
   case (.case0, .case0): return true
   case (.case1, .case1): return true
@@ -460,7 +460,8 @@ func quiteBigEnough() -> Bool {
   case (.case11, .case11): return true
   }
 
-  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}}
+  // No diagnostic
+  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
   // expected-note@-1 {{do you want to add a default clause?}}
   case (.case0, _): return true
   case (.case1, _): return true
@@ -476,9 +477,8 @@ func quiteBigEnough() -> Bool {
   }
 
 
-  // expected-error@+1 {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}}
+  // No diagnostic
   switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
-  // expected-note@-1 {{do you want to add a default clause?}}
   case (.case0, _): return true
   case (.case1, _): return true
   case (.case2, _): return true
@@ -493,9 +493,8 @@ func quiteBigEnough() -> Bool {
   case (.case11, _): return true
   }
 
-  // expected-error@+1 {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}}
+  // No diagnostic
   switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
-  // expected-note@-1 {{do you want to add a default clause?}}
   case (_, .case0): return true
   case (_, .case1): return true
   case (_, .case2): return true
@@ -510,22 +509,21 @@ func quiteBigEnough() -> Bool {
   case (_, .case11): return true
   }
 
-  // No diagnostic 
+  // No diagnostic
   switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
   case (_, _): return true
   }
 
-  // expected-error@+1 {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}} 
+  // No diagnostic
   switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
-  // expected-note@-1 {{do you want to add a default clause?}}
   case (.case0, .case0): return true
   case (.case1, .case1): return true
   case (.case2, .case2): return true
   case (.case3, .case3): return true
   case _: return true
   }
 
-  // No diagnostic 
+  // No diagnostic
   switch ContainsOverlyLargeEnum.one(.case0) {
   case .one: return true
   case .two: return true
@@ -561,28 +559,6 @@ func infinitelySized() -> Bool {
   }
 }
 
-// SR-6316: Size heuristic is insufficient to catch space covering when the
-// covered space size is greater than or equal to the master space size.
-func largeSpaceMatch(_ x: Bool) {
-  switch (x, (x, x, x, x), (x, x, x, x)) { // expected-error {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}}
-  // expected-note@-1 {{do you want to add a default clause?}}
-  case (true, (_, _, _, _), (_, true, true, _)):
-    break
-  case (true, (_, _, _, _), (_, _, false, _)):
-    break
-  case (_, (_, true, true, _), (_, _, false, _)):
-    break
-  case (_, (_, _, false, _), (_, true, true, _)):
-    break
-  case (_, (_, true, true, _), (_, true, true, _)):
-    break
-  case (_, (_, _, false, _), (_, _, false, _)):
-    break
-  case (_, (false, false, false, false), (_, _, _, _)):
-    break
-  }
-}
-
 func diagnoseDuplicateLiterals() {
   let str = "def"
   let int = 2