Parse / AST: Allow let / var as argument labels with a warning.

The diagnostic is now a warning and the new message alerts the user that
though it is valid to have let and var as argument label names,
they are interpreted as argument labels, not keywords.

Author: sl

include/swift/AST/DiagnosticsParse.def

@@ -854,8 +854,8 @@ ERROR(parameter_specifier_as_attr_disallowed,none,
 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",
+WARNING(parameter_let_var_as_attr,none,
+      "'%0' in this position is interpreted as an argument label",
       (StringRef))
 
 

include/swift/Parse/Parser.h

@@ -587,6 +587,8 @@ class Parser {
 
   void skipUntilDeclRBrace(tok T1, tok T2);
 
+  void skipListUntilDeclRBrace(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/AST/DiagnosticEngine.cpp

@@ -173,12 +173,10 @@ InFlightDiagnostic &InFlightDiagnostic::fixItRemove(SourceRange R) {
   // If we're removing something (e.g. a keyword), do a bit of extra work to
   // make sure that we leave the code in a good place, without extraneous white
   // space around its hole.  Specifically, check to see there is whitespace
-  // or a paren before and whitespace after the end of range.  If so, nuke
-  // the space afterward to keep things consistent.
-  char charBefore = extractCharBefore(SM, charRange.getStart());
-
+  // before and after the end of range.  If so, nuke the space afterward to keep
+  // things consistent.
   if (extractCharAfter(SM, charRange.getEnd()) == ' ' &&
-      (isspace(charBefore) || charBefore == '(')) {
+      isspace(extractCharBefore(SM, charRange.getStart()))) {
     charRange = CharSourceRange(charRange.getStart(),
                                 charRange.getByteLength()+1);
   }

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/ParseExpr.cpp

@@ -2109,7 +2109,15 @@ void Parser::parseOptionalArgumentLabel(Identifier &name, SourceLoc &loc) {
     // the syntax for referring to the function pointer (foo(_:)),
     auto escaped = Tok.isEscapedIdentifier();
     auto underscore = Tok.is(tok::kw__) || (escaped && text == "_");
-    if (escaped && !underscore && canBeArgumentLabel(text)) {
+    
+    auto requiresEscaping = [](StringRef identifier) {
+      return llvm::StringSwitch<bool>(identifier)
+        .Case("let", false)
+        .Case("var", false)
+        .Default(canBeArgumentLabel(identifier));
+    };
+    
+    if (escaped && !underscore && requiresEscaping(text)) {
       SourceLoc start = Tok.getLoc();
       SourceLoc end = start.getAdvancedLoc(Tok.getLength());
       diagnose(Tok, diag::escaped_parameter_name, text)

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,33 @@ void Parser::skipUntilDeclStmtRBrace(tok T1, tok T2) {
   }
 }
 
+void Parser::skipListUntilDeclRBrace(tok T1, tok T2) {
+  while (Tok.isNot(T1, T2, tok::eof, tok::r_brace, tok::pound_endif,
+                   tok::pound_else, tok::pound_elseif)) {
+    if (isStartOfDecl()) {
+      
+      // Could have encountered something like `_ var:` 
+      // or `let foo:` or `var:`
+      if (Tok.isAny(tok::kw_var, tok::kw_let)) {
+        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) &&
@@ -1001,7 +1028,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(RightK, tok::comma);
       if (Tok.is(RightK) || Tok.isNot(tok::comma))
         break;
     }