Skip to content

Fix a pair of crashes with '@unknown' #15889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 17, 2018
Merged

Fix a pair of crashes with '@unknown' #15889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9f26ab
Don't crash when using `@unknown default` with a non-enum type.
jrose-apple Apr 12, 2018
8157e80
Limit exhaustivity checking when weird things happen with '@unknown'
jrose-apple Apr 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3925,6 +3925,9 @@ NOTE(missing_several_cases,none,
NOTE(missing_unknown_case,none,
"handle unknown values using \"@unknown default\"", ())

NOTE(non_exhaustive_switch_drop_unknown,none,
"remove '@unknown' to handle remaining values", ())

NOTE(missing_particular_case,none,
"add missing case: '%0'", (StringRef))
WARNING(redundant_particular_case,none,
Expand Down
19 changes: 11 additions & 8 deletions lib/Sema/TypeCheckStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {

template<typename StmtTy>
bool typeCheckStmt(StmtTy *&S) {
PrettyStackTraceStmt trace(TC.Context, "type-checking", S);
StmtTy *S2 = cast_or_null<StmtTy>(visit(S));
if (S2 == nullptr)
return true;
Expand Down Expand Up @@ -821,7 +822,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
// Type-check the subject expression.
Expr *subjectExpr = S->getSubjectExpr();
auto resultTy = TC.typeCheckExpression(subjectExpr, DC);
auto hadError = !resultTy;
auto limitExhaustivityChecks = !resultTy;
if (Expr *newSubjectExpr = TC.coerceToRValue(subjectExpr))
subjectExpr = newSubjectExpr;
S->setSubjectExpr(subjectExpr);
Expand Down Expand Up @@ -856,7 +857,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
// Coerce the pattern to the subject's type.
if (!subjectType || TC.coercePatternToType(pattern, DC, subjectType,
TypeResolutionFlags::InExpression)) {
hadError = true;
limitExhaustivityChecks = true;

// If that failed, mark any variables binding pieces of the pattern
// as invalid to silence follow-on errors.
Expand Down Expand Up @@ -907,7 +908,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
}
// Check the guard expression, if present.
if (auto *guard = labelItem.getGuardExpr()) {
hadError |= TC.typeCheckCondition(guard, DC);
limitExhaustivityChecks |= TC.typeCheckCondition(guard, DC);
labelItem.setGuardExpr(guard);
}
}
Expand All @@ -920,11 +921,13 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
TC.diagnose(caseBlock->getLoc(),
diag::unknown_case_multiple_patterns)
.highlight(caseBlock->getCaseLabelItems()[1].getSourceRange());
limitExhaustivityChecks = true;
}

if (!caseBlock->isDefault() && FallthroughDest != nullptr) {
TC.diagnose(caseBlock->getLoc(),
diag::unknown_case_must_be_last);
if (FallthroughDest != nullptr) {
if (!caseBlock->isDefault())
TC.diagnose(caseBlock->getLoc(), diag::unknown_case_must_be_last);
limitExhaustivityChecks = true;
}

const CaseLabelItem &labelItem = caseBlock->getCaseLabelItems().front();
Expand Down Expand Up @@ -981,13 +984,13 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
// Type-check the body statements.
PreviousFallthrough = nullptr;
Stmt *body = caseBlock->getBody();
hadError |= typeCheckStmt(body);
limitExhaustivityChecks |= typeCheckStmt(body);
caseBlock->setBody(body);
previousBlock = caseBlock;
}

if (!S->isImplicit()) {
TC.checkSwitchExhaustiveness(S, DC, /*limitChecking*/hadError);
TC.checkSwitchExhaustiveness(S, DC, limitExhaustivityChecks);
}

return S;
Expand Down
23 changes: 19 additions & 4 deletions lib/Sema/TypeCheckSwitchStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,7 @@ namespace {
SmallVector<Space, 4> spaces;
for (auto *caseBlock : Switch->getCases()) {
if (caseBlock->hasUnknownAttr()) {
assert(unknownCase == nullptr);
assert(unknownCase == nullptr && "multiple unknown cases");
unknownCase = caseBlock;
continue;
}
Expand Down Expand Up @@ -1342,10 +1342,25 @@ namespace {

// Decide whether we want an error or a warning.
auto mainDiagType = diag::non_exhaustive_switch;
if (!uncovered.isEmpty() && unknownCase) {
assert(defaultReason == RequiresDefault::No);
mainDiagType = diag::non_exhaustive_switch_warn;
if (unknownCase) {
switch (defaultReason) {
case RequiresDefault::EmptySwitchBody:
llvm_unreachable("there's an @unknown case; the body can't be empty");
case RequiresDefault::No:
if (!uncovered.isEmpty())
mainDiagType = diag::non_exhaustive_switch_warn;
break;
case RequiresDefault::UncoveredSwitch:
case RequiresDefault::SpaceTooLarge:
TC.diagnose(startLoc, diag::non_exhaustive_switch);
TC.diagnose(unknownCase->getLoc(),
diag::non_exhaustive_switch_drop_unknown)
.fixItRemoveChars(unknownCase->getStartLoc(),
unknownCase->getLoc());
return;
}
}

switch (uncovered.checkDowngradeToWarning()) {
case DowngradeToWarning::No:
break;
Expand Down
19 changes: 19 additions & 0 deletions test/Compatibility/exhaustive_switch.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,21 @@ func quiteBigEnough() -> Bool {
case (.case10, _): return true
}

switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
case (.case0, _): return true
case (.case1, _): return true
case (.case2, _): return true
case (.case3, _): return true
case (.case4, _): return true
case (.case5, _): return true
case (.case6, _): return true
case (.case7, _): return true
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=}}
}


// No diagnostic
switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
Expand Down Expand Up @@ -526,6 +541,10 @@ func quiteBigEnough() -> Bool {
case .two: return true
case .three: return true
}

// Make sure we haven't just stopped emitting diagnostics.
switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}} expected-note {{handle unknown values}}
}
}

