Skip to content

Commit 282cbc3

Browse files
ahoppenahoppen
authored
Merge pull request #36930 from ahoppen/pr/complete-switch-expr-in-closure
[Parse] Create SwitchStmt nodes for `switch` statements with errors
2 parents 341b870 + 931f339 commit 282cbc3

13 files changed

+77
-29
lines changed

‎include/swift/AST/Stmt.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,15 +1110,19 @@ class SwitchStmt final : public LabeledStmt,
11101110
friend TrailingObjects;
11111111

11121112
SourceLoc SwitchLoc, LBraceLoc, RBraceLoc;
1113+
/// The location of the last token in the 'switch' statement. For valid
1114+
/// 'switch' statements this is the same as \c RBraceLoc. If the '}' is
1115+
/// missing this points to the last token before the '}' was expected.
1116+
SourceLoc EndLoc;
11131117
Expr *SubjectExpr;
11141118

11151119
SwitchStmt(LabeledStmtInfo LabelInfo, SourceLoc SwitchLoc, Expr *SubjectExpr,
11161120
SourceLoc LBraceLoc, unsigned CaseCount, SourceLoc RBraceLoc,
1117-
Optional<bool> implicit = None)
1121+
SourceLoc EndLoc, Optional<bool> implicit = None)
11181122
: LabeledStmt(StmtKind::Switch, getDefaultImplicitFlag(implicit, SwitchLoc),
11191123
LabelInfo),
11201124
SwitchLoc(SwitchLoc), LBraceLoc(LBraceLoc), RBraceLoc(RBraceLoc),
1121-
SubjectExpr(SubjectExpr) {
1125+
EndLoc(EndLoc), SubjectExpr(SubjectExpr) {
11221126
Bits.SwitchStmt.CaseCount = CaseCount;
11231127
}
11241128

@@ -1129,6 +1133,7 @@ class SwitchStmt final : public LabeledStmt,
11291133
SourceLoc LBraceLoc,
11301134
ArrayRef<ASTNode> Cases,
11311135
SourceLoc RBraceLoc,
1136+
SourceLoc EndLoc,
11321137
ASTContext &C);
11331138

11341139
/// Get the source location of the 'switch' keyword.
@@ -1141,8 +1146,8 @@ class SwitchStmt final : public LabeledStmt,
11411146
SourceLoc getLoc() const { return SwitchLoc; }
11421147

11431148
SourceLoc getStartLoc() const { return getLabelLocOrKeywordLoc(SwitchLoc); }
1144-
SourceLoc getEndLoc() const { return RBraceLoc; }
1145-
1149+
SourceLoc getEndLoc() const { return EndLoc; }
1150+
11461151
/// Get the subject expression of the switch.
11471152
Expr *getSubjectExpr() const { return SubjectExpr; }
11481153
void setSubjectExpr(Expr *e) { SubjectExpr = e; }

‎lib/AST/Stmt.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ SwitchStmt *SwitchStmt::create(LabeledStmtInfo LabelInfo, SourceLoc SwitchLoc,
482482
SourceLoc LBraceLoc,
483483
ArrayRef<ASTNode> Cases,
484484
SourceLoc RBraceLoc,
485+
SourceLoc EndLoc,
485486
ASTContext &C) {
486487
#ifndef NDEBUG
487488
for (auto N : Cases)
@@ -494,7 +495,8 @@ SwitchStmt *SwitchStmt::create(LabeledStmtInfo LabelInfo, SourceLoc SwitchLoc,
494495
alignof(SwitchStmt));
495496
SwitchStmt *theSwitch = ::new (p) SwitchStmt(LabelInfo, SwitchLoc,
496497
SubjectExpr, LBraceLoc,
497-
Cases.size(), RBraceLoc);
498+
Cases.size(), RBraceLoc,
499+
EndLoc);
498500

