Merge pull request #72340 from ahoppen/ahoppen/disallow-specifier-before-param

[Parse] Disallow specifiers in front of parameter declarations

Author: ahoppen

lib/Parse/ParsePattern.cpp

@@ -232,6 +232,15 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
       // ('inout' | '__shared' | '__owned' | isolated)?
       bool hasSpecifier = false;
       while (isParameterSpecifier()) {
+        // Placing 'inout' in front of the parameter specifiers was allowed in
+        // the Swift 2-ish era and got moved to the return type in Swift 3
+        // (SE-0031).
+        // But new parameters that don't store there location in
+        // `SpecifierLoc` were added afterwards and didn't get diagnosed.
+        // We thus need to parameter specifiers that don't store their location
+        // in `SpecifierLoc` here. `SpecifierLoc` parameters get diagnosed in
+        // `validateParameterWithOwnership`
+
         // is this token the identifier of an argument label? `inout` is a
         // reserved keyword but the other modifiers are not.
         if (!Tok.is(tok::kw_inout)) {
@@ -247,6 +256,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         }
 
         if (Tok.isContextualKeyword("isolated")) {
+          diagnose(Tok, diag::parameter_specifier_as_attr_disallowed, Tok.getText())
+                    .warnUntilSwiftVersion(6);
           // did we already find an 'isolated' type modifier?
           if (param.IsolatedLoc.isValid()) {
             diagnose(Tok, diag::parameter_specifier_repeated)
@@ -261,12 +272,16 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         }
 
         if (Tok.isContextualKeyword("_const")) {
+          diagnose(Tok, diag::parameter_specifier_as_attr_disallowed, Tok.getText())
+                    .warnUntilSwiftVersion(6);
           param.CompileConstLoc = consumeToken();
           continue;
         }
 
         if (Context.LangOpts.hasFeature(Feature::TransferringArgsAndResults) &&
             Tok.isContextualKeyword("transferring")) {
+          diagnose(Tok, diag::parameter_specifier_as_attr_disallowed, Tok.getText())
+                    .warnUntilSwiftVersion(6);
           if (param.TransferringLoc.isValid()) {
             diagnose(Tok, diag::parameter_specifier_repeated)
                 .fixItRemove(Tok.getLoc());

test/Parse/parameter_specifier_before_param_name.swift

@@ -0,0 +1,25 @@
+// RUN: %target-typecheck-verify-swift
+// REQUIRES: concurrency
+
+@available(SwiftStdlib 5.1, *)
+actor MyActor {}
+
+class MyClass {}
+
+// Lifetime specifiers before parameter names were disallowed in Swift 3 (SE-0031).
+// `isolated`, `transferring` and `_const` got added after Swift 3 without a diagnostic 
+// to disallow them before parameter names.
+
+func foo(inout x b: MyClass) {} // expected-error {{'inout' before a parameter name is not allowed, place it before the parameter type instead}}
+
+func foo(borrowing x b: MyClass) {} // expected-error {{'borrowing' before a parameter name is not allowed, place it before the parameter type instead}}
+
+@available(SwiftStdlib 5.1, *)
+func foo(isolated x b: MyActor) {} // expected-warning {{'isolated' before a parameter name is not allowed, place it before the parameter type instead; this is an error in the Swift 6 language mode}}
+
+func foo(_const x b: MyClass) {} // expected-warning {{'_const' before a parameter name is not allowed, place it before the parameter type instead; this is an error in the Swift 6 language mode}}
+
+// expected-error@+3 {{expected ',' separator}}
+// expected-error@+2 {{expected ':' following argument label and parameter name}}
+@available(SwiftStdlib 5.1, *)
+func foo(transferring x b: MyActor) {}

test/Sema/const_keypath.swift

@@ -6,7 +6,8 @@ struct Article {
 
 let keypath = \Article.id
 func keypath_generator() -> KeyPath<Article, String> { return \.id }
-func const_map(_const _ map: KeyPath<Article, String>) {}
+func const_map(_ map: _const KeyPath<Article, String>) {}
+func const_map_in_wrong_position(_const _ map: KeyPath<Article, String>) {} // expected-warning {{'_const' before a parameter name is not allowed, place it before the parameter type instead; this is an error in the Swift 6 language mode}}
 
 const_map(\.id)
 const_map(\Article.id)