Skip to content

Commit 8157e80

Browse files
committed
Limit exhaustivity checking when weird things happen with '@unknown'
If someone uses it twice, or on a non-final case, or tries to use it with a multiple-pattern case, don't bother checking the exhaustiveness of the switch. (For other violations, the pattern or where-clause is ignored.) https://bugs.swift.org/browse/SR-7409
1 parent a9f26ab commit 8157e80

File tree

3 files changed

+84
-11
lines changed

3 files changed

+84
-11
lines changed

lib/Sema/TypeCheckStmt.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
377377

378378
template<typename StmtTy>
379379
bool typeCheckStmt(StmtTy *&S) {
380+
PrettyStackTraceStmt trace(TC.Context, "type-checking", S);
380381
StmtTy *S2 = cast_or_null<StmtTy>(visit(S));
381382
if (S2 == nullptr)
382383
return true;
@@ -821,7 +822,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
821822
// Type-check the subject expression.
822823
Expr *subjectExpr = S->getSubjectExpr();
823824
auto resultTy = TC.typeCheckExpression(subjectExpr, DC);
824-
auto hadError = !resultTy;
825+
auto limitExhaustivityChecks = !resultTy;
825826
if (Expr *newSubjectExpr = TC.coerceToRValue(subjectExpr))
826827
subjectExpr = newSubjectExpr;
827828
S->setSubjectExpr(subjectExpr);
@@ -856,7 +857,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
856857
// Coerce the pattern to the subject's type.
857858
if (!subjectType || TC.coercePatternToType(pattern, DC, subjectType,
858859
TypeResolutionFlags::InExpression)) {
859-
hadError = true;
860+
limitExhaustivityChecks = true;
860861

861862
// If that failed, mark any variables binding pieces of the pattern
862863
// as invalid to silence follow-on errors.
@@ -907,7 +908,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
907908
}
908909
// Check the guard expression, if present.
909910
if (auto *guard = labelItem.getGuardExpr()) {
910-
hadError |= TC.typeCheckCondition(guard, DC);
911+
limitExhaustivityChecks |= TC.typeCheckCondition(guard, DC);
911912
labelItem.setGuardExpr(guard);
912913
}
913914
}
@@ -920,11 +921,13 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
920921
TC.diagnose(caseBlock->getLoc(),
921922
diag::unknown_case_multiple_patterns)
922923
.highlight(caseBlock->getCaseLabelItems()[1].getSourceRange());
924+
limitExhaustivityChecks = true;
923925
}
924926

925-
if (!caseBlock->isDefault() && FallthroughDest != nullptr) {
926-
TC.diagnose(caseBlock->getLoc(),
927-
diag::unknown_case_must_be_last);
927+
if (FallthroughDest != nullptr) {
928+
if (!caseBlock->isDefault())
929+
TC.diagnose(caseBlock->getLoc(), diag::unknown_case_must_be_last);
930+
limitExhaustivityChecks = true;
928931
}
929932

930933
const CaseLabelItem &labelItem = caseBlock->getCaseLabelItems().front();
@@ -981,13 +984,13 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
981984
// Type-check the body statements.
982985
PreviousFallthrough = nullptr;
983986
Stmt *body = caseBlock->getBody();
984-
hadError |= typeCheckStmt(body);
987+
limitExhaustivityChecks |= typeCheckStmt(body);
985988
caseBlock->setBody(body);
986989
previousBlock = caseBlock;
987990
}
988991

989992
if (!S->isImplicit()) {
990-
TC.checkSwitchExhaustiveness(S, DC, /*limitChecking*/hadError);
993+
TC.checkSwitchExhaustiveness(S, DC, limitExhaustivityChecks);
991994
}
992995

993996
return S;

lib/Sema/TypeCheckSwitchStmt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,7 @@ namespace {
12161216
SmallVector<Space, 4> spaces;
12171217
for (auto *caseBlock : Switch->getCases()) {
12181218
if (caseBlock->hasUnknownAttr()) {
1219-
assert(unknownCase == nullptr);
1219+
assert(unknownCase == nullptr && "multiple unknown cases");
12201220
unknownCase = caseBlock;
12211221
continue;
12221222
}

test/Parse/switch.swift

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,12 +427,12 @@ case .Thing: // expected-error{{additional 'case' blocks cannot appear after the
427427
break
428428
}
429429

430-
switch Whatever.Thing { // expected-warning {{switch must be exhaustive}} expected-note{{add missing case: '.Thing'}}
430+
switch Whatever.Thing {
431431
@unknown case _, _: // expected-error {{'@unknown' cannot be applied to multiple patterns}}
432432
break
433433
}
434434

435-
switch Whatever.Thing { // expected-warning {{switch must be exhaustive}} expected-note{{add missing case: '.Thing'}}
435+
switch Whatever.Thing {
436436
@unknown case _, _, _: // expected-error {{'@unknown' cannot be applied to multiple patterns}}
437437
break
438438
}
@@ -539,3 +539,73 @@ switch x { // expected-error {{switch must be exhaustive}}
539539
@unknown default: // expected-note {{remove '@unknown' to handle remaining values}} {{1-10=}}
540540
break
541541
}
542+
543+
switch Whatever.Thing {
544+
case .Thing:
545+
break
546+
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
547+
break
548+
@unknown case _:
549+
break
550+
}
551+
552+
switch Whatever.Thing {
553+
case .Thing:
554+
break
555+
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
556+
break
557+
@unknown default:
558+
break
559+
}
560+
561+
switch Whatever.Thing {
562+
case .Thing:
563+
break
564+
@unknown default:
565+
break
566+
@unknown default: // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}}
567+
break
568+
}
569+
570+
switch Whatever.Thing {
571+
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
572+
break
573+
@unknown case _:
574+
break
575+
}
576+
577+
switch Whatever.Thing {
578+
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
579+
break
580+
@unknown default:
581+
break
582+
}
583+
584+
switch Whatever.Thing {
585+
@unknown default:
586+
break
587+
@unknown default: // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}}
588+
break
589+
}
590+
591+
592+
switch x {
593+
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
594+
break
595+
@unknown case _:
596+
break
597+
}
598+
599+
switch x {
600+
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
601+
break
602+
@unknown default:
603+
break
604+
}
605+
606+
switch x {
607+
@unknown default:
608+
break
609+
@unknown default: // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}}
610+
break
611+
}

0 commit comments

Comments
 (0)