499501
std::uninitialized_copy(Cases.begin(), Cases.end(),
500502
theSwitch->getTrailingObjects<ASTNode>());

‎lib/Parse/ParseStmt.cpp

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2249,23 +2249,31 @@ ParserResult<Stmt> Parser::parseStmtSwitch(LabeledStmtInfo LabelInfo) {
22492249
SubjectExpr = makeParserErrorResult(new (Context) ErrorExpr(SubjectLoc));
22502250
} else {
22512251
SubjectExpr = parseExprBasic(diag::expected_switch_expr);
2252-
if (SubjectExpr.hasCodeCompletion()) {
2253-
return makeParserCodeCompletionResult<Stmt>();
2254-
}
22552252
if (SubjectExpr.isNull()) {
22562253
SubjectExpr = makeParserErrorResult(new (Context) ErrorExpr(SubjectLoc));
22572254
}
22582255
Status |= SubjectExpr;
22592256
}
22602257

2261-
if (!Tok.is(tok::l_brace)) {
2262-
diagnose(Tok, diag::expected_lbrace_after_switch);
2263-
return nullptr;
2264-
}
2265-
SourceLoc lBraceLoc = consumeToken(tok::l_brace);
2258+
SourceLoc lBraceLoc;
22662259
SourceLoc rBraceLoc;
2267-
22682260
SmallVector<ASTNode, 8> cases;
2261+
2262+
if (Status.isErrorOrHasCompletion()) {
2263+
return makeParserResult(
2264+
Status, SwitchStmt::create(LabelInfo, SwitchLoc, SubjectExpr.get(),
2265+
lBraceLoc, cases, rBraceLoc,
2266+
/*EndLoc=*/PreviousLoc, Context));
2267+
}
2268+
2269+
if (!consumeIf(tok::l_brace, lBraceLoc)) {
2270+
diagnose(Tok, diag::expected_lbrace_after_switch);
2271+
return makeParserResult(
2272+
Status, SwitchStmt::create(LabelInfo, SwitchLoc, SubjectExpr.get(),
2273+
lBraceLoc, cases, rBraceLoc,
2274+
/*EndLoc=*/PreviousLoc, Context));
2275+
}
2276+
22692277
Status |= parseStmtCases(cases, /*IsActive=*/true);
22702278

22712279
// We cannot have additional cases after a default clause. Complain on
@@ -2288,7 +2296,8 @@ ParserResult<Stmt> Parser::parseStmtSwitch(LabeledStmtInfo LabelInfo) {
22882296

22892297
return makeParserResult(
22902298
Status, SwitchStmt::create(LabelInfo, SwitchLoc, SubjectExpr.get(),
2291-
lBraceLoc, cases, rBraceLoc, Context));
2299+
lBraceLoc, cases, rBraceLoc,
2300+
/*EndLoc=*/rBraceLoc, Context));
22922301
}
22932302

22942303
ParserStatus

‎lib/Sema/DerivedConformanceCodable.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,7 +1164,8 @@ deriveBodyEncodable_enum_encode(AbstractFunctionDecl *encodeDecl, void *) {
11641164
/*implicit*/ true, AccessSemantics::Ordinary);
11651165

11661166
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), enumRef,
1167-
SourceLoc(), cases, SourceLoc(), C);
1167+
SourceLoc(), cases, SourceLoc(),
1168+
SourceLoc(), C);
11681169
statements.push_back(switchStmt);
11691170

11701171
auto *body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc(),
@@ -1801,7 +1802,7 @@ deriveBodyDecodable_enum_init(AbstractFunctionDecl *initDecl, void *) {
18011802

18021803
auto switchStmt =
18031804
SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), firstExpr,
1804-
SourceLoc(), cases, SourceLoc(), C);
1805+
SourceLoc(), cases, SourceLoc(), SourceLoc(), C);
18051806

