Stricter enforcement of the "large space" heuristic

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,
-        "%select{case|default}0 will never be executed", (bool))
+        "case will never be executed", ())
 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

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

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -169,6 +169,7 @@ namespace {
         }
         }
       }
+
 
     public:
       explicit Space(Type T, Identifier NameForPrinting)
@@ -201,6 +202,29 @@ 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;
       }
@@ -1011,14 +1035,16 @@ 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;
       }
 
@@ -1063,23 +1089,18 @@ namespace {
 
       Space totalSpace(subjectType, Identifier());
       Space coveredSpace(spaces);
-      size_t totalSpaceSize = totalSpace.getSize(TC);
-      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());
+      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());
         return;
       }
-      
+
       auto uncovered = totalSpace.minus(coveredSpace, TC).simplify(TC);
       if (uncovered.isEmpty()) {
         return;
@@ -1092,11 +1113,12 @@ namespace {
         if (Space::canDecompose(uncovered.getType())) {
           SmallVector<Space, 4> spaces;
           Space::decompose(TC, uncovered.getType(), spaces);
-          diagnoseMissingCases(TC, Switch,
-                               /*justNeedsDefault*/ false, Space(spaces));
+          diagnoseMissingCases(TC, Switch, RequiresDefault::No, Space(spaces));
         } else {
-          diagnoseMissingCases(TC, Switch,
-                               /*justNeedsDefault*/ true, Space());
+          diagnoseMissingCases(TC, Switch, Switch->getCases().empty()
+                                            ? RequiresDefault::EmptySwitchBody
+                                            : RequiresDefault::UncoveredSwitch,
+                               Space());
         }
         return;
       }
@@ -1107,7 +1129,7 @@ namespace {
         uncovered = Space(spaces);
       }
 
-      diagnoseMissingCases(TC, Switch, /*justNeedsDefault*/ false, uncovered,
+      diagnoseMissingCases(TC, Switch, RequiresDefault::No, uncovered,
                            sawDowngradablePattern);
     }
 
@@ -1139,8 +1161,15 @@ namespace {
       }
     }
 
+    enum class RequiresDefault {
+      No,
+      EmptySwitchBody,
+      UncoveredSwitch,
+      SpaceTooLarge,
+    };
+
     static void diagnoseMissingCases(TypeChecker &TC, const SwitchStmt *SS,
-                                     bool justNeedsDefault,
+                                     RequiresDefault defaultReason,
                                      Space uncovered,
                                      bool sawDowngradablePattern = false) {
       SourceLoc startLoc = SS->getStartLoc();
@@ -1149,20 +1178,30 @@ namespace {
       llvm::SmallString<128> buffer;
       llvm::raw_svector_ostream OS(buffer);
 
-      bool InEditor = TC.Context.LangOpts.DiagnosticsEditorMode;
-
-      if (justNeedsDefault) {
+      switch (defaultReason) {
+      case RequiresDefault::EmptySwitchBody: {
         OS << tok::kw_default << ":\n" << placeholder << "\n";
-        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());
-        }
+        TC.diagnose(startLoc, diag::empty_switch_stmt)
+          .fixItInsert(endLoc, buffer.str());
+      }
+        return;
+      case RequiresDefault::UncoveredSwitch: {
+        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());
+      }
+        return;
+      case RequiresDefault::No:
+        // Break out to diagnose below.
+        break;
       }
 
       // If there's nothing else to diagnose, bail.
@@ -1189,7 +1228,7 @@ namespace {
       //
       // missing case '(.none, .some(_))'
       // missing case '(.some(_), .none)'
-      if (InEditor) {
+      if (TC.Context.LangOpts.DiagnosticsEditorMode) {
         buffer.clear();
         SmallVector<Space, 8> emittedSpaces;
         for (auto &uncoveredSpace : uncovered.getSpaces()) {
@@ -1212,7 +1251,7 @@ namespace {
         TC.diagnose(startLoc, diag::missing_several_cases, false)
           .fixItInsert(endLoc, buffer.str());
       } else {
-        TC.Context.Diags.diagnose(startLoc, mainDiagType);
+        TC.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:  // expected-warning {{default will never be executed}}
+  default:  
     return
   }
 }

test/SILOptimizer/unreachable_code.swift

@@ -208,7 +208,7 @@ class r20097963MyClass {
       str = "A"
     case .B:
       str = "B"
-    default:    // expected-warning {{default will never be executed}}
+    default: 
       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 {{switch must be exhaustive}}
+  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // 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 (.case0, .case0): return true
   case (.case1, .case1): return true
@@ -460,8 +460,7 @@ func quiteBigEnough() -> Bool {
   case (.case11, .case11): return true
   }
 
-  // No diagnostic
-  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
+  switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // 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 (.case0, _): return true
   case (.case1, _): return true
@@ -477,8 +476,9 @@ func quiteBigEnough() -> Bool {
   }
 
 
-  // No diagnostic
+  // expected-error@+1 {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}}
   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,8 +493,9 @@ func quiteBigEnough() -> Bool {
   case (.case11, _): return true
   }
 
-  // No diagnostic
+  // expected-error@+1 {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}}
   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
@@ -509,21 +510,22 @@ func quiteBigEnough() -> Bool {
   case (_, .case11): return true
   }
 
-  // No diagnostic
+  // No diagnostic 
   switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
   case (_, _): return true
   }
 
-  // No diagnostic
+  // expected-error@+1 {{analysis of uncovered switch statement is too complex to perform in a reasonable amount of time}} 
   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
@@ -559,6 +561,28 @@ 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