Skip to content

Commit daac020

Browse files
nkcsgexinkcsgexi
authored
Sema: Reject empty switch statements during type checking so that we can issue fixits to fill the unhandled switch cases. (#7766)
1 parent fbd2c99 commit daac020

File tree

12 files changed

+139
-100
lines changed

12 files changed

+139
-100
lines changed

‎include/swift/AST/DiagnosticsParse.def

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -975,10 +975,6 @@ ERROR(case_after_default,none,
975975
"additional 'case' blocks cannot appear after the 'default' block of a 'switch'",
976976
())
977977

978-
ERROR(empty_switch_stmt,none,
979-
"'switch' statement body must have at least one 'case' or 'default' block",
980-
())
981-
982978
// Case Stmt
983979
ERROR(expected_case_where_expr,PointsToFirstBadToken,
984980
"expected expression for 'where' guard of 'case'", ())

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2689,6 +2689,8 @@ ERROR(trailing_closure_requires_parens,none,
26892689
"trailing closure requires parentheses for disambiguation in this"
26902690
" context", ())
26912691

2692+
ERROR(empty_switch_stmt,none,
2693+
"'switch' statement body must have at least one 'case' or 'default' block",())
26922694
//------------------------------------------------------------------------------
26932695
// Type Check Patterns
26942696
//------------------------------------------------------------------------------

‎include/swift/AST/DiagnosticsSema.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "swift/AST/DiagnosticsCommon.h"
2222

2323
namespace swift {
24+
class SwitchStmt;
2425
namespace diag {
2526

2627
/// Describes the kind of requirement in a protocol.
@@ -36,6 +37,8 @@ namespace swift {
3637
extern detail::DiagWithArguments<void Signature>::type ID;
3738
#include "DiagnosticsSema.def"
3839
}
40+
void diagnoseMissingCases(ASTContext &Context, const SwitchStmt *SwitchS,
41+
Diagnostic Id);
3942
}
4043

4144
#endif

‎include/swift/Subsystems.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,6 @@ namespace swift {
346346
struct Implementation;
347347
Implementation &Impl;
348348
};
349-
350349
} // end namespace swift
351350

352351
#endif // SWIFT_SUBSYSTEMS_H

‎lib/Parse/ParseStmt.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2500,10 +2500,6 @@ ParserResult<Stmt> Parser::parseStmtSwitch(LabeledStmtInfo LabelInfo) {
25002500
Status.setIsParseError();
25012501
}
25022502

2503-
// Reject an empty 'switch'.
2504-
if (!DiagnosedNotCoveredStmt && cases.empty())
2505-
diagnose(rBraceLoc, diag::empty_switch_stmt);
2506-
25072503
return makeParserResult(
25082504
Status, SwitchStmt::create(LabelInfo, SwitchLoc, SubjectExpr.get(),
25092505
lBraceLoc, cases, rBraceLoc, Context));

‎lib/SILOptimizer/Mandatory/DataflowDiagnostics.cpp

Lines changed: 3 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,15 @@
1313
#include "swift/SILOptimizer/PassManager/Passes.h"
1414
#include "swift/SILOptimizer/PassManager/Transforms.h"
1515
#include "swift/AST/ASTContext.h"
16-
#include "swift/AST/ASTPrinter.h"
1716
#include "swift/AST/DiagnosticEngine.h"
17+
#include "swift/AST/DiagnosticsSema.h"
1818
#include "swift/AST/DiagnosticsSIL.h"
1919
#include "swift/AST/Expr.h"
2020
#include "swift/SIL/SILFunction.h"
2121
#include "swift/SIL/SILInstruction.h"
2222
#include "swift/SIL/SILLocation.h"
2323
#include "swift/SIL/SILModule.h"
2424
#include "swift/SIL/SILVisitor.h"
25-
#include "swift/Syntax/TokenKinds.h"
2625

2726
using namespace swift;
2827

@@ -59,89 +58,6 @@ static void diagnoseMissingReturn(const UnreachableInst *UI,
5958
FLoc.isASTNode<ClosureExpr>() ? 1 : 0);
6059
}
6160

