Skip to content

Commit 275e101

Browse files
jrose-applejrose-apple
authored
Merge pull request #15889 from jrose-apple/ready-for-the-unknown
Fix a pair of crashes with '@unknown'
2 parents 76eceb0 + 8157e80 commit 275e101

File tree

6 files changed

+160
-14
lines changed

6 files changed

+160
-14
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3937,6 +3937,9 @@ NOTE(missing_several_cases,none,
39373937
NOTE(missing_unknown_case,none,
39383938
"handle unknown values using \"@unknown default\"", ())
39393939

3940+
NOTE(non_exhaustive_switch_drop_unknown,none,
3941+
"remove '@unknown' to handle remaining values", ())
3942+
39403943
NOTE(missing_particular_case,none,
39413944
"add missing case: '%0'", (StringRef))
39423945
WARNING(redundant_particular_case,none,

‎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: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,7 +1219,7 @@ namespace {
12191219
SmallVector<Space, 4> spaces;
12201220
for (auto *caseBlock : Switch->getCases()) {
12211221
if (caseBlock->hasUnknownAttr()) {
1222-
assert(unknownCase == nullptr);
1222+
assert(unknownCase == nullptr && "multiple unknown cases");
12231223
unknownCase = caseBlock;
12241224
continue;
12251225
}
@@ -1345,10 +1345,25 @@ namespace {
13451345

13461346
// Decide whether we want an error or a warning.
13471347
auto mainDiagType = diag::non_exhaustive_switch;
1348-
if (!uncovered.isEmpty() && unknownCase) {
1349-
assert(defaultReason == RequiresDefault::No);
1350-
mainDiagType = diag::non_exhaustive_switch_warn;
1348+
if (unknownCase) {
1349+
switch (defaultReason) {
1350+
case RequiresDefault::EmptySwitchBody:
1351+
llvm_unreachable("there's an @unknown case; the body can't be empty");
1352+
case RequiresDefault::No:
1353+
if (!uncovered.isEmpty())
1354+
mainDiagType = diag::non_exhaustive_switch_warn;
1355+
break;
1356+
case RequiresDefault::UncoveredSwitch:
1357+
case RequiresDefault::SpaceTooLarge:
1358+
TC.diagnose(startLoc, diag::non_exhaustive_switch);
1359+
TC.diagnose(unknownCase->getLoc(),
1360+
diag::non_exhaustive_switch_drop_unknown)
1361+
.fixItRemoveChars(unknownCase->getStartLoc(),
1362+
unknownCase->getLoc());
1363+
return;
1364+
}
13511365
}
1366+
13521367
switch (uncovered.checkDowngradeToWarning()) {
13531368
case DowngradeToWarning::No:
13541369
break;

‎test/Compatibility/exhaustive_switch.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,21 @@ func quiteBigEnough() -> Bool {
473473
case (.case10, _): return true
474474
}
475475

476+
switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
477+
case (.case0, _): return true
478+
case (.case1, _): return true
479+
case (.case2, _): return true
480+
case (.case3, _): return true
481+
case (.case4, _): return true
482+
case (.case5, _): return true
483+
case (.case6, _): return true
484+
case (.case7, _): return true
485+
case (.case8, _): return true
486+
case (.case9, _): return true
487+
case (.case10, _): return true
488+
@unknown default: return false // expected-note {{remove '@unknown' to handle remaining values}} {{3-12=}}
489+
}
490+
476491

477492
// No diagnostic
478493
switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
@@ -526,6 +541,10 @@ func quiteBigEnough() -> Bool {
526541
case .two: return true
527542
case .three: return true
528543
}
544+
545+
// Make sure we haven't just stopped emitting diagnostics.
546+
switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}} expected-note {{handle unknown values}}
547+
}
529548
}
530549

531550
indirect enum InfinitelySized {

‎test/Parse/switch.swift

Lines changed: 89 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
}
@@ -522,3 +522,90 @@ switch Whatever.Thing { // expected-warning {{switch must be exhaustive}} expect
522522
case _:
523523
break
524524
}
525+
526+
switch x { // expected-error {{switch must be exhaustive}}
527+
case 1:
528+
break
529+
@unknown case _: // expected-note {{remove '@unknown' to handle remaining values}} {{1-10=}}
530+
break
531+
}
532+
533+
switch x { // expected-error {{switch must be exhaustive}}
534+
@unknown case _: // expected-note {{remove '@unknown' to handle remaining values}} {{1-10=}}
535+
break
536+
}
537+
538+
switch x { // expected-error {{switch must be exhaustive}}
539+
@unknown default: // expected-note {{remove '@unknown' to handle remaining values}} {{1-10=}}
540+
break
541+
}
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+
}

‎test/Sema/exhaustive_switch.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,21 @@ func quiteBigEnough() -> Bool {
472472
case (.case10, _): return true
473473
}
474474

475+
switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
476+
case (.case0, _): return true
477+
case (.case1, _): return true
478+
case (.case2, _): return true
479+
case (.case3, _): return true
480+
case (.case4, _): return true
481+
case (.case5, _): return true
482+
case (.case6, _): return true
483+
case (.case7, _): return true
484+
case (.case8, _): return true
485+
case (.case9, _): return true
486+
case (.case10, _): return true
487+
@unknown default: return false // expected-note {{remove '@unknown' to handle remaining values}} {{3-12=}}
488+
}
489+
475490

476491
// No diagnostic
477492
switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
@@ -525,6 +540,10 @@ func quiteBigEnough() -> Bool {
525540
case .two: return true
526541
case .three: return true
527542
}
543+
544+
// Make sure we haven't just stopped emitting diagnostics.
545+
switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}} expected-note {{handle unknown values}}
546+
}
528547
}
529548

530549
indirect enum InfinitelySized {

0 commit comments

Comments
 (0)