Skip to content

Allow var / let as parameter names, but provide a warning, and fixit to add backticks. #24059

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.

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 3 commits into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 4 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -852,10 +852,9 @@ ERROR(parameter_specifier_as_attr_disallowed,none,
"'%0' before a parameter name is not allowed, place it before the parameter type instead",
(StringRef))
ERROR(parameter_specifier_repeated,none,
"parameter must not have multiple '__owned', 'inout', '__shared',"
" 'var', or 'let' specifiers", ())
ERROR(parameter_let_var_as_attr,none,
"'%0' as a parameter attribute is not allowed",
"parameter must not have multiple '__owned', 'inout', or '__shared' specifiers", ())
WARNING(parameter_let_var_as_attr,none,
"'%0' in this position is interpreted as an argument label",
(StringRef))


Expand Down
2 changes: 2 additions & 0 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,8 @@ class Parser {

void skipUntilDeclRBrace(tok T1, tok T2);

void skipListUntilDeclRBrace(SourceLoc startLoc, tok T1, tok T2);

/// Skip a single token, but match parentheses, braces, and square brackets.
///
/// Note: this does \em not match angle brackets ("<" and ">")! These are
Expand Down
4 changes: 2 additions & 2 deletions include/swift/Parse/Token.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ class Token {
return true;
}

// 'let', 'var', and 'inout' cannot be argument labels.
if (isAny(tok::kw_let, tok::kw_var, tok::kw_inout))
// inout cannot be used as an argument label.
if (is(tok::kw_inout))
return false;

// All other keywords can be argument labels.
Expand Down
2 changes: 0 additions & 2 deletions lib/Basic/StringExtras.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ using namespace camel_case;

bool swift::canBeArgumentLabel(StringRef identifier) {
return llvm::StringSwitch<bool>(identifier)
.Case("var", false)
.Case("let", false)
.Case("inout", false)
.Case("$", false)
.Default(true);
Expand Down
17 changes: 10 additions & 7 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,9 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
}
}

// ('inout' | 'let' | 'var' | '__shared' | '__owned')?
// ('inout' | '__shared' | '__owned')?
bool hasSpecifier = false;
while (Tok.isAny(tok::kw_inout, tok::kw_let, tok::kw_var) ||
while (Tok.is(tok::kw_inout) ||
(Tok.is(tok::identifier) &&
(Tok.getRawText().equals("__shared") ||
Tok.getRawText().equals("__owned")))) {
Expand All @@ -254,10 +254,6 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
// better fixits.
param.SpecifierKind = VarDecl::Specifier::Owned;
param.SpecifierLoc = consumeToken();
} else {
diagnose(Tok, diag::parameter_let_var_as_attr, Tok.getText())
.fixItRemove(Tok.getLoc());
consumeToken();
}
hasSpecifier = true;
} else {
Expand All @@ -268,7 +264,14 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
consumeToken();
}
}


// If let or var is being used as an argument label, allow it but
// generate a warning.
if (!isClosure && Tok.isAny(tok::kw_let, tok::kw_var)) {
diagnose(Tok, diag::parameter_let_var_as_attr, Tok.getText())
.fixItReplace(Tok.getLoc(), "`" + Tok.getText().str() + "`");
}

if (startsParameterName(*this, isClosure)) {
// identifier-or-none for the first name
param.FirstNameLoc = consumeArgumentLabel(param.FirstName);
Expand Down
35 changes: 34 additions & 1 deletion lib/Parse/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,39 @@ void Parser::skipUntilDeclStmtRBrace(tok T1, tok T2) {
}
}

void Parser::skipListUntilDeclRBrace(SourceLoc startLoc, tok T1, tok T2) {
while (Tok.isNot(T1, T2, tok::eof, tok::r_brace, tok::pound_endif,
tok::pound_else, tok::pound_elseif)) {
bool hasDelimiter = Tok.getLoc() == startLoc || consumeIf(tok::comma);
bool possibleDeclStartsLine = Tok.isAtStartOfLine();

if (isStartOfDecl()) {

// Could have encountered something like `_ var:`
// or `let foo:` or `var:`
if (Tok.isAny(tok::kw_var, tok::kw_let)) {
if (possibleDeclStartsLine && !hasDelimiter) {
break;
}

Parser::BacktrackingScope backtrack(*this);
// Consume the let or var
consumeToken();

// If the following token is either <identifier> or :, it means that
// this `var` or `let` shoud be interpreted as a label
if ((Tok.canBeArgumentLabel() && peekToken().is(tok::colon)) ||
peekToken().is(tok::colon)) {
backtrack.cancelBacktrack();
continue;
}
}
break;
}
skipSingle();
}
}