62-
63-
/// If the pattern handles an enum element, remove the enum element from the
64-
/// given set. If seeing uncertain patterns, e.g. any pattern, return true;
65-
/// otherwise return false.
66-
static bool
67-
removedHandledEnumElements(Pattern *P,
68-
llvm::DenseSet<EnumElementDecl*> &UnhandledElements) {
69-
if (auto *EEP = dyn_cast<EnumElementPattern>(P)) {
70-
UnhandledElements.erase(EEP->getElementDecl());
71-
} else if (auto *VP = dyn_cast<VarPattern>(P)) {
72-
return removedHandledEnumElements(VP->getSubPattern(), UnhandledElements);
73-
} else {
74-
return true;
75-
}
76-
return false;
77-
};
78-
79-
static void diagnoseMissingCases(ASTContext &Context,
80-
const SwitchStmt *SwitchS) {
81-
SourceLoc EndLoc = SwitchS->getEndLoc();
82-
StringRef Placeholder = "<#Code#>";
83-
SmallString<32> Buffer;
84-
llvm::raw_svector_ostream OS(Buffer);
85-
86-
auto DefaultDiag = [&]() {
87-
OS << tok::kw_default << ": " << Placeholder << "\n";
88-
Context.Diags.diagnose(EndLoc, diag::non_exhaustive_switch).
89-
fixItInsert(EndLoc, Buffer.str());
90-
};
91-
// To find the subject enum decl for this switch statement.
92-
EnumDecl *SubjectED = nullptr;
93-
if (auto SubjectTy = SwitchS->getSubjectExpr()->getType()) {
94-
if (auto *ND = SubjectTy->getAnyNominal()) {
95-
SubjectED = ND->getAsEnumOrEnumExtensionContext();
96-
}
97-
}
98-
99-
// The switch is not about an enum decl, add "default:" instead.
100-
if (!SubjectED) {
101-
DefaultDiag();
102-
return;
103-
}
104-
105-
// Assume enum elements are not handled in the switch statement.
106-
llvm::DenseSet<EnumElementDecl*> UnhandledElements;
107-
108-
SubjectED->getAllElements(UnhandledElements);
109-
for (auto Current : SwitchS->getCases()) {
110-
// For each handled enum element, remove it from the bucket.
111-
for (auto Item : Current->getCaseLabelItems()) {
112-
if (removedHandledEnumElements(Item.getPattern(), UnhandledElements)) {
113-
DefaultDiag();
114-
return;
115-
}
116-
}
117-
}
118-
119-
// No unhandled cases, so we are not sure the exact case to add, fall back to
120-
// default instead.
121-
if (UnhandledElements.empty()) {
122-
DefaultDiag();
123-
return;
124-
}
125-
126-
// Sort the missing elements to a vector because set does not guarantee orders.
127-
SmallVector<EnumElementDecl*, 4> SortedElements;
128-
SortedElements.insert(SortedElements.begin(), UnhandledElements.begin(),
129-
UnhandledElements.end());
130-
std::sort(SortedElements.begin(),SortedElements.end(),
131-
[](EnumElementDecl *LHS, EnumElementDecl *RHS) {
132-
return LHS->getNameStr().compare(RHS->getNameStr()) < 0;
133-
});
134-
135-
// Print each enum element name.
136-
std::for_each(SortedElements.begin(), SortedElements.end(),
137-
[&](EnumElementDecl *EE) {
138-
OS << tok::kw_case << " ." << EE->getNameStr() << ": " <<
139-
Placeholder << "\n";
140-
});
141-
Context.Diags.diagnose(EndLoc, diag::non_exhaustive_switch).
142-
fixItInsert(EndLoc, Buffer.str());
143-
}
144-
14561
static void diagnoseUnreachable(const SILInstruction *I,
14662
ASTContext &Context) {
14763
if (auto *UI = dyn_cast<UnreachableInst>(I)) {
@@ -168,7 +84,8 @@ static void diagnoseUnreachable(const SILInstruction *I,
16884

16985
// A non-exhaustive switch would also produce an unreachable instruction.
17086
if (L.isASTNode<SwitchStmt>()) {
171-
diagnoseMissingCases(Context, L.getAsASTNode<SwitchStmt>());
87+
diagnoseMissingCases(Context, L.getAsASTNode<SwitchStmt>(),
88+
diag::non_exhaustive_switch);
17289
return;
17390
}
17491

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
#include "TypeChecker.h"
1818
#include "MiscDiagnostics.h"
1919
#include "swift/Subsystems.h"
20+
#include "swift/AST/ASTPrinter.h"
2021
#include "swift/AST/ASTWalker.h"
2122
#include "swift/AST/ASTVisitor.h"
23+
#include "swift/AST/DiagnosticsSema.h"
2224
#include "swift/AST/Identifier.h"
2325
#include "swift/AST/Initializer.h"
2426
#include "swift/AST/NameLookup.h"
@@ -29,6 +31,7 @@
2931
#include "swift/Basic/Statistic.h"
3032
#include "swift/Parse/Lexer.h"
3133
#include "swift/Parse/LocalContext.h"
34+
#include "swift/Syntax/TokenKinds.h"
3235
#include "llvm/ADT/DenseMap.h"
3336
#include "llvm/ADT/PointerUnion.h"
3437
#include "llvm/ADT/SmallString.h"
@@ -193,6 +196,86 @@ namespace {
193196
};
194197
} // end anonymous namespace
195198

199+
/// If the pattern handles an enum element, remove the enum element from the
200+
/// given set. If seeing uncertain patterns, e.g. any pattern, return true;
201+
/// otherwise return false.
202+
static bool
203+
removedHandledEnumElements(Pattern *P,
204+
llvm::DenseSet<EnumElementDecl*> &UnhandledElements) {
205+
if (auto *EEP = dyn_cast<EnumElementPattern>(P)) {
206+
UnhandledElements.erase(EEP->getElementDecl());
207+
} else if (auto *VP = dyn_cast<VarPattern>(P)) {
208+
return removedHandledEnumElements(VP->getSubPattern(), UnhandledElements);
209+
} else {
210+
return true;
211+
}
212+
return false;
213+
};
214+
215+
void swift::diagnoseMissingCases(ASTContext &Context, const SwitchStmt *SwitchS,
216+
Diagnostic Id) {
217+
SourceLoc EndLoc = SwitchS->getEndLoc();
218+
StringRef Placeholder = "<#Code#>";
219+
SmallString<32> Buffer;
220+
llvm::raw_svector_ostream OS(Buffer);
221+
222+
auto DefaultDiag = [&]() {
223+
OS << tok::kw_default << ": " << Placeholder << "\n";
224+
Context.Diags.diagnose(EndLoc, Id).fixItInsert(EndLoc, Buffer.str());
225+
};
226+
// To find the subject enum decl for this switch statement.
227+
EnumDecl *SubjectED = nullptr;
228+
if (auto SubjectTy = SwitchS->getSubjectExpr()->getType()) {
229+
if (auto *ND = SubjectTy->getAnyNominal()) {
230+
SubjectED = ND->getAsEnumOrEnumExtensionContext();
231+
}
232+
}
233+
234+
// The switch is not about an enum decl, add "default:" instead.
235+
if (!SubjectED) {
236+
DefaultDiag();
237+
return;
238+
}
239+
240+
// Assume enum elements are not handled in the switch statement.
241+
llvm::DenseSet<EnumElementDecl*> UnhandledElements;
242+
243+
SubjectED->getAllElements(UnhandledElements);
244+
for (auto Current : SwitchS->getCases()) {
245+
// For each handled enum element, remove it from the bucket.
246+
for (auto Item : Current->getCaseLabelItems()) {
247+
if (removedHandledEnumElements(Item.getPattern(), UnhandledElements)) {
248+
DefaultDiag();
249+
return;
250+
}
251+
}
252+
}
253+
254+
// No unhandled cases, so we are not sure the exact case to add, fall back to
255+
// default instead.
256+
if (UnhandledElements.empty()) {
257+
DefaultDiag();
258+
return;
259+
}
260+
261+
// Sort the missing elements to a vector because set does not guarantee orders.
262+
SmallVector<EnumElementDecl*, 4> SortedElements;
263+
SortedElements.insert(SortedElements.begin(), UnhandledElements.begin(),
264+
UnhandledElements.end());
265+
std::sort(SortedElements.begin(),SortedElements.end(),
266+
[](EnumElementDecl *LHS, EnumElementDecl *RHS) {
267+
return LHS->getNameStr().compare(RHS->getNameStr()) < 0;
268+
});
269+
270+
// Print each enum element name.
271+
std::for_each(SortedElements.begin(), SortedElements.end(),
272+
[&](EnumElementDecl *EE) {
273+
OS << tok::kw_case << " ." << EE->getNameStr() << ": " <<
274+
Placeholder << "\n";
275+
});
276+
Context.Diags.diagnose(EndLoc, Id).fixItInsert(EndLoc, Buffer.str());
277+
}
278+
196279
static void setAutoClosureDiscriminators(DeclContext *DC, Stmt *S) {
197280
S->walk(ContextualizeClosures(DC));
198281
}
@@ -862,6 +945,9 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
862945
AddSwitchNest switchNest(*this);
863946
AddLabeledStmt labelNest(*this, S);
864947

948+
// Reject switch statements with empty blocks.
949+
if (S->getCases().empty())
950+
diagnoseMissingCases(TC.Context, S, diag::empty_switch_stmt);
865951
for (unsigned i = 0, e = S->getCases().size(); i < e; ++i) {
866952
auto *caseBlock = S->getCases()[i];
867953
// Fallthrough transfers control to the next case block. In the

‎test/FixCode/fixits-empty-switch.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: not %swift -typecheck -target %target-triple %s -emit-fixits-path %t.remap -I %S/Inputs
2+
// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
3+
4+
enum E1 {
5+
case e1
6+
case e2
7+
case e3
8+
}
9+
10+
func foo1(_ e: E1) {
11+
switch e {
12+
}
13+
}
14+
15+
func foo1 (_ i : Int) {
16+
switch i {
17+
}
18+
}

‎test/FixCode/fixits-empty-switch.swift.result

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: not %swift -typecheck -target %target-triple %s -emit-fixits-path %t.remap -I %S/Inputs
2+
// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
3+
4+
enum E1 {
5+
case e1
6+
case e2
7+
case e3
8+
}
9+
10+
func foo1(_ e: E1) {
11+
switch e {
12+
case .e1: <#Code#>
13+
case .e2: <#Code#>
14+
case .e3: <#Code#>
15+
}
16+
}
17+
18+
func foo1 (_ i : Int) {
19+
switch i {
20+
default: <#Code#>
21+
}
22+
}

‎test/Parse/invalid.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func testNotCoveredCase(x: Int) {
6060
default:
6161
break
6262
#endif
63-
}
63+
} // expected-error{{'switch' statement body must have at least one 'case' or 'default' block}}
6464
}
6565

6666
// rdar://18926814

‎test/Parse/recovery.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,11 +219,11 @@ func missingControllingExprInSwitch() {
219219
case _: return
220220
}
221221

222-
switch { 42 } { // expected-error {{expected expression in 'switch' statement}} expected-error{{all statements inside a switch must be covered by a 'case' or 'default'}} expected-error{{consecutive statements on a line must be separated by ';'}} {{16-16=;}} expected-error{{closure expression is unused}} expected-note{{did you mean to use a 'do' statement?}} {{17-17=do }}
222+
switch { 42 } { // expected-error {{expected expression in 'switch' statement}} expected-error{{all statements inside a switch must be covered by a 'case' or 'default'}} expected-error{{consecutive statements on a line must be separated by ';'}} {{16-16=;}} expected-error{{closure expression is unused}} expected-note{{did you mean to use a 'do' statement?}} {{17-17=do }} // expected-error{{'switch' statement body must have at least one 'case' or 'default' block}}
223223
case _: return // expected-error{{'case' label can only appear inside a 'switch' statement}}
224224
}
225225

226-
switch { 42 }() { // expected-error {{expected expression in 'switch' statement}} expected-error {{all statements inside a switch must be covered by a 'case' or 'default'}} expected-error 2 {{consecutive statements on a line must be separated by ';'}} expected-error{{closure expression is unused}} expected-note{{did you mean to use a 'do' statement?}} {{19-19=do }}
226+
switch { 42 }() { // expected-error {{expected expression in 'switch' statement}} expected-error {{all statements inside a switch must be covered by a 'case' or 'default'}} expected-error 2 {{consecutive statements on a line must be separated by ';'}} expected-error{{closure expression is unused}} expected-note{{did you mean to use a 'do' statement?}} {{19-19=do }} // expected-error{{'switch' statement body must have at least one 'case' or 'default' block}}
227227
case _: return // expected-error{{'case' label can only appear inside a 'switch' statement}}
228228
}
229229
}

‎test/Parse/switch.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,12 @@ default: // expected-error{{additional 'case' blocks cannot appear after the 'de
123123

124124
switch x {
125125
x = 1 // expected-error{{all statements inside a switch must be covered by a 'case' or 'default'}}
126-
}
126+
} // expected-error{{'switch' statement body must have at least one 'case' or 'default' block}}
127127

128128
switch x {
129129
x = 1 // expected-error{{all statements inside a switch must be covered by a 'case' or 'default'}}
130130
x = 2
131-
}
131+
} // expected-error{{'switch' statement body must have at least one 'case' or 'default' block}}
132132

133133
switch x {
134134
default: // expected-error{{'default' label in a 'switch' should have at least one executable statement}} {{9-9= break}}

0 commit comments

Comments
 (0)