18061807
statements.push_back(switchStmt);
18071808
}
@@ -2093,4 +2094,4 @@ ValueDecl *DerivedConformance::deriveDecodable(ValueDecl *requirement) {
20932094
}
20942095

20952096
return deriveDecodable_init(*this);
2096-
}
2097+
}

‎lib/Sema/DerivedConformanceCodingKey.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ deriveBodyCodingKey_enum_stringValue(AbstractFunctionDecl *strValDecl, void *) {
235235
auto *selfRef = DerivedConformance::createSelfDeclRef(strValDecl);
236236
auto *switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(),
237237
selfRef, SourceLoc(), cases,
238-
SourceLoc(), C);
238+
SourceLoc(), SourceLoc(), C);
239239
body = BraceStmt::create(C, SourceLoc(), ASTNode(switchStmt), SourceLoc());
240240
}
241241

@@ -315,7 +315,7 @@ deriveBodyCodingKey_init_stringValue(AbstractFunctionDecl *initDecl, void *) {
315315
/*Implicit=*/true);
316316
auto *switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(),
317317
stringValueRef, SourceLoc(), cases,
318-
SourceLoc(), C);
318+
SourceLoc(), SourceLoc(), C);
319319
auto *body = BraceStmt::create(C, SourceLoc(), ASTNode(switchStmt),
320320
SourceLoc());
321321
return { body, /*isTypeChecked=*/false };

‎lib/Sema/DerivedConformanceComparable.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,8 @@ deriveBodyComparable_enum_hasAssociatedValues_lt(AbstractFunctionDecl *ltDecl, v
211211
SourceLoc(), /*HasTrailingClosure*/ false,
212212
/*implicit*/ true);
213213
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), abExpr,
214-
SourceLoc(), cases, SourceLoc(), C);
214+
SourceLoc(), cases, SourceLoc(),
215+
SourceLoc(), C);
215216
statements.push_back(switchStmt);
216217

217218
auto body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc());

‎lib/Sema/DerivedConformanceEquatableHashable.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ deriveBodyEquatable_enum_uninhabited_eq(AbstractFunctionDecl *eqDecl, void *) {
8080
/*implicit*/ true,
8181
TupleType::get(abTupleElts, C));
8282
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), abExpr,
83-
SourceLoc(), cases, SourceLoc(), C);
83+
SourceLoc(), cases, SourceLoc(),
84+
SourceLoc(), C);
8485
statements.push_back(switchStmt);
8586

8687
auto body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc());
@@ -277,7 +278,8 @@ deriveBodyEquatable_enum_hasAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
277278
SourceLoc(), /*HasTrailingClosure*/ false,
278279
/*implicit*/ true);
279280
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), abExpr,
280-
SourceLoc(), cases, SourceLoc(), C);
281+
SourceLoc(), cases, SourceLoc(),
282+
SourceLoc(), C);
281283
statements.push_back(switchStmt);
282284

283285
auto body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc());
@@ -751,7 +753,8 @@ deriveBodyHashable_enum_hasAssociatedValues_hashInto(
751753
auto enumRef = new (C) DeclRefExpr(selfDecl, DeclNameLoc(),
752754
/*implicit*/true);
753755
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), enumRef,
754-
SourceLoc(), cases, SourceLoc(), C);
756+
SourceLoc(), cases, SourceLoc(),
757+
SourceLoc(), C);
755758

756759
auto body = BraceStmt::create(C, SourceLoc(), {ASTNode(switchStmt)},
757760
SourceLoc());