indirect enum InfinitelySized {
Expand Down
91 changes: 89 additions & 2 deletions test/Parse/switch.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,12 +427,12 @@ case .Thing: // expected-error{{additional 'case' blocks cannot appear after the
break
}

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

switch Whatever.Thing { // expected-warning {{switch must be exhaustive}} expected-note{{add missing case: '.Thing'}}
switch Whatever.Thing {
@unknown case _, _, _: // expected-error {{'@unknown' cannot be applied to multiple patterns}}
break
}
Expand Down Expand Up @@ -522,3 +522,90 @@ switch Whatever.Thing { // expected-warning {{switch must be exhaustive}} expect
case _:
break
}

switch x { // expected-error {{switch must be exhaustive}}
case 1:
break
@unknown case _: // expected-note {{remove '@unknown' to handle remaining values}} {{1-10=}}
break
}

switch x { // expected-error {{switch must be exhaustive}}
@unknown case _: // expected-note {{remove '@unknown' to handle remaining values}} {{1-10=}}
break
}

switch x { // expected-error {{switch must be exhaustive}}
@unknown default: // expected-note {{remove '@unknown' to handle remaining values}} {{1-10=}}
break
}

switch Whatever.Thing {
case .Thing:
break
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
break
@unknown case _:
break
}

switch Whatever.Thing {
case .Thing:
break
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
break
@unknown default:
break
}

switch Whatever.Thing {
case .Thing:
break
@unknown default:
break
@unknown default: // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}}
break
}

switch Whatever.Thing {
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
break
@unknown case _:
break
}

switch Whatever.Thing {
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
break
@unknown default:
break
}

switch Whatever.Thing {
@unknown default:
break
@unknown default: // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}}
break
}


switch x {
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
break
@unknown case _:
break
}

switch x {
@unknown case _: // expected-error {{'@unknown' can only be applied to the last case in a switch}}
break
@unknown default:
break
}

switch x {
@unknown default:
break
@unknown default: // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}}
break
}
19 changes: 19 additions & 0 deletions test/Sema/exhaustive_switch.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,21 @@ func quiteBigEnough() -> Bool {
case (.case10, _): return true
}

switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) { // expected-error {{switch must be exhaustive}}
case (.case0, _): return true
case (.case1, _): return true
case (.case2, _): return true
case (.case3, _): return true
case (.case4, _): return true
case (.case5, _): return true
case (.case6, _): return true
case (.case7, _): return true
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=}}
}


// No diagnostic
switch (OverlyLargeSpaceEnum.case1, OverlyLargeSpaceEnum.case2) {
Expand Down Expand Up @@ -525,6 +540,10 @@ func quiteBigEnough() -> Bool {
case .two: return true
case .three: return true
}

// Make sure we haven't just stopped emitting diagnostics.
switch OverlyLargeSpaceEnum.case1 { // expected-error {{switch must be exhaustive}} expected-note 12 {{add missing case}} expected-note {{handle unknown values}}
}
}

indirect enum InfinitelySized {
Expand Down