Skip to content

Commit f442321

Browse files
LucianoPAlmeidarintaro
authored andcommitted
SR-11261: [Diagnostics][Qol] Parser recovery for keyword (#26596)
* Fixes SR-11261 * Improving diagnostics for multi-case declaration that uses a keyword * Only doing pattern matching if token is not a keywork or if is a possible identifier * Using BacktrackingScope to fix expected pattern * Updating NameLoc with the token consumed. * Updating Name with the consumed token
1 parent ae8ab3f commit f442321

File tree

4 files changed

+69
-22
lines changed

4 files changed

+69
-22
lines changed

include/swift/AST/DiagnosticsParse.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ ERROR(expected_keyword_in_decl,none,
221221
ERROR(number_cant_start_decl_name,none,
222222
"%0 name can only start with a letter or underscore, not a number",
223223
(StringRef))
224-
ERROR(expected_identifier_after_case_comma,none,
224+
ERROR(expected_identifier_after_case_comma, PointsToFirstBadToken,
225225
"expected identifier after comma in enum 'case' declaration", ())
226226
ERROR(decl_redefinition,none,
227227
"definition conflicts with previous value", ())

lib/Parse/ParseDecl.cpp

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5803,24 +5803,32 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags,
58035803

58045804
// For recovery, see if the user typed something resembling a switch
58055805
// "case" label.
5806-
llvm::SaveAndRestore<decltype(InVarOrLetPattern)>
5807-
T(InVarOrLetPattern, Parser::IVOLP_InMatchingPattern);
5808-
parseMatchingPattern(/*isExprBasic*/false);
5809-
5810-
if (consumeIf(tok::colon)) {
5811-
diagnose(CaseLoc, diag::case_outside_of_switch, "case");
5812-
Status.setIsParseError();
5813-
return Status;
5814-
}
5815-
if (CommaLoc.isValid()) {
5816-
diagnose(Tok, diag::expected_identifier_after_case_comma);
5817-
Status.setIsParseError();
5818-
return Status;
5806+
{
5807+
BacktrackingScope backtrack(*this);
5808+
llvm::SaveAndRestore<decltype(InVarOrLetPattern)>
5809+
T(InVarOrLetPattern, Parser::IVOLP_InMatchingPattern);
5810+
parseMatchingPattern(/*isExprBasic*/false);
5811+
5812+
if (consumeIf(tok::colon)) {
5813+
backtrack.cancelBacktrack();
5814+
diagnose(CaseLoc, diag::case_outside_of_switch, "case");
5815+
Status.setIsParseError();
5816+
return Status;
5817+
}
58195818
}
5819+
58205820
if (NameIsKeyword) {
58215821
diagnose(TokLoc, diag::keyword_cant_be_identifier, TokText);
58225822
diagnose(TokLoc, diag::backticks_to_escape)
58235823
.fixItReplace(TokLoc, "`" + TokText.str() + "`");
5824+
if (!Tok.isAtStartOfLine()) {
5825+
Name = Context.getIdentifier(Tok.getText());
5826+
NameLoc = consumeToken();
5827+
}
5828+
} else if (CommaLoc.isValid()) {
5829+
diagnose(Tok, diag::expected_identifier_after_case_comma);
5830+
Status.setIsParseError();
5831+
return Status;
58245832
} else {
58255833
diagnose(CaseLoc, diag::expected_identifier_in_decl, "enum 'case'");
58265834
}

test/Parse/enum.swift

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ enum ImproperlyHasIVars {
101101

102102
// We used to crash on this. rdar://14678675
103103
enum rdar14678675 {
104-
case U1,
105-
case U2 // expected-error{{expected identifier after comma in enum 'case' declaration}}
104+
case U1, // expected-error{{expected identifier after comma in enum 'case' declaration}}
105+
case U2
106106
case U3
107107
}
108108

@@ -125,10 +125,10 @@ enum Recovery5 {
125125
// expected-error@-2{{extraneous '.' in enum 'case' declaration}} {{14-15=}}
126126
}
127127
enum Recovery6 {
128-
case Snout, _; // expected-error {{expected identifier after comma in enum 'case' declaration}}
129-
case _; // expected-error {{keyword '_' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{8-9=`_`}}
130-
case Tusk, // expected-error {{expected pattern}}
131-
} // expected-error {{expected identifier after comma in enum 'case' declaration}}
128+
case Snout, _; // expected-error {{keyword '_' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{15-16=`_`}} expected-note {{'_' previously declared here}}
129+
case _; // expected-error {{keyword '_' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{8-9=`_`}} expected-error {{invalid redeclaration of '_'}}
130+
case Tusk, // expected-error {{expected identifier after comma in enum 'case' declaration}}
131+
}
132132

133133
enum RawTypeEmpty : Int {} // expected-error {{an enum with no cases cannot declare a raw type}} expected-note {{do you want to add protocol stubs?}}
134134
// expected-error@-1{{'RawTypeEmpty' declares raw type 'Int', but does not conform to RawRepresentable and conformance could not be synthesized}}
@@ -552,3 +552,42 @@ enum SE0155 {
552552
// expected-note@-1 {{did you mean to remove the empty associated value list?}} {{17-18=}}
553553
// expected-note@-2 {{did you mean to explicitly add a 'Void' associated value?}} {{17-17=Void}}
554554
}
555+
556+
// SR-11261
557+
enum SR11261 {
558+
case identifier
559+
case operator // expected-error {{keyword 'operator' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{8-16=`operator`}}
560+
case identifier2
561+
}
562+
563+
enum SR11261_var {
564+
case identifier
565+
case var // expected-error {{keyword 'var' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{8-11=`var`}}
566+
case identifier2
567+
}
568+
569+
enum SR11261_underscore {
570+
case identifier
571+
case _ // expected-error {{keyword '_' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{8-9=`_`}}
572+
case identifier2
573+
}
574+
575+
enum SR11261_Comma {
576+
case a, b, c, func, d // expected-error {{keyword 'func' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{17-21=`func`}}
577+
}
578+
579+
enum SR11261_Newline {
580+
case identifier1
581+
case identifier2
582+
case
583+
case identifier // expected-error {{keyword 'case' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{3-7=`case`}}
584+
}
585+
586+
enum SR11261_Newline2 {
587+
case
588+
func foo() {} // expected-error {{keyword 'func' cannot be used as an identifier here}} expected-note {{if this name is unavoidable, use backticks to escape it}} {{3-7=`func`}}
589+
}
590+
591+
enum SR11261_PatternMatching {
592+
case let .foo(x, y): // expected-error {{'case' label can only appear inside a 'switch' statement}}
593+
}

test/stmt/statements.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,6 @@ outerLoop1: repeat { // expected-note {{did you mean 'outerLoop1'?}} {{14-23=out
637637

638638
// Errors in case syntax
639639
class
640-
case, // expected-error {{expected identifier in enum 'case' declaration}} expected-error {{expected pattern}}
641-
case // expected-error {{expected identifier after comma in enum 'case' declaration}} expected-error {{expected identifier in enum 'case' declaration}} expected-error {{enum 'case' is not allowed outside of an enum}} expected-error {{expected pattern}}
640+
case, // expected-error {{expected identifier in enum 'case' declaration}} expected-error {{expected identifier after comma in enum 'case' declaration}}
641+
case // expected-error {{expected identifier in enum 'case' declaration}} expected-error {{enum 'case' is not allowed outside of an enum}}
642642
// NOTE: EOF is important here to properly test a code path that used to crash the parser

0 commit comments

Comments
 (0)