[ASTGen] Better handle ParamDecl arg/param name generation

Move the logic onto the ASTGen side, and plumb
through the flag that will allow subscripts
to have a different argument name defaulting
behavior.

Author: hamishknight

include/swift/AST/ASTBridging.h

@@ -426,13 +426,13 @@ BridgedPatternBindingDecl BridgedPatternBindingDecl_createParsed(
     BridgedSourceLoc cBindingKeywordLoc, BridgedArrayRef cBindingEntries,
     bool isStatic, bool isLet);
 
-SWIFT_NAME("BridgedParamDecl.createParsed(_:declContext:specifierLoc:firstName:"
-           "firstNameLoc:secondName:secondNameLoc:type:defaultValue:)")
+SWIFT_NAME("BridgedParamDecl.createParsed(_:declContext:specifierLoc:argName:"
+           "argNameLoc:paramName:paramNameLoc:type:defaultValue:)")
 BridgedParamDecl BridgedParamDecl_createParsed(
     BridgedASTContext cContext, BridgedDeclContext cDeclContext,
-    BridgedSourceLoc cSpecifierLoc, BridgedIdentifier cFirstName,
-    BridgedSourceLoc cFirstNameLoc, BridgedIdentifier cSecondName,
-    BridgedSourceLoc cSecondNameLoc, BridgedNullableTypeRepr type,
+    BridgedSourceLoc cSpecifierLoc, BridgedIdentifier cArgName,
+    BridgedSourceLoc cArgNameLoc, BridgedIdentifier cParamName,
+    BridgedSourceLoc cParamNameLoc, BridgedNullableTypeRepr type,
     BridgedNullableExpr defaultValue);
 
 SWIFT_NAME("BridgedConstructorDecl.setParsedBody(self:_:)")

lib/AST/ASTBridging.cpp