‎lib/Sema/DerivedConformanceRawRepresentable.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ deriveBodyRawRepresentable_raw(AbstractFunctionDecl *toRawDecl, void *) {
129129

130130
auto selfRef = DerivedConformance::createSelfDeclRef(toRawDecl);
131131
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), selfRef,
132-
SourceLoc(), cases, SourceLoc(), C);
132+
SourceLoc(), cases, SourceLoc(),
133+
SourceLoc(), C);
133134
auto body = BraceStmt::create(C, SourceLoc(),
134135
ASTNode(switchStmt),
135136
SourceLoc());
@@ -386,7 +387,8 @@ deriveBodyRawRepresentable_init(AbstractFunctionDecl *initDecl, void *) {
386387
switchArg = CallExpr;
387388
}
388389
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), switchArg,
389-
SourceLoc(), cases, SourceLoc(), C);
390+
SourceLoc(), cases, SourceLoc(),
391+
SourceLoc(), C);
390392
auto body = BraceStmt::create(C, SourceLoc(),
391393
ASTNode(switchStmt),
392394
SourceLoc());

‎lib/Sema/DerivedConformances.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,8 @@ DeclRefExpr *DerivedConformance::convertEnumToIndex(SmallVectorImpl<ASTNode> &st
666666
AccessSemantics::Ordinary,
667667
enumVarDecl->getType());
668668
auto switchStmt = SwitchStmt::create(LabeledStmtInfo(), SourceLoc(), enumRef,
669-
SourceLoc(), cases, SourceLoc(), C);
669+
SourceLoc(), cases, SourceLoc(),
670+
SourceLoc(), C);
670671

671672
stmts.push_back(indexBind);
672673
stmts.push_back(switchStmt);

‎lib/Sema/TypeCheckSwitchStmt.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,6 +1058,12 @@ namespace {
10581058

10591059
void diagnoseMissingCases(RequiresDefault defaultReason, Space uncovered,
10601060
const CaseStmt *unknownCase = nullptr) {
1061+
if (!Switch->getLBraceLoc().isValid()) {
1062+
// There is no '{' in the switch statement, which we already diagnosed
1063+
// in the parser. So there's no real body to speak of and it doesn't
1064+
// make sense to emit diagnostics about missing cases.
1065+
return;
1066+
}
10611067
auto &DE = Context.Diags;
10621068
SourceLoc startLoc = Switch->getStartLoc();
10631069
SourceLoc insertLoc;

‎test/IDE/complete_sr14454.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t
2+
3+
struct Foo {
4+
let bar: Bool
5+
}
6+
7+
func foo(_: (Foo) -> Void) {}
8+
9+
foo { x in
10+
switch x.#^COMPLETE_WITHOUT_BRACE?check=CHECK^#
11+
}
12+
foo { x in
13+
switch x.#^COMPLETE_WITH_BRACES?check=CHECK^# {}
14+
}
15+
16+
// CHECK: Begin completions
17+
// CHECK: Decl[InstanceVar]/CurrNominal: bar[#Bool#];
18+
// CHECK: End completions

‎test/Parse/recovery.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ func missingControllingExprInForEach() {
234234
}
235235

236236
func missingControllingExprInSwitch() {
237-
switch // expected-error {{expected expression in 'switch' statement}} expected-error {{expected '{' after 'switch' subject expression}}
237+
switch // expected-error {{expected expression in 'switch' statement}}
238238

239239
switch { // expected-error {{expected expression in 'switch' statement}} expected-error {{'switch' statement body must have at least one 'case' or 'default' block}}
240240
}

‎test/Parse/switch.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ func ~= (x: (Int,Int), y: (Int,Int)) -> Bool {
77
}
88

99
func parseError1(x: Int) {
10-
switch func {} // expected-error {{expected expression in 'switch' statement}} expected-error {{expected '{' after 'switch' subject expression}} expected-error {{expected identifier in function declaration}} expected-error {{closure expression is unused}} expected-note{{did you mean to use a 'do' statement?}} {{15-15=do }}
10+
switch func {} // expected-error {{expected expression in 'switch' statement}} expected-error {{expected identifier in function declaration}} expected-error {{closure expression is unused}} expected-note{{did you mean to use a 'do' statement?}} {{15-15=do }}
1111
}
1212

1313
func parseError2(x: Int) {

0 commit comments

Comments
 (0)