Merge pull request swiftlang#24059 from sl/sl/sr-10293

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

Author: sl

include/swift/AST/DiagnosticsParse.def

@@ -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))
 
 

include/swift/Parse/Parser.h

@@ -609,6 +609,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

include/swift/Parse/Token.h

@@ -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.

lib/Basic/StringExtras.cpp

@@ -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);

lib/Parse/ParsePattern.cpp

@@ -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")))) {
@@ -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 {
@@ -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);

lib/Parse/Parser.cpp

@@ -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) &&
@@ -1006,7 +1039,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;
     }

test/IDE/complete_escaped_keyword.swift

@@ -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)
@@ -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)
@@ -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)

test/IDE/print_ast_tc_decls.swift

@@ -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 }
@@ -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: {{^}}}{{$}}
 }
 

test/Parse/invalid.swift

@@ -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() {
@@ -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}}

test/Sema/immutability.swift

@@ -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.
@@ -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]
 }
 
@@ -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) {}
@@ -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}}