@@ -398,21 +398,10 @@ BridgedPatternBindingDecl BridgedPatternBindingDecl_createParsed(
 
 BridgedParamDecl BridgedParamDecl_createParsed(
     BridgedASTContext cContext, BridgedDeclContext cDeclContext,
-    BridgedSourceLoc cSpecifierLoc, BridgedIdentifier cFirstName,
-    BridgedSourceLoc cFirstNameLoc, BridgedIdentifier cSecondName,
-    BridgedSourceLoc cSecondNameLoc, BridgedNullableTypeRepr opaqueType,
+    BridgedSourceLoc cSpecifierLoc, BridgedIdentifier cArgName,
+    BridgedSourceLoc cArgNameLoc, BridgedIdentifier cParamName,
+    BridgedSourceLoc cParamNameLoc, BridgedNullableTypeRepr opaqueType,
     BridgedNullableExpr opaqueDefaultValue) {
-  auto firstName = cFirstName.unbridged();
-  auto firstNameLoc = cFirstNameLoc.unbridged();
-  auto secondName = cSecondName.unbridged();
-  auto secondNameLoc = cSecondNameLoc.unbridged();
-
-  assert(secondNameLoc.isValid() == secondName.nonempty());
-  if (secondName.empty()) {
-    secondName = firstName;
-    secondNameLoc = firstNameLoc;
-  }
-
   auto *declContext = cDeclContext.unbridged();
 
   auto *defaultValue = opaqueDefaultValue.unbridged();
@@ -426,9 +415,9 @@ BridgedParamDecl BridgedParamDecl_createParsed(
     defaultArgumentKind = getDefaultArgKind(defaultValue);
   }
 
-  auto *paramDecl = new (cContext.unbridged())
-      ParamDecl(cSpecifierLoc.unbridged(), firstNameLoc, firstName,
-                secondNameLoc, secondName, declContext);
+  auto *paramDecl = new (cContext.unbridged()) ParamDecl(
+      cSpecifierLoc.unbridged(), cArgNameLoc.unbridged(), cArgName.unbridged(),
+      cParamNameLoc.unbridged(), cParamName.unbridged(), declContext);
 
   if (auto type = opaqueType.unbridged()) {
     paramDecl->setTypeRepr(type);

lib/ASTGen/Sources/ASTGen/Decls.swift

@@ -498,7 +498,7 @@ extension ASTGenVisitor {
       name: name,
       nameLoc: nameLoc,
       genericParamList: self.generate(genericParameterClause: node.genericParameterClause),
-      parameterList: self.generate(functionParameterClause: node.signature.parameterClause),
+      parameterList: self.generate(functionParameterClause: node.signature.parameterClause, forSubscript: false),
       asyncSpecifierLoc: self.generateSourceLoc(node.signature.effectSpecifiers?.asyncSpecifier),
       throwsSpecifierLoc: self.generateSourceLoc(node.signature.effectSpecifiers?.throwsSpecifier),
       thrownType: self.generate(type: node.signature.effectSpecifiers?.thrownError),
@@ -523,7 +523,7 @@ extension ASTGenVisitor {
       failabilityMarkLoc: self.generateSourceLoc(node.optionalMark),
       isIUO: node.optionalMark?.rawTokenKind == .exclamationMark,
       genericParamList: self.generate(genericParameterClause: node.genericParameterClause),
-      parameterList: self.generate(functionParameterClause: node.signature.parameterClause),
+      parameterList: self.generate(functionParameterClause: node.signature.parameterClause, forSubscript: false),
       asyncSpecifierLoc: self.generateSourceLoc(node.signature.effectSpecifiers?.asyncSpecifier),
       throwsSpecifierLoc: self.generateSourceLoc(node.signature.effectSpecifiers?.throwsSpecifier),
       thrownType: self.generate(type: node.signature.effectSpecifiers?.thrownError),

lib/ASTGen/Sources/ASTGen/ParameterClause.swift

@@ -61,22 +61,41 @@ extension EnumCaseParameterSyntax: ValueParameterSyntax {
 }
 
 extension ASTGenVisitor {
-  func generate(functionParameter node: FunctionParameterSyntax) -> BridgedParamDecl {
-    self.makeParamDecl(node)
+  func generate(functionParameter node: FunctionParameterSyntax, forSubscript: Bool) -> BridgedParamDecl {
+    // For non-subscripts, the argument name is defaulted to the parameter name.
+    self.makeParamDecl(node, argNameByDefault: !forSubscript)
   }
 
   func generate(enumCaseParameter node: EnumCaseParameterSyntax) -> BridgedParamDecl {
-    self.makeParamDecl(node)
+    self.makeParamDecl(node, argNameByDefault: true)
   }
 
-  private func makeParamDecl(_ node: some ValueParameterSyntax) -> BridgedParamDecl {
+  /// Generate a ParamDecl. If `argNameByDefault` is true, then the parameter's
+  /// argument label is inferred from the first name if no second name is present.
+  private func makeParamDecl(_ node: some ValueParameterSyntax, argNameByDefault: Bool) -> BridgedParamDecl {
     // FIXME: This location should be derived from the type repr.
     let specifierLoc: BridgedSourceLoc = nil
 
-    let (firstName, firstNameLoc) =
-      self.generateIdentifierAndSourceLoc(node.optionalFirstName)
-    let (secondName, secondNameLoc) =
-      self.generateIdentifierAndSourceLoc(node.secondName)
+    let paramName: BridgedIdentifier
+    let paramNameLoc: BridgedSourceLoc
+    let argName: BridgedIdentifier
+    let argNameLoc: BridgedSourceLoc
+
+    // Map the first name and second name to argument name and parameter name.
+    // If we have both, use them. If we only have one, then use that as the
+    // parameter name, inferring it as the argument name if we're allowed to.
+    switch (node.optionalFirstName, node.secondName) {
+    case let (argNameNode?, paramNameNode?):
+      (argName, argNameLoc) = self.generateIdentifierAndSourceLoc(argNameNode)
+      (paramName, paramNameLoc) = self.generateIdentifierAndSourceLoc(paramNameNode)
+    case let (nameNode?, nil):
+      (paramName, paramNameLoc) = self.generateIdentifierAndSourceLoc(nameNode)
+      (argName, argNameLoc) = argNameByDefault ? (paramName, paramNameLoc) : (nil, nil)
+    case (nil, _?):
+      preconditionFailure("If only one name is present, it should be the first")
+    default:
+      (argName, argNameLoc, paramName, paramNameLoc) = (nil, nil, nil, nil)
+    }
 
     var type = node.optionalType.map(generate(type:))
     if let ellipsis = node.ellipsis, let base = type {
@@ -92,10 +111,10 @@ extension ASTGenVisitor {
       self.ctx,
       declContext: self.declContext,
       specifierLoc: specifierLoc,
-      firstName: firstName,
-      firstNameLoc: firstNameLoc,
-      secondName: secondName,
-      secondNameLoc: secondNameLoc,
+      argName: argName,
+      argNameLoc: argNameLoc,
+      paramName: paramName,
+      paramNameLoc: paramNameLoc,
       type: type.asNullable,
       defaultValue: self.generate(expr: node.defaultValue?.value)
     )
@@ -105,11 +124,14 @@ extension ASTGenVisitor {
 // MARK: - ParameterList
 
 extension ASTGenVisitor {
-  func generate(functionParameterClause node: FunctionParameterClauseSyntax) -> BridgedParameterList {
+  func generate(
+    functionParameterClause node: FunctionParameterClauseSyntax,
+    forSubscript: Bool
+  ) -> BridgedParameterList {
     BridgedParameterList.createParsed(
       self.ctx,
       leftParenLoc: self.generateSourceLoc(node.leftParen),
-      parameters: self.generate(functionParameterList: node.parameters),
+      parameters: self.generate(functionParameterList: node.parameters, forSubscript: forSubscript),
       rightParenLoc: self.generateSourceLoc(node.rightParen)
     )
   }
@@ -129,10 +151,10 @@ extension ASTGenVisitor {
       self.ctx,
       declContext: self.declContext,
       specifierLoc: nil,
-      firstName: nil,
-      firstNameLoc: nil,
-      secondName: name,
-      secondNameLoc: nameLoc,
+      argName: nil,
+      argNameLoc: nil,
+      paramName: name,
+      paramNameLoc: nameLoc,
       type: nil,
       defaultValue: nil
     )
@@ -147,7 +169,7 @@ extension ASTGenVisitor {
 
 extension ASTGenVisitor {
   @inline(__always)
-  func generate(functionParameterList node: FunctionParameterListSyntax) -> BridgedArrayRef {
-    node.lazy.map(self.generate).bridgedArray(in: self)
+  func generate(functionParameterList node: FunctionParameterListSyntax, forSubscript: Bool) -> BridgedArrayRef {
+    node.lazy.map({ self.generate(functionParameter: $0, forSubscript: forSubscript) }).bridgedArray(in: self)
   }
 }

test/ASTGen/verify-parse.swift

@@ -107,6 +107,8 @@ func test11() {
   var binding1 = 0, binding2 = ""
 }
 
+func test12(_: Int) {}
+
 func testVars() {
   var a = 0
   var b: Int = 0