void Parser::skipUntilDeclRBrace(tok T1, tok T2) {
while (Tok.isNot(T1, T2, tok::eof, tok::r_brace, tok::pound_endif,
tok::pound_else, tok::pound_elseif) &&
Expand Down Expand Up @@ -1001,7 +1034,7 @@ Parser::parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
// If we haven't made progress, or seeing any error, skip ahead.
if (Tok.getLoc() == StartLoc || Status.isError()) {
assert(Status.isError() && "no progress without error");
skipUntilDeclRBrace(RightK, tok::comma);
skipListUntilDeclRBrace(LeftLoc, RightK, tok::comma);
if (Tok.is(RightK) || Tok.isNot(tok::comma))
break;
}
Expand Down
6 changes: 3 additions & 3 deletions test/IDE/complete_escaped_keyword.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ enum MyEnum {
// STATIC_PRIMARY: Begin completion
// STATIC_PRIMARY-DAG: Decl[LocalVar]/Local: self[#MyEnum.Type#]; name=self
// STATIC_PRIMARY-DAG: Decl[EnumElement]/CurrNominal: `class`({#struct: String#})[#MyEnum#]; name=`class`(struct: String)
// STATIC_PRIMARY-DAG: Decl[EnumElement]/CurrNominal: `let`({#`var`: String#})[#MyEnum#]; name=`let`(`var`: String)
// STATIC_PRIMARY-DAG: Decl[EnumElement]/CurrNominal: `let`({#var: String#})[#MyEnum#]; name=`let`(var: String)
// STATIC_PRIMARY-DAG: Decl[StaticMethod]/CurrNominal: `public`({#private: String#})[#Int#]; name=`public`(private: String)
// STATIC_PRIMARY-DAG: Decl[InstanceMethod]/CurrNominal: `init`({#(self): MyEnum#})[#(deinit: String) -> Int#]; name=`init`(self: MyEnum)
// STATIC_PRIMARY-DAG: Decl[InstanceMethod]/CurrNominal: `if`({#(self): MyEnum#})[#(else: String) -> Int#]; name=`if`(self: MyEnum)
Expand All @@ -37,7 +37,7 @@ enum MyEnum {
// STATIC_SELF_NODOT: Begin completions
// STATIC_SELF_NODOT-DAG: Keyword[self]/CurrNominal: .self[#MyEnum.Type#]; name=self
// STATIC_SELF_NODOT-DAG: Decl[EnumElement]/CurrNominal: .class({#struct: String#})[#MyEnum#]; name=class(struct: String)
// STATIC_SELF_NODOT-DAG: Decl[EnumElement]/CurrNominal: .let({#`var`: String#})[#MyEnum#]; name=let(`var`: String)
// STATIC_SELF_NODOT-DAG: Decl[EnumElement]/CurrNominal: .let({#var: String#})[#MyEnum#]; name=let(var: String)
// STATIC_SELF_NODOT-DAG: Decl[Constructor]/CurrNominal: .init({#init: String#})[#MyEnum#]; name=init(init: String)
// STATIC_SELF_NODOT-DAG: Decl[StaticMethod]/CurrNominal: .public({#private: String#})[#Int#]; name=public(private: String)
// STATIC_SELF_NODOT-DAG: Decl[InstanceMethod]/CurrNominal: .`init`({#(self): MyEnum#})[#(deinit: String) -> Int#]; name=`init`(self: MyEnum)
Expand All @@ -48,7 +48,7 @@ enum MyEnum {
// STATIC_SELF_DOT: Begin completions
// STATIC_SELF_DOT-DAG: Keyword[self]/CurrNominal: self[#MyEnum.Type#]; name=self
// STATIC_SELF_DOT-DAG: Decl[EnumElement]/CurrNominal: class({#struct: String#})[#MyEnum#]; name=class(struct: String)
// STATIC_SELF_DOT-DAG: Decl[EnumElement]/CurrNominal: let({#`var`: String#})[#MyEnum#]; name=let(`var`: String)
// STATIC_SELF_DOT-DAG: Decl[EnumElement]/CurrNominal: let({#var: String#})[#MyEnum#]; name=let(var: String)
// STATIC_SELF_DOT-DAG: Decl[Constructor]/CurrNominal: init({#init: String#})[#MyEnum#]; name=init(init: String)
// STATIC_SELF_DOT-DAG: Decl[StaticMethod]/CurrNominal: public({#private: String#})[#Int#]; name=public(private: String)
// STATIC_SELF_DOT-DAG: Decl[InstanceMethod]/CurrNominal: `init`({#(self): MyEnum#})[#(deinit: String) -> Int#]; name=`init`(self: MyEnum)
Expand Down
4 changes: 2 additions & 2 deletions test/IDE/print_ast_tc_decls.swift
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ struct d0200_EscapedIdentifiers {
// PASS_COMMON-NEXT: {{^}} @_hasInitialValue var `var`: {{(d0200_EscapedIdentifiers.)?}}`struct`{{$}}

var tupleType: (`var`: Int, `let`: `struct`)
// PASS_COMMON-NEXT: {{^}} var tupleType: (`var`: Int, `let`: {{(d0200_EscapedIdentifiers.)?}}`struct`){{$}}
// PASS_COMMON-NEXT: {{^}} var tupleType: (var: Int, let: {{(d0200_EscapedIdentifiers.)?}}`struct`){{$}}

var accessors1: Int {
get { return 0 }
Expand All @@ -633,7 +633,7 @@ struct d0200_EscapedIdentifiers {
static func `static`(protocol: Int) {}
// PASS_COMMON-NEXT: {{^}} static func `static`(protocol: Int){{$}}

// PASS_COMMON-NEXT: {{^}} init(`var`: {{(d0200_EscapedIdentifiers.)?}}`struct` = {{(d0200_EscapedIdentifiers.)?}}`struct`(), tupleType: (`var`: Int, `let`: {{(d0200_EscapedIdentifiers.)?}}`struct`)){{$}}
// PASS_COMMON-NEXT: {{^}} init(var: {{(d0200_EscapedIdentifiers.)?}}`struct` = {{(d0200_EscapedIdentifiers.)?}}`struct`(), tupleType: (var: Int, let: {{(d0200_EscapedIdentifiers.)?}}`struct`)){{$}}
// PASS_COMMON-NEXT: {{^}}}{{$}}
}

Expand Down
40 changes: 14 additions & 26 deletions test/Parse/invalid.swift
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
// RUN: %target-typecheck-verify-swift

// rdar://15946844
func test1(inout var x : Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{18-22=}}
func test1(inout var x : Int) {} // expected-warning {{'var' in this position is interpreted as an argument label}} {{18-21=`var`}}
// expected-error @-1 {{'inout' before a parameter name is not allowed, place it before the parameter type instead}} {{12-17=}} {{26-26=inout }}
func test2(inout let x : Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{18-22=}}
func test2(inout let x : Int) {} // expected-warning {{'let' in this position is interpreted as an argument label}} {{18-21=`let`}}
// expected-error @-1 {{'inout' before a parameter name is not allowed, place it before the parameter type instead}} {{12-17=}} {{26-26=inout }}
func test3(f : (inout _ x : Int) -> Void) {} // expected-error {{'inout' before a parameter name is not allowed, place it before the parameter type instead}}

func test1s(__shared var x : Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{22-26=}}
func test1s(__shared var x : Int) {} // expected-warning {{'var' in this position is interpreted as an argument label}} {{22-25=`var`}}
// expected-error @-1 {{'__shared' before a parameter name is not allowed, place it before the parameter type instead}} {{13-21=}} {{30-30=__shared }}
func test2s(__shared let x : Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{22-26=}}
func test2s(__shared let x : Int) {} // expected-warning {{'let' in this position is interpreted as an argument label}} {{22-25=`let`}}
// expected-error @-1 {{'__shared' before a parameter name is not allowed, place it before the parameter type instead}} {{13-21=}} {{30-30=__shared }}

func test1o(__owned var x : Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{21-25=}}
func test1o(__owned var x : Int) {} // expected-warning {{'var' in this position is interpreted as an argument label}} {{21-24=`var`}}
// expected-error @-1 {{'__owned' before a parameter name is not allowed, place it before the parameter type instead}} {{13-20=}}
func test2o(__owned let x : Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{21-25=}}
func test2o(__owned let x : Int) {} // expected-warning {{'let' in this position is interpreted as an argument label}} {{21-24=`let`}}
// expected-error @-1 {{'__owned' before a parameter name is not allowed, place it before the parameter type instead}} {{13-20=}}

func test3() {
Expand Down Expand Up @@ -84,27 +84,15 @@ func SR698(_ a: Int, b: Int) {}
SR698(1, b: 2,) // expected-error {{unexpected ',' separator}}

// SR-979 - Two inout crash compiler
func SR979a(a : inout inout Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{17-23=}}
func SR979a(a : inout inout Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{17-23=}}
func SR979b(inout inout b: Int) {} // expected-error {{inout' before a parameter name is not allowed, place it before the parameter type instead}} {{13-18=}} {{28-28=inout }}
// expected-error@-1 {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{19-25=}}
func SR979c(let a: inout Int) {} // expected-error {{'let' as a parameter attribute is not allowed}} {{13-16=}}
func SR979d(let let a: Int) {} // expected-error {{'let' as a parameter attribute is not allowed}} {{13-16=}}
// expected-error @-1 {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{17-21=}}
func SR979e(inout x: inout String) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{13-18=}}
func SR979f(var inout x : Int) { // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{17-23=}}
// expected-error @-1 {{'var' as a parameter attribute is not allowed}}
x += 10 // expected-error {{left side of mutating operator isn't mutable: 'x' is a 'let' constant}}
}
func SR979g(inout i: inout Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{13-18=}}
func SR979h(let inout x : Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{17-23=}}
// expected-error @-1 {{'let' as a parameter attribute is not allowed}}
class VarTester {
init(var a: Int, var b: Int) {} // expected-error {{'var' as a parameter attribute is not allowed}}
// expected-error @-1 {{'var' as a parameter attribute is not allowed}}
func x(var b: Int) { //expected-error {{'var' as a parameter attribute is not allowed}}
b += 10 // expected-error {{left side of mutating operator isn't mutable: 'b' is a 'let' constant}}
}
}
// expected-error@-1 {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{19-25=}}
func SR979d(let let a: Int) {} // expected-warning {{'let' in this position is interpreted as an argument label}} {{13-16=`let`}}
// expected-error @-1 {{expected ',' separator}} {{20-20=,}}
// expected-error @-2 {{parameter requires an explicit type}}
// expected-warning @-3 {{extraneous duplicate parameter name; 'let' already has an argument label}} {{13-17=}}
func SR979e(inout x: inout String) {} // expected-error {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{13-18=}}
func SR979g(inout i: inout Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{13-18=}}

func repeat() {}
// expected-error @-1 {{keyword 'repeat' cannot be used as an identifier here}}
Expand Down
31 changes: 17 additions & 14 deletions test/Sema/immutability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func test_mutability() {

func test_arguments(_ a : Int,
b : Int,
let c : Int) { // expected-error {{'let' as a parameter attribute is not allowed}} {{21-25=}}
let c : Int) { // expected-warning {{'let' in this position is interpreted as an argument label}} {{21-24=`let`}}
var b = b
a = 1 // expected-error {{cannot assign to value: 'a' is a 'let' constant}}
b = 2 // ok.
Expand Down Expand Up @@ -354,7 +354,7 @@ protocol SubscriptNoGetter {
subscript (i: Int) -> Int { get }
}

func testSubscriptNoGetter(let iis: SubscriptNoGetter) { // expected-error {{'let' as a parameter attribute is not allowed}}{{28-31=}}
func testSubscriptNoGetter(let iis: SubscriptNoGetter) { // expected-warning {{'let' in this position is interpreted as an argument label}}{{28-31=`let`}}
var _: Int = iis[17]
}

Expand All @@ -367,24 +367,30 @@ func testSelectorStyleArguments1(_ x: Int, bar y: Int) {
_ = y
}

func testSelectorStyleArguments2(let x: Int, // expected-error {{'let' as a parameter attribute is not allowed}}{{34-37=}}
let bar y: Int) { // expected-error {{'let' as a parameter attribute is not allowed}}{{34-38=}}
func testSelectorStyleArguments2(let x: Int, // expected-warning {{'let' in this position is interpreted as an argument label}}{{34-37=`let`}}
let bar y: Int) { // expected-warning {{'let' in this position is interpreted as an argument label}}{{34-37=`let`}}
// expected-error @-1 {{expected ',' separator}}
// expected-error @-2 {{parameter requires an explicit type}}
}
func testSelectorStyleArguments3(_ x: Int, bar y: Int) {
++x // expected-error {{cannot pass immutable value to mutating operator: 'x' is a 'let' constant}}
++y // expected-error {{cannot pass immutable value to mutating operator: 'y' is a 'let' constant}}
}

func invalid_inout(inout var x : Int) { // expected-error {{parameter must not have multiple '__owned', 'inout', '__shared', 'var', or 'let' specifiers}} {{26-30=}}
// expected-error @-1 {{'inout' before a parameter name is not allowed, place it before the parameter type instead}}{{20-25=}}{{34-34=inout }}
func invalid_inout(inout var x : Int) { // expected-warning {{'var' in this position is interpreted as an argument label}} {{26-29=`var`}}
// expected-error @-1 {{'inout' before a parameter name is not allowed, place it before the parameter type instead}} {{20-25=}} {{34-34=inout }}
}
func invalid_var(var x: Int) { // expected-error {{'var' as a parameter attribute is not allowed}}


class VarTester {
init(var a: Int, var b: Int) {} // expected-warning {{'var' in this position is interpreted as an argument label}} {{8-11=`var`}}
// expected-warning @-1 {{'var' in this position is interpreted as an argument label}} {{20-23=`var`}}
func x(var b: Int) { //expected-warning {{'var' in this position is interpreted as an argument label}} {{10-13=`var`}}
b += 10 // expected-error {{left side of mutating operator isn't mutable: 'b' is a 'let' constant}}
}
}

func takesClosure(_: (Int) -> Int) {
takesClosure { (var d) in d } // expected-error {{'var' as a parameter attribute is not allowed}}
takesClosure { (var d) in d } // expected-error {{closure cannot have keyword arguments}}
}

func updateInt(_ x : inout Int) {}
Expand Down Expand Up @@ -501,9 +507,6 @@ struct StructWithDelegatingInit {
init() { self.init(x: 0); self.x = 22 } // expected-error {{'let' property 'x' may not be initialized directly; use "self.init(...)" or "self = ..." instead}}
}

func test_recovery_missing_name_2(let: Int) {} // expected-error {{'let' as a parameter attribute is not allowed}}{{35-38=}}
// expected-error @-1 {{expected parameter name followed by ':'}}

// <rdar://problem/16792027> compiler infinite loops on a really really mutating function
struct F { // expected-note 1 {{in declaration of 'F'}}
mutating mutating mutating f() { // expected-error 2 {{duplicate modifier}} expected-note 2 {{modifier already specified here}} expected-error {{expected declaration}}
Expand Down
4 changes: 2 additions & 2 deletions test/decl/class/override.swift
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ class H : G {

@objc func manyA(_: AnyObject, _: AnyObject) {}
@objc func manyB(_ a: AnyObject, b: AnyObject) {}
@objc func manyC(var a: AnyObject, // expected-error {{'var' as a parameter attribute is not allowed}}
var b: AnyObject) {} // expected-error {{'var' as a parameter attribute is not allowed}}
@objc func manyC(var a: AnyObject, // expected-warning {{'var' in this position is interpreted as an argument label}} {{20-23=`var`}}
var b: AnyObject) {} // expected-warning {{'var' in this position is interpreted as an argument label}} {{20-23=`var`}}

@objc func result() -> AnyObject? { return nil }
@objc func both(_ x: AnyObject) -> AnyObject? { return x }
Expand Down
Loading