Ungate accepted parts of SE439

include/swift/Basic/Features.def

@@ -398,7 +398,7 @@ EXPERIMENTAL_FEATURE(ReinitializeConsumeInMultiBlockDefer, false)
 EXPERIMENTAL_FEATURE(SE427NoInferenceOnExtension, false)
 
 // Enable trailing comma for comma-separated lists.
-EXPERIMENTAL_FEATURE(TrailingComma, true)
+EXPERIMENTAL_FEATURE(TrailingComma, false)
 
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE

lib/Parse/ParseDecl.cpp

@@ -2549,10 +2549,9 @@ Parser::parseMacroRoleAttribute(
   bool sawNames = false;
   SmallVector<MacroIntroducedDeclName, 2> names;
   SmallVector<TypeExpr *, 2> conformances;
-  auto argumentsStatus = parseList(tok::r_paren, lParenLoc, rParenLoc,
+  auto argumentsStatus = parseList(
-                                   /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
+      tok::r_paren, lParenLoc, rParenLoc,
-                                   diag::expected_rparen_expr_list,
+      /*AllowSepAfterLast=*/true, diag::expected_rparen_expr_list, [&] {
-                                   [&] {
         ParserStatus status;
 
         if (consumeIf(tok::code_complete)) {
@@ -2561,12 +2560,14 @@ Parser::parseMacroRoleAttribute(
             sawRole = true;
             if (this->CodeCompletionCallbacks) {
               this->CodeCompletionCallbacks->completeDeclAttrParam(
-              getCustomSyntaxAttributeKind(isAttached), 0, /*HasLabel=*/false);
+                  getCustomSyntaxAttributeKind(isAttached), 0,
+                  /*HasLabel=*/false);
             }
           } else if (!sawNames) {
             if (this->CodeCompletionCallbacks) {
               this->CodeCompletionCallbacks->completeDeclAttrParam(
-              getCustomSyntaxAttributeKind(isAttached), 1, /*HasLabel=*/false);
+                  getCustomSyntaxAttributeKind(isAttached), 1,
+                  /*HasLabel=*/false);
             }
           }
         }
@@ -2579,21 +2580,20 @@ Parser::parseMacroRoleAttribute(
         // If there is a field name, it better be 'names'.
         if (!(fieldName.empty() || fieldName.is("names") ||
               fieldName.is("conformances"))) {
-      diagnose(
+          diagnose(fieldNameLoc, diag::macro_attribute_unknown_label,
-         fieldNameLoc, diag::macro_attribute_unknown_label, isAttached,
+                   isAttached, fieldName);
-         fieldName);
           status.setIsParseError();
           return status;
         }
 
-    // If there is no field name and we haven't seen either names or the role,
+        // If there is no field name and we haven't seen either names or the
-    // this is the role.
+        // role, this is the role.
         if (fieldName.empty() && !sawConformances && !sawNames && !sawRole) {
           // Whether we saw anything we tried to treat as a role.
           sawRole = true;
 
-      auto diagKind = isAttached
+          auto diagKind =
-        ? diag::macro_role_attr_expected_attached_kind
+              isAttached ? diag::macro_role_attr_expected_attached_kind
                          : diag::macro_role_attr_expected_freestanding_kind;
           Identifier roleName;
           SourceLoc roleNameLoc;
@@ -2610,7 +2610,8 @@ Parser::parseMacroRoleAttribute(
             role = getMacroRole(roleName.str());
 
           if (!role) {
-        diagnose(roleNameLoc, diag::macro_role_attr_expected_kind, isAttached);
+            diagnose(roleNameLoc, diag::macro_role_attr_expected_kind,
+                     isAttached);
             status.setIsParseError();
             return status;
           }
@@ -2622,9 +2623,8 @@ Parser::parseMacroRoleAttribute(
 
           // Check that the role makes sense.
           if (isAttached == !isAttachedMacro(*role)) {
-        diagnose(
+            diagnose(roleNameLoc, diag::macro_role_syntax_mismatch, isAttached,
-            roleNameLoc, diag::macro_role_syntax_mismatch, isAttached, roleName
+                     roleName);
-        );
 
             status.setIsParseError();
             return status;
@@ -2637,8 +2637,7 @@ Parser::parseMacroRoleAttribute(
             (fieldName.empty() && sawConformances && !sawNames)) {
           if (fieldName.is("conformances") && sawConformances) {
             diagnose(fieldNameLoc.isValid() ? fieldNameLoc : Tok.getLoc(),
-                 diag::macro_attribute_duplicate_label,
+                     diag::macro_attribute_duplicate_label, isAttached,
-                 isAttached,
                      "conformances");
           }
 
@@ -2652,14 +2651,13 @@ Parser::parseMacroRoleAttribute(
           return status;
         }
 
-    // If the field name is empty and we haved seen "names", or the field name
+        // If the field name is empty and we haved seen "names", or the field
-    // is "names" but we've already seen the argument label, complain.
+        // name is "names" but we've already seen the argument label, complain.
         if (fieldName.empty() != sawNames) {
           diagnose(fieldNameLoc.isValid() ? fieldNameLoc : Tok.getLoc(),
                    sawNames ? diag::macro_attribute_duplicate_label
                             : diag::macro_attribute_missing_label,
-               isAttached,
+                   isAttached, "names");
-               "names");
         }
         sawNames = true;
 
@@ -2679,12 +2677,11 @@ Parser::parseMacroRoleAttribute(
           return status;
         }
 
-    auto introducedKind = getMacroIntroducedDeclNameKind(introducedNameKind);
+        auto introducedKind =
+            getMacroIntroducedDeclNameKind(introducedNameKind);
         if (!introducedKind) {
-      diagnose(
+          diagnose(introducedNameKindLoc,
-          introducedNameKindLoc, diag::macro_attribute_unknown_name_kind,
+                   diag::macro_attribute_unknown_name_kind, introducedNameKind);
-          introducedNameKind
-      );
           status.setIsParseError();
           return status;
         }
@@ -2693,8 +2690,8 @@ Parser::parseMacroRoleAttribute(
         if (!macroIntroducedNameRequiresArgument(*introducedKind)) {
           // If there is an argument, complain about it.
           if (Tok.is(tok::l_paren)) {
-        diagnose(
+            diagnose(Tok,
-            Tok, diag::macro_attribute_introduced_name_requires_no_argument,
+                     diag::macro_attribute_introduced_name_requires_no_argument,
                      introducedNameKind);
             skipSingle();
           }
@@ -2704,8 +2701,7 @@ Parser::parseMacroRoleAttribute(
         }
 
         if (!Tok.is(tok::l_paren)) {
-      diagnose(
+          diagnose(Tok, diag::macro_attribute_introduced_name_requires_argument,
-          Tok, diag::macro_attribute_introduced_name_requires_argument,
                    introducedNameKind);
           status.setIsParseError();
           return status;
@@ -2716,8 +2712,7 @@ Parser::parseMacroRoleAttribute(
         DeclNameLoc nameLoc;
         DeclNameRef name = parseDeclNameRef(
             nameLoc, diag::macro_attribute_unknown_argument_form,
-        (DeclNameFlag::AllowOperators |
+            (DeclNameFlag::AllowOperators | DeclNameFlag::AllowKeywords |
-         DeclNameFlag::AllowKeywords |
              DeclNameFlag::AllowKeywordsUsingSpecialNames |
              DeclNameFlag::AllowCompoundNames |
              DeclNameFlag::AllowZeroArgCompoundNames));
@@ -6804,10 +6799,13 @@ ParserStatus Parser::parseInheritance(
   ParserStatus Status;
   SourceLoc prevComma;
   bool HasNextType;
+  bool IsEndOfList;
   do {
     SWIFT_DEFER {
       // Check for a ',', which indicates that there are more protocols coming.
       HasNextType = consumeIf(tok::comma, prevComma);
+      IsEndOfList = (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+                     (Tok.is(tok::l_brace) || Tok.is(tok::kw_where)));
     };
     // Parse the 'class' keyword for a class requirement.
     if (Tok.is(tok::kw_class)) {
@@ -6859,7 +6857,7 @@ ParserStatus Parser::parseInheritance(
     // Record the type if its a single type.
     if (ParsedTypeResult.isNonNull())
       Inherited.push_back(InheritedEntry(ParsedTypeResult.get()));
-  } while (HasNextType);
+  } while (HasNextType && !IsEndOfList);
 
   return Status;
 }

lib/Parse/ParseExpr.cpp

@@ -1926,8 +1926,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
   default:
   UnknownCharacter:
     checkForInputIncomplete();
-    if (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+    if (Tok.is(tok::eof) && Tok.getText() == ")") {
-        Tok.is(tok::eof) && Tok.getText() == ")") {
       return nullptr;
     }
     // FIXME: offer a fixit: 'Self' -> 'self'
@@ -2774,8 +2773,7 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
         VD->setIsSelfParamCapture();
 
       captureList.push_back(CLE);
-    } while (HasNext && !(Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+    } while (HasNext && !Tok.is(tok::r_square));
-                          Tok.is(tok::r_square)));
 
     // The capture list needs to be closed off with a ']'.
     SourceLoc rBracketLoc = Tok.getLoc();
@@ -3295,11 +3293,12 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
   StructureMarkerRAII ParsingExprList(*this, Tok);
   
   leftLoc = consumeToken(leftTok);
-  return parseList(rightTok, leftLoc, rightLoc, /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
+  return parseList(rightTok, leftLoc, rightLoc, /*AllowSepAfterLast=*/true,
                    rightTok == tok::r_paren ? diag::expected_rparen_expr_list
                                             : diag::expected_rsquare_expr_list,
-                   [&] () -> ParserStatus {
+                   [&]() -> ParserStatus {
-    return parseExprListElement(rightTok, isArgumentList, leftLoc, elts);
+                     return parseExprListElement(rightTok, isArgumentList,
+                                                 leftLoc, elts);
                    });
 }
 

lib/Parse/ParseGeneric.cpp

@@ -134,8 +134,7 @@ Parser::parseGenericParametersBeforeWhere(SourceLoc LAngleLoc,
 
     // Parse the comma, if the list continues.
     HasNextParam = consumeIf(tok::comma);
-    IsEndOfList = (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+    IsEndOfList = startsWithGreater(Tok);
-                   startsWithGreater(Tok));
   } while (HasNextParam && !IsEndOfList);
 
   return Result;

lib/Parse/ParsePattern.cpp

@@ -213,9 +213,9 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
 
   // Parse the parameter list.
   bool isClosure = paramContext == ParameterContextKind::Closure;
-  return parseList(tok::r_paren, leftParenLoc, rightParenLoc,
+  return parseList(
-                      /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
+      tok::r_paren, leftParenLoc, rightParenLoc,
-                      diag::expected_rparen_parameter,
+      /*AllowSepAfterLast=*/true, diag::expected_rparen_parameter,
       [&]() -> ParserStatus {
         ParsedParameter param;
         ParserStatus status;
@@ -228,7 +228,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
           auto AttrStatus = parseDeclAttributeList(param.Attrs);
           if (AttrStatus.hasCodeCompletion()) {
             if (this->CodeCompletionCallbacks)
-          this->CodeCompletionCallbacks->setAttrTargetDeclKind(DeclKind::Param);
+              this->CodeCompletionCallbacks->setAttrTargetDeclKind(
+                  DeclKind::Param);
             status.setHasCodeCompletionAndIsError();
           }
         }
@@ -237,23 +238,24 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
           // ('inout' | '__shared' | '__owned' | isolated)?
           bool hasSpecifier = false;
           while (isParameterSpecifier()) {
-        // Placing 'inout' in front of the parameter specifiers was allowed in
+            // Placing 'inout' in front of the parameter specifiers was allowed
-        // the Swift 2-ish era and got moved to the return type in Swift 3
+            // in the Swift 2-ish era and got moved to the return type in Swift
-        // (SE-0031).
+            // 3 (SE-0031). But new parameters that don't store there location
-        // But new parameters that don't store there location in
+            // in `SpecifierLoc` were added afterwards and didn't get diagnosed.
-        // `SpecifierLoc` were added afterwards and didn't get diagnosed.
+            // We thus need to parameter specifiers that don't store their
-        // We thus need to parameter specifiers that don't store their location
+            // location in `SpecifierLoc` here. `SpecifierLoc` parameters get
-        // in `SpecifierLoc` here. `SpecifierLoc` parameters get diagnosed in
+            // diagnosed in `validateParameterWithOwnership`
-        // `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)) {
-          bool partOfArgumentLabel = lookahead<bool>(1, [&](CancellableBacktrackingScope &) {
+              bool partOfArgumentLabel =
+                  lookahead<bool>(1, [&](CancellableBacktrackingScope &) {
                     if (Tok.is(tok::colon))
                       return true; // isolated :
 
-            return Tok.canBeArgumentLabel() && peekToken().is(tok::colon);
+                    return Tok.canBeArgumentLabel() &&
+                           peekToken().is(tok::colon);
                   });
 
               if (partOfArgumentLabel)
@@ -261,7 +263,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
             }
 
             if (Tok.isContextualKeyword("isolated")) {
-          diagnose(Tok, diag::parameter_specifier_as_attr_disallowed, Tok.getText())
+              diagnose(Tok, diag::parameter_specifier_as_attr_disallowed,
+                       Tok.getText())
                   .warnUntilSwiftVersion(6);
               // did we already find an 'isolated' type modifier?
               if (param.IsolatedLoc.isValid()) {
@@ -277,7 +280,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
             }
 
             if (Tok.isContextualKeyword("_const")) {
-          diagnose(Tok, diag::parameter_specifier_as_attr_disallowed, Tok.getText())
+              diagnose(Tok, diag::parameter_specifier_as_attr_disallowed,
+                       Tok.getText())
                   .warnUntilSwiftVersion(6);
               param.CompileConstLoc = consumeToken();
               continue;
@@ -285,7 +289,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
 
             if (Context.LangOpts.hasFeature(Feature::SendingArgsAndResults) &&
                 Tok.isContextualKeyword("transferring")) {
-          diagnose(Tok, diag::parameter_specifier_as_attr_disallowed, Tok.getText())
+              diagnose(Tok, diag::parameter_specifier_as_attr_disallowed,
+                       Tok.getText())
                   .warnUntilSwiftVersion(6);
               if (param.TransferringLoc.isValid()) {
                 diagnose(Tok, diag::parameter_specifier_repeated)
@@ -379,8 +384,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
 
         auto parseParamType = [&]() -> ParserResult<TypeRepr> {
           // Currently none of the parameter type completions are relevant for
-      // enum cases, so don't include them. We'll complete for a regular type
+          // enum cases, so don't include them. We'll complete for a regular
-      // beginning instead.
+          // type beginning instead.
           if (paramContext != ParameterContextKind::EnumElement) {
             if (auto CCLoc = tryCompleteFunctionParamTypeBeginning()) {
               auto *ET = ErrorTypeRepr::create(Context, CCLoc);
@@ -392,20 +397,21 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
 
         if (startsParameterName(isClosure)) {
           // identifier-or-none for the first name
-      param.FirstNameLoc = consumeArgumentLabel(param.FirstName,
+          param.FirstNameLoc =
+              consumeArgumentLabel(param.FirstName,
                                    /*diagnoseDollarPrefix=*/!isClosure);
 
           // identifier-or-none? for the second name
           if (Tok.canBeArgumentLabel())
-        param.SecondNameLoc = consumeArgumentLabel(param.SecondName,
+            param.SecondNameLoc =
+                consumeArgumentLabel(param.SecondName,
                                      /*diagnoseDollarPrefix=*/true);
 
           // Operators, closures, and enum elements cannot have API names.
           if ((paramContext == ParameterContextKind::Operator ||
                paramContext == ParameterContextKind::Closure ||
                paramContext == ParameterContextKind::EnumElement) &&
-          !param.FirstName.empty() &&
+              !param.FirstName.empty() && param.SecondNameLoc.isValid()) {
-          param.SecondNameLoc.isValid()) {
             enum KeywordArgumentDiagnosticContextKind {
               Operator = 0,
               Closure = 1,
@@ -425,7 +431,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
             default:
               llvm_unreachable("Unhandled parameter context kind!");
             }
-        diagnose(param.FirstNameLoc, diag::parameter_operator_keyword_argument,
+            diagnose(param.FirstNameLoc,
+                     diag::parameter_operator_keyword_argument,
                      unsigned(diagContextKind))
                 .fixItRemoveChars(param.FirstNameLoc, param.SecondNameLoc);
             param.FirstName = param.SecondName;
@@ -441,8 +448,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
             status |= type;
             param.Type = type.getPtrOrNull();
 
-        // If we didn't parse a type, then we already diagnosed that the type
+            // If we didn't parse a type, then we already diagnosed that the
-        // was invalid.  Remember that.
+            // type was invalid.  Remember that.
             if (type.isNull() && !type.hasCodeCompletion())
               param.isInvalid = true;
           } else if (paramContext != Parser::ParameterContextKind::Closure) {
@@ -455,8 +462,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
           bool isBareType = false;
           {
             BacktrackingScope backtrack(*this);
-        isBareType = canParseType() && Tok.isAny(tok::comma, tok::r_paren,
+            isBareType = canParseType() &&
-                                                 tok::equal);
+                         Tok.isAny(tok::comma, tok::r_paren, tok::equal);
           }
 
           if (isBareType && paramContext == ParameterContextKind::EnumElement) {
@@ -468,12 +475,11 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
             param.SecondName = Identifier();
             param.SecondNameLoc = SourceLoc();
           } else if (isBareType && !Tok.is(tok::code_complete)) {
-        // Otherwise, if this is a bare type, then the user forgot to name the
+            // Otherwise, if this is a bare type, then the user forgot to name
-        // parameter, e.g. "func foo(Int) {}"
+            // the parameter, e.g. "func foo(Int) {}" Don't enter this case if
-        // Don't enter this case if the element could only be parsed as a bare
+            // the element could only be parsed as a bare type because a code
-        // type because a code completion token is positioned here. In this case
+            // completion token is positioned here. In this case the user is
-        // the user is about to type the parameter label and we shouldn't
+            // about to type the parameter label and we shouldn't suggest types.
-        // suggest types.
             SourceLoc typeStartLoc = Tok.getLoc();
             auto type = parseParamType();
             status |= type;
@@ -495,10 +501,11 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
                 // SE-110 depending on the kind of declaration.  We now need to
                 // warn about the misuse of this syntax and offer to
                 // fix it.
-            // An exception to this rule is when the type is declared with type sugar
+                // An exception to this rule is when the type is declared with
-            // Reference: https://github.com/apple/swift/issues/54133
+                // type sugar Reference:
-            if (isa<OptionalTypeRepr>(param.Type)
+                // https://github.com/apple/swift/issues/54133
-                || isa<ImplicitlyUnwrappedOptionalTypeRepr>(param.Type)) {
+                if (isa<OptionalTypeRepr>(param.Type) ||
+                    isa<ImplicitlyUnwrappedOptionalTypeRepr>(param.Type)) {
                   diagnose(typeStartLoc, diag::parameter_unnamed)
                       .fixItInsert(typeStartLoc, "_: ");
                 } else {
@@ -509,8 +516,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
               }
             }
           } else {
-        // Otherwise, we're not sure what is going on, but this doesn't smell
+            // Otherwise, we're not sure what is going on, but this doesn't
-        // like a parameter.
+            // smell like a parameter.
             diagnose(Tok, diag::expected_parameter_name);
             param.isInvalid = true;
             param.FirstNameLoc = Tok.getLoc();
@@ -535,7 +542,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
           SourceLoc EqualLoc = Tok.getLoc();
 
           if (isEqualBinaryOperator) {
-        diagnose(Tok, diag::expected_assignment_instead_of_comparison_operator)
+            diagnose(Tok,
+                     diag::expected_assignment_instead_of_comparison_operator)
                 .fixItReplace(EqualLoc, "=");
           }
 
@@ -545,9 +553,10 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
 
         // If we haven't made progress, don't add the parameter.
         if (Tok.getLoc() == StartLoc) {
-      // If we took a default argument index for this parameter, but didn't add
+          // If we took a default argument index for this parameter, but didn't
-      // one, then give it back.
+          // add one, then give it back.
-      if (defaultArgs) --defaultArgs->NextIndex;
+          if (defaultArgs)
+            --defaultArgs->NextIndex;
           return status;
         }
 
@@ -1312,11 +1321,10 @@ ParserResult<Pattern> Parser::parsePatternTuple() {
 
   // Parse all the elements.
   SmallVector<TuplePatternElt, 8> elts;
-  ParserStatus ListStatus =
+  ParserStatus ListStatus = parseList(
-    parseList(tok::r_paren, LPLoc, RPLoc,
+      tok::r_paren, LPLoc, RPLoc,
-              /*AllowSepAfterLast=*/Context.LangOpts.hasFeature(Feature::TrailingComma),
+      /*AllowSepAfterLast=*/true, diag::expected_rparen_tuple_pattern_list,
-              diag::expected_rparen_tuple_pattern_list,
+      [&]() -> ParserStatus {
-              [&] () -> ParserStatus {
         // Parse the pattern tuple element.
         ParserStatus EltStatus;
         std::optional<TuplePatternElt> elt;

lib/Parse/ParseStmt.cpp

@@ -1523,8 +1523,7 @@ Parser::parseAvailabilitySpecList(SmallVectorImpl<AvailabilitySpec *> &Specs,
       Status.setIsParseError();
     } else if (consumeIf(tok::comma)) {
       // End of list with a trailing comma.
-      if (Context.LangOpts.hasFeature(Feature::TrailingComma) &&
+      if (Source != AvailabilitySpecSource::Available && Tok.is(tok::r_paren)) {
-          Tok.is(tok::r_paren)) {
         break;
       }
       // There is more to parse in this list.