Skip to content

[DiagnosticsQoI] SR-3600: Better recovery for trying to name an initializer, deinitializer, or subscript #6863

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
Jan 24, 2017
Merged

[DiagnosticsQoI] SR-3600: Better recovery for trying to name an initializer, deinitializer, or subscript #6863

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
7882ddc
[DiagnosticsQoI] SR-3600: Better recovery for trying to name an initi…
matthewcarroll Jan 17, 2017
f8f147e
[DiagnosticsQoI] SR-3600: Better recovery for trying to name an initi…
matthewcarroll Jan 21, 2017
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
7 changes: 7 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,8 @@ ERROR(subscript_decl_wrong_scope,none,
"'subscript' functions may only be declared within a type", ())
ERROR(expected_lparen_subscript,PointsToFirstBadToken,
"expected '(' for subscript parameters", ())
ERROR(subscript_has_name,PointsToFirstBadToken,
"subscripts cannot have a name", ())
ERROR(expected_arrow_subscript,PointsToFirstBadToken,
"expected '->' for subscript element type", ())
ERROR(expected_type_subscript,PointsToFirstBadToken,
Expand All @@ -360,12 +362,17 @@ ERROR(initializer_decl_wrong_scope,none,
"initializers may only be declared within a type", ())
ERROR(expected_lparen_initializer,PointsToFirstBadToken,
"expected '(' for initializer parameters", ())
ERROR(initializer_has_name,PointsToFirstBadToken,
"initializers cannot have a name", ())

// Destructor
ERROR(destructor_decl_outside_class,none,
"deinitializers may only be declared within a class", ())
ERROR(expected_lbrace_destructor,PointsToFirstBadToken,
"expected '{' for deinitializer", ())
ERROR(destructor_has_name,PointsToFirstBadToken,
"deinitializers cannot have a name", ())

ERROR(opened_destructor_expected_rparen,none,
"expected ')' to close parameter list", ())
ERROR(destructor_params,none,
Expand Down
5 changes: 4 additions & 1 deletion lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5515,7 +5515,10 @@ parseDeclDeinit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
// '{'
if (!Tok.is(tok::l_brace)) {
if (!Tok.is(tok::l_brace) && !isInSILMode()) {
diagnose(Tok, diag::expected_lbrace_destructor);
if (Tok.is(tok::identifier)) {
diagnose(Tok, diag::destructor_has_name).fixItRemove(Tok.getLoc());
} else
diagnose(Tok, diag::expected_lbrace_destructor);
return nullptr;
}
}
Expand Down
17 changes: 15 additions & 2 deletions lib/Parse/ParsePattern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,16 +476,23 @@ Parser::parseSingleParameterClause(ParameterContextKind paramContext,
if (!Tok.is(tok::l_paren)) {
// If we don't have the leading '(', complain.
Diag<> diagID;
bool skipIdentifier = false;
switch (paramContext) {
case ParameterContextKind::Function:
case ParameterContextKind::Operator:
diagID = diag::func_decl_without_paren;
break;
case ParameterContextKind::Subscript:
diagID = diag::expected_lparen_subscript;
skipIdentifier = Tok.is(tok::identifier) &&
peekToken().is(tok::l_paren);
diagID = skipIdentifier ? diag::subscript_has_name
: diag::expected_lparen_subscript;
break;
case ParameterContextKind::Initializer:
diagID = diag::expected_lparen_initializer;
skipIdentifier = Tok.is(tok::identifier) &&
peekToken().is(tok::l_paren);
diagID = skipIdentifier ? diag::initializer_has_name
: diag::expected_lparen_initializer;
break;
case ParameterContextKind::Closure:
case ParameterContextKind::Curried:
Expand All @@ -496,6 +503,12 @@ Parser::parseSingleParameterClause(ParameterContextKind paramContext,
if (Tok.isAny(tok::l_brace, tok::arrow, tok::kw_throws, tok::kw_rethrows))
diag.fixItInsertAfter(PreviousLoc, "()");

if (skipIdentifier) {
diag.fixItRemove(Tok.getLoc());
consumeToken();
skipSingle();
}

// Create an empty parameter list to recover.
return makeParserErrorResult(
ParameterList::createEmpty(Context, PreviousLoc, PreviousLoc));
Expand Down
2 changes: 2 additions & 0 deletions test/Parse/init_deinit.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ struct FooStructConstructorC {

struct FooStructDeinitializerA {
deinit // expected-error {{expected '{' for deinitializer}}
deinit x // expected-error {{deinitializers cannot have a name}} {{10-12=}}
deinit x() // expected-error {{deinitializers cannot have a name}} {{10-11=}}
}

struct FooStructDeinitializerB {
Expand Down
16 changes: 16 additions & 0 deletions test/Parse/recovery.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,22 @@ func a(s: S[{{g) -> Int {}
// expected-error@+1{{expected an identifier to name generic parameter}}
func F() { init<( } )} // expected-note {{did you mean 'F'?}}

struct InitializerWithName {
init x() {} // expected-error {{initializers cannot have a name}} {{8-9=}}
}

struct InitializerWithNameAndParam {
init a(b: Int) {} // expected-error {{initializers cannot have a name}} {{8-9=}}
}

struct InitializerWithLabels {
init c d: Int {}
// expected-error @-1 {{expected '(' for initializer parameters}}
// expected-error @-2 {{expected declaration}}
// expected-error @-3 {{consecutive declarations on a line must be separated by ';'}}
// expected-note @-5 {{in declaration of 'InitializerWithLabels'}}
}

// rdar://20337695
func f1() {

Expand Down
26 changes: 21 additions & 5 deletions test/Parse/subscripting.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,8 @@ let y2 = Y2() // expected-note{{change 'let' to 'var' to make it mutable}}{{1-4=
_ = y2[0] // expected-error{{cannot use mutating getter on immutable value: 'y2' is a 'let' constant}}

// Parsing errors
// FIXME: Recovery here is horrible
struct A0 {
subscript // expected-error{{expected '(' for subscript parameters}}
subscript // expected-error {{expected '(' for subscript parameters}}
i : Int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting; because of the following colon, we ought to realize that it's parens that are missing, not a spurious name being added. I'm not sure this is a net improvement in this case.

Mind checking for a colon too? That would help initializers as well, though less so since init foo bar : Int could be a mistake for init foo(bar: Int) or init(foo bar: Int).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it’s preferable to treat init foo bar : Int as a mistake for int(foo bar : Int), since adding the parens would make the syntax valid. If so, I can restrict this diagnostic to require a following paren, and let the existing one handle that case (and do the same for subscripts). What do you think? (No problem if not. I can check for a colon following the identifier.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your way seems reasonable. Thanks, Matthew!

-> Int {
get {
Expand All @@ -92,6 +91,10 @@ struct A0 {
stored = value
}
}

subscript -> Int { // expected-error {{expected '(' for subscript parameters}} {{12-12=()}}
return 1
}
}

struct A1 {
Expand Down Expand Up @@ -172,10 +175,23 @@ struct A8 {
stored = value
}
}
} // expected-error{{extraneous '}' at top level}} {{1-3=}}

struct A9 {
subscript -> Int { // expected-error {{expected '(' for subscript parameters}} {{12-12=()}}
return 1
subscript x() -> Int { // expected-error {{subscripts cannot have a name}} {{13-14=}}
return 0
}
}

struct A10 {
subscript x(i: Int) -> Int { // expected-error {{subscripts cannot have a name}} {{13-14=}}
return 0
}
}

struct A11 {
subscript x y : Int -> Int { // expected-error {{expected '(' for subscript parameters}}
return 0
}
}

} // expected-error{{extraneous '}' at top level}} {{1-3=}}