Skip to content

Commit 34ca3e9

Browse files
lattnerlattner
committed
Merge pull request #1812 from manavgabhawala/master
[Parser] Cleans up parsing of function parameter attributes
2 parents 589d9a8 + 7862f10 commit 34ca3e9

39 files changed

+299
-146
lines changed

‎include/swift/AST/DiagnosticsParse.def

+6-6
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,6 @@ WARNING(lex_editor_placeholder_in_playground,none,
148148

149149
NOTE(note_in_decl_extension,none,
150150
"in %select{declaration|extension}0 of %1", (bool, Identifier))
151-
WARNING(inout_as_attr_deprecated,none,
152-
"'inout' before a parameter name is deprecated, place it before the parameter type instead",
153-
())
154151
WARNING(line_directive_style_deprecated,none,
155152
"#line directive is deprecated, please use #sourceLocation instead",
156153
())
@@ -692,12 +689,15 @@ ERROR(multiple_parameter_ellipsis,none,
692689
"only a single variadic parameter '...' is permitted", ())
693690
ERROR(parameter_vararg_default,none,
694691
"variadic parameter cannot have a default value", ())
695-
ERROR(parameter_inout_var_let,none,
692+
ERROR(inout_as_attr_disallowed,none,
693+
"'inout' before a parameter name is not allowed, place it before the parameter type instead",
694+
())
695+
ERROR(parameter_inout_var_let_repeated,none,
696696
"parameter may not have multiple 'inout', 'var', or 'let' specifiers",
697697
())
698+
ERROR(parameter_let_as_attr,none,
699+
"'let' as a parameter attribute is not allowed", ())
698700

699-
ERROR(var_parameter_not_allowed,none,
700-
"parameters may not have the 'var' specifier", ())
701701

702702
ERROR(expected_behavior_name,none,
703703
"expected behavior name after '[' in property declaration", ())

‎include/swift/AST/DiagnosticsSema.def

+3
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,9 @@ ERROR(inout_cant_be_variadic,none,
778778
"inout arguments cannot be variadic", ())
779779
ERROR(inout_only_parameter,none,
780780
"'inout' is only valid in parameter lists", ())
781+
ERROR(var_parameter_not_allowed,none,
782+
"parameters may not have the 'var' specifier", ())
783+
781784

782785
ERROR(mutating_invalid_global_scope,none,
783786
"'mutating' is only valid on methods", ())

‎include/swift/Parse/Parser.h

-1
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,6 @@ class Parser {
933933
DeclAttributes Attrs;
934934

935935
/// The location of the 'let', 'var', or 'inout' keyword, if present.
936-
///
937936
SourceLoc LetVarInOutLoc;
938937

939938
enum SpecifierKindTy {

‎lib/Parse/ParsePattern.cpp

+43-31
Original file line numberDiff line numberDiff line change
@@ -186,27 +186,31 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
186186
status |= makeParserCodeCompletionStatus();
187187
}
188188
}
189+
189190
// ('inout' | 'let' | 'var')?
190-
if (Tok.is(tok::kw_inout)) {
191-
param.LetVarInOutLoc = consumeToken();
192-
param.SpecifierKind = ParsedParameter::InOut;
193-
} else if (Tok.is(tok::kw_let)) {
194-
param.LetVarInOutLoc = consumeToken();
195-
param.SpecifierKind = ParsedParameter::Let;
196-
} else if (Tok.is(tok::kw_var)) {
197-
diagnose(Tok.getLoc(), diag::var_parameter_not_allowed)
198-
.fixItRemove(Tok.getLoc());
199-
param.LetVarInOutLoc = consumeToken();
200-
param.SpecifierKind = ParsedParameter::Var;
201-
}
202-
203-
// Redundant specifiers are fairly common, recognize, reject, and recover
204-
// from this gracefully.
205-
if (Tok.isAny(tok::kw_inout, tok::kw_let, tok::kw_var)) {
206-
diagnose(Tok, diag::parameter_inout_var_let)
191+
bool hasSpecifier = false;
192+
while (Tok.isAny(tok::kw_inout, tok::kw_let, tok::kw_var)) {
193+
if (!hasSpecifier) {
194+
if (Tok.is(tok::kw_let)) {
195+
diagnose(Tok, diag::parameter_let_as_attr)
196+
.fixItRemove(Tok.getLoc());
197+
param.isInvalid = true;
198+
} else {
199+
// We handle the var error in sema for a better fixit and inout is
200+
// handled later in this function for better fixits.
201+
param.SpecifierKind = Tok.is(tok::kw_inout) ? ParsedParameter::InOut :
202+
ParsedParameter::Var;
203+
}
204+
param.LetVarInOutLoc = consumeToken();
205+
hasSpecifier = true;
206+
} else {
207+
// Redundant specifiers are fairly common, recognize, reject, and recover
208+
// from this gracefully.
209+
diagnose(Tok, diag::parameter_inout_var_let_repeated)
207210
.fixItRemove(Tok.getLoc());
208-
consumeToken();
209-
param.isInvalid = true;
211+
consumeToken();
212+
param.isInvalid = true;
213+
}
210214
}
211215

212216
if (startsParameterName(*this, isClosure)) {
@@ -248,20 +252,28 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
248252

249253
bool hasDeprecatedInOut =
250254
param.SpecifierKind == ParsedParameter::InOut;
251-
252-
if (Tok.is(tok::kw_inout)) {
253-
param.LetVarInOutLoc = consumeToken();
254-
param.SpecifierKind = ParsedParameter::InOut;
255-
if (hasDeprecatedInOut) {
256-
diagnose(param.LetVarInOutLoc, diag::inout_as_attr_deprecated)
257-
.fixItRemove(param.LetVarInOutLoc);
255+
bool hasValidInOut = false;
256+
257+
while (Tok.is(tok::kw_inout)) {
258+
hasValidInOut = true;
259+
if (hasSpecifier) {
260+
diagnose(Tok.getLoc(), diag::parameter_inout_var_let_repeated)
261+
.fixItRemove(param.LetVarInOutLoc);
262+
consumeToken(tok::kw_inout);
263+
param.isInvalid = true;
264+
} else {
265+
hasSpecifier = true;
266+
param.LetVarInOutLoc = consumeToken(tok::kw_inout);
267+
param.SpecifierKind = ParsedParameter::InOut;
258268
}
259-
} else if (hasDeprecatedInOut) {
260-
diagnose(param.LetVarInOutLoc, diag::inout_as_attr_deprecated)
261-
.fixItRemove(param.LetVarInOutLoc)
262-
.fixItInsert(postColonLoc, "inout ");
263269
}
264-
270+
if (!hasValidInOut && hasDeprecatedInOut) {
271+
diagnose(Tok.getLoc(), diag::inout_as_attr_disallowed)
272+
.fixItRemove(param.LetVarInOutLoc)
273+
.fixItInsert(postColonLoc, "inout ");
274+
param.isInvalid = true;
275+
}
276+
265277
auto type = parseType(diag::expected_parameter_type);
266278
status |= type;
267279
param.Type = type.getPtrOrNull();

‎lib/Sema/TypeCheckPattern.cpp

+76
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,73 @@ static bool validateTypedPattern(TypeChecker &TC, DeclContext *DC,
711711
return hadError;
712712
}
713713

714+
static void diagnoseAndMigrateVarParameterToBody(ParamDecl *decl,
715+
AbstractFunctionDecl *func,
716+
TypeChecker &TC) {
717+
if (!func || !func->hasBody()) {
718+
// If there is no function body, just suggest removal.
719+
TC.diagnose(decl->getLetVarInOutLoc(),
720+
diag::var_parameter_not_allowed)
721+
.fixItRemove(decl->getLetVarInOutLoc());
722+
return;
723+
}
724+
// Insert the shadow copy. The computations that follow attempt to
725+
// 'best guess' the indentation and new lines so that the user
726+
// doesn't have to add any whitespace.
727+
auto declBody = func->getBody();
728+
729+
auto &SM = TC.Context.SourceMgr;
730+
731+
SourceLoc insertionStartLoc;
732+
std::string start;
733+
std::string end;
734+
735+
auto lBraceLine = SM.getLineNumber(declBody->getLBraceLoc());
736+
auto rBraceLine = SM.getLineNumber(declBody->getRBraceLoc());
737+
738+
if (!declBody->getNumElements()) {
739+
740+
// Empty function body.
741+
insertionStartLoc = declBody->getRBraceLoc();
742+
743+
if (lBraceLine == rBraceLine) {
744+
// Same line braces, means we probably have something
745+
// like {} as the func body. Insert directly into body with spaces.
746+
start = " ";
747+
end = " ";
748+
} else {
749+
// Different line braces, so use RBrace's indentation.
750+
end = "\n" + Lexer::getIndentationForLine(SM, declBody->
751+
getRBraceLoc()).str();
752+
start = " "; // Guess 4 spaces as extra indentation.
753+
}
754+
} else {
755+
auto firstLine = declBody->getElement(0);
756+
insertionStartLoc = firstLine.getStartLoc();
757+
if (lBraceLine == SM.getLineNumber(firstLine.getStartLoc())) {
758+
// Function on same line, insert with semi-colon. Not ideal but
759+
// better than weird space alignment.
760+
start = "";
761+
end = "; ";
762+
} else {
763+
start = "";
764+
end = "\n" + Lexer::getIndentationForLine(SM, firstLine.
765+
getStartLoc()).str();
766+
}
767+
}
768+
if (insertionStartLoc.isInvalid()) {
769+
TC.diagnose(decl->getLetVarInOutLoc(),
770+
diag::var_parameter_not_allowed)
771+
.fixItRemove(decl->getLetVarInOutLoc());
772+
return;
773+
}
774+
auto parameterName = decl->getNameStr().str();
775+
TC.diagnose(decl->getLetVarInOutLoc(),
776+
diag::var_parameter_not_allowed)
777+
.fixItRemove(decl->getLetVarInOutLoc())
778+
.fixItInsert(insertionStartLoc, start + "var " + parameterName + " = " +
779+
parameterName + end);
780+
}
714781

715782
static bool validateParameterType(ParamDecl *decl, DeclContext *DC,
716783
TypeResolutionOptions options,
@@ -736,6 +803,15 @@ static bool validateParameterType(ParamDecl *decl, DeclContext *DC,
736803
}
737804
decl->getTypeLoc().setType(Ty);
738805
}
806+
// If the param is not a 'let' and it is not an 'inout'.
807+
// It must be a 'var'. Provide helpful diagnostics like a shadow copy
808+
// in the function body to fix the 'var' attribute.
809+
if (!decl->isLet() && !Ty->is<InOutType>()) {
810+
auto func = dyn_cast_or_null<AbstractFunctionDecl>(DC);
811+
diagnoseAndMigrateVarParameterToBody(decl, func, TC);
812+
decl->setInvalid();
813+
hadError = true;
814+
}
739815

740816
if (hadError)
741817
decl->getTypeLoc().setType(ErrorType::get(TC.Context), /*validated*/true);

‎test/Constraints/tuple.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func testLValue(c: C) {
103103

104104

105105
// <rdar://problem/21444509> Crash in TypeChecker::coercePatternToType
106-
func invalidPatternCrash(let k : Int) {
106+
func invalidPatternCrash(k : Int) {
107107
switch k {
108108
case (k, cph_: k) as UInt8: // expected-error {{tuple pattern cannot match values of the non-tuple type 'UInt8'}} expected-warning {{cast from 'Int' to unrelated type 'UInt8' always fails}}
109109
break

‎test/FixCode/fixits-apply-all.swift

+10
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,13 @@
77
func ftest1() {
88
let myvar = 0
99
}
10+
11+
func foo() -> Int {
12+
do {
13+
} catch var err {
14+
goo(err)
15+
}
16+
}
17+
func goo(e: ErrorProtocol) {
18+
19+
}

‎test/FixCode/fixits-apply-all.swift.result

+10
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,13 @@
77
func ftest1() {
88
_ = 0
99
}
10+
11+
func foo() -> Int {
12+
do {
13+
} catch let err {
14+
goo(err)
15+
}
16+
}
17+
func goo(e: ErrorProtocol) {
18+
19+
}

‎test/FixCode/fixits-apply.swift

+16-5
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,25 @@ func supported() -> MyMask {
3131
return Int(MyMask.Bingo.rawValue)
3232
}
3333

34-
func foo() -> Int {
35-
do {
36-
} catch var err {
37-
goo(err)
34+
func goo(var e : ErrorProtocol) {
35+
}
36+
func goo2(var e: ErrorProtocol) {}
37+
func goo3(var e: Int) { e = 3 }
38+
protocol A {
39+
func bar(var s: Int)
40+
}
41+
extension A {
42+
func bar(var s: Int) {
43+
s += 5
3844
}
3945
}
4046

41-
func goo(var e : ErrorProtocol) {}
47+
func baz(var x: Int) {
48+
x += 10
49+
}
50+
func foo(let y: String, inout x: Int) {
51+
52+
}
4253

4354
struct Test1 : OptionSet {
4455
init(rawValue: Int) {}

‎test/FixCode/fixits-apply.swift.result

+19-5
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,28 @@ func supported() -> MyMask {
3131
return MyMask.Bingo
3232
}
3333

34-
func foo() -> Int {
35-
do {
36-
} catch let err {
37-
goo(err)
34+
func goo(e : ErrorProtocol) {
35+
var e = e
36+
}
37+
func goo2(e: ErrorProtocol) { var e = e }
38+
func goo3(e: Int) { var e = e; e = 3 }
39+
protocol A {
40+
func bar(s: Int)
41+
}
42+
extension A {
43+
func bar(s: Int) {
44+
var s = s
45+
s += 5
3846
}
3947
}
4048

41-
func goo(e : ErrorProtocol) {}
49+
func baz(x: Int) {
50+
var x = x
51+
x += 10
52+
}
53+
func foo(y: String, x: inout Int) {
54+
55+
}
4256

4357
struct Test1 : OptionSet {
4458
init(rawValue: Int) {}

‎test/IDE/complete_from_stdlib.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func testPostfixOperator1(x: Int) {
229229
// POSTFIX_RVALUE_INT-NOT: ++
230230
// POSTFIX_RVALUE_INT-NOT: --
231231

232-
func testPostfixOperator2(var x: Int) {
232+
func testPostfixOperator2(x: inout Int) {
233233
x#^POSTFIX_INT_2^#
234234
}
235235
// POSTFIX_LVALUE_INT: Decl[PostfixOperatorFunction]/OtherModule[Swift]: ++[#Int#]; name=
@@ -253,7 +253,7 @@ func testInfixOperator1(x: Int) {
253253
// INFIX_INT: End completions
254254
// NEGATIVE_INFIX_INT-NOT: &&
255255
// NEGATIVE_INFIX_INT-NOT: +=
256-
func testInfixOperator2(var x: Int) {
256+
func testInfixOperator2(x: inout Int) {
257257
x#^INFIX_INT_2^#
258258
}
259259
// INFIX_LVALUE_INT: Begin completions

‎test/IDE/complete_operators.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func testPostfix1(x: S) {
6666
}
6767
// POSTFIX_1-NOT: ++
6868

69-
func testPostfix2(var x: S) {
69+
func testPostfix2(x: inout S) {
7070
x#^POSTFIX_2^#
7171
}
7272
// POSTFIX_2: Begin completions
@@ -130,7 +130,7 @@ func testPostfix10<G: P where G.T : Fooable>(x: G) {
130130
}
131131
// POSTFIX_10: Decl[PostfixOperatorFunction]/CurrModule: ***[#G.T#]
132132

133-
func testPostfixSpace(var x: S) {
133+
func testPostfixSpace(x: inout S) {
134134
x #^S_POSTFIX_SPACE^#
135135
}
136136
// S_POSTFIX_SPACE: Decl[PostfixOperatorFunction]/CurrModule/Erase[1]: ++[#S#]
@@ -167,7 +167,7 @@ func testInfix1(x: S2) {
167167
// NEGATIVE_S2_INFIX-NOT: ~>
168168
// NEGATIVE_S2_INFIX-NOT: = {#
169169

170-
func testInfix2(var x: S2) {
170+
func testInfix2(x: inout S2) {
171171
x#^INFIX_2^#
172172
}
173173
// S2_INFIX_LVALUE: Begin completions
@@ -296,7 +296,7 @@ func testSpace(x: S2) {
296296
// S2_INFIX_SPACE-DAG: Decl[InfixOperatorFunction]/OtherModule[Swift]: [' ']+ {#S2#}[#S2#]
297297
// S2_INFIX_SPACE: End completions
298298

299-
func testExtInfix1(var x: S2) {
299+
func testExtInfix1(x: inout S2) {
300300
x + S2() + x + S2() + x + S2() + x#^EXT_INFIX_1^#
301301
}
302302

0 commit comments

Comments
 (0)