Merge pull request swiftlang#12359 from xedin/ast-func-mangling-changes

Changes to AST mangling to always respect parens in function signature

Author: xedin

docs/ABI/Mangling.rst

@@ -343,7 +343,7 @@ Types
 
   function-signature ::= params-type params-type throws? // results and parameters
 
-  params-type := type                        // tuple in case of multiple parameters
+  params-type := type                        // tuple in case of multiple parameters or a single parameter with a single tuple type
   params-type := empty-list                  // shortcut for no parameters
 
   throws ::= 'K'                             // 'throws' annotation on function types

include/swift/AST/ASTMangler.h

@@ -187,13 +187,16 @@ class ASTMangler : public Mangler {
 
   void appendAnyGenericType(const GenericTypeDecl *decl);
 
-  void appendFunctionType(AnyFunctionType *fn, bool forceSingleParam);
+  void appendFunctionType(AnyFunctionType *fn);
 
-  void appendFunctionSignature(AnyFunctionType *fn, bool forceSingleParam);
+  void appendFunctionSignature(AnyFunctionType *fn);
 
-  void appendParams(Type ParamsTy, bool forceSingleParam);
+  void appendFunctionInputType(ArrayRef<AnyFunctionType::Param> params);
+  void appendFunctionResultType(Type resultType);
 
   void appendTypeList(Type listTy);
+  void appendTypeListElement(Identifier name, Type elementType,
+                             ParameterTypeFlags flags);
 
   /// Append a generic signature to the mangling.
   ///

include/swift/AST/Types.h

@@ -2347,7 +2347,8 @@ class AnyFunctionType : public TypeBase {
     /// FIXME(Remove InOutType): This is mostly for copying between param
     /// types and should go away.
     Type getPlainType() const { return Ty; }
-    
+
+    bool hasLabel() const { return !Label.empty(); }
     Identifier getLabel() const { return Label; }
 
     ParameterTypeFlags getParameterFlags() const { return Flags; }

lib/AST/ASTMangler.cpp

@@ -900,7 +900,7 @@ void ASTMangler::appendType(Type type) {
 
     case TypeKind::GenericFunction: {
       auto genFunc = cast<GenericFunctionType>(tybase);
-      appendFunctionType(genFunc, /*forceSingleParam*/ false);
+      appendFunctionType(genFunc);
       appendGenericSignature(genFunc->getGenericSignature());
       appendOperator("u");
       return;
@@ -943,7 +943,7 @@ void ASTMangler::appendType(Type type) {
     }
 
     case TypeKind::Function:
-      appendFunctionType(cast<FunctionType>(tybase), /*forceSingleParam*/ false);
+      appendFunctionType(cast<FunctionType>(tybase));
       return;
 
     case TypeKind::SILBox: {
@@ -1438,12 +1438,11 @@ void ASTMangler::appendAnyGenericType(const GenericTypeDecl *decl) {
   addSubstitution(key.getPointer());
 }
 
-void ASTMangler::appendFunctionType(AnyFunctionType *fn,
-                                    bool forceSingleParam) {
+void ASTMangler::appendFunctionType(AnyFunctionType *fn) {
   assert((DWARFMangling || fn->isCanonical()) &&
          "expecting canonical types when not mangling for the debugger");
 
-  appendFunctionSignature(fn, forceSingleParam);
+  appendFunctionSignature(fn);
 
   // Note that we do not currently use thin representations in the AST
   // for the types of function decls.  This may need to change at some
@@ -1470,48 +1469,66 @@ void ASTMangler::appendFunctionType(AnyFunctionType *fn,
   }
 }
 
-void ASTMangler::appendFunctionSignature(AnyFunctionType *fn,
-                                         bool forceSingleParam) {
-  appendParams(fn->getResult(), /*forceSingleParam*/ false);
-  appendParams(fn->getInput(), forceSingleParam);
+void ASTMangler::appendFunctionSignature(AnyFunctionType *fn) {
+  appendFunctionResultType(fn->getResult());
+  appendFunctionInputType(fn->getParams());
   if (fn->throws())
     appendOperator("K");
 }
 
-void ASTMangler::appendParams(Type ParamsTy, bool forceSingleParam) {
-  if (TupleType *Tuple = ParamsTy->getAs<TupleType>()) {
-    if (Tuple->getNumElements() == 0) {
-      if (forceSingleParam) {
-        // A tuple containing a single empty tuple.
-        appendOperator("y");
-        appendOperator("t");
-        appendListSeparator();
-        appendOperator("t");
-      } else {
-        appendOperator("y");
-      }
-      return;
+void ASTMangler::appendFunctionInputType(
+    ArrayRef<AnyFunctionType::Param> params) {
+  auto getParamType = [](const AnyFunctionType::Param &param) -> Type {
+    auto type = param.getType();
+
+    // FIXME: Change mangling for variadic parameters so
+    //        the enclosing array type is not required.
+    if (param.isVariadic()) {
+      auto *arrayDecl = type->getASTContext().getArrayDecl();
+      assert(arrayDecl);
+      return BoundGenericType::get(arrayDecl, Type(), {type});
     }
-    if (forceSingleParam && Tuple->getNumElements() > 1) {
-      auto flags = ParameterTypeFlags();
-      if (ParenType *Paren = dyn_cast<ParenType>(ParamsTy.getPointer())) {
-        ParamsTy = Paren->getUnderlyingType();
-        flags = Paren->getParameterFlags();
-      }
 
-      appendType(ParamsTy);
-      if (flags.isShared())
-        appendOperator("h");
-      appendListSeparator();
-      appendOperator("t");
-      return;
+    return type;
+  };
+
+  switch (params.size()) {
+  case 0:
+    appendOperator("y");
+    break;
+
+  case 1: {
+    const auto &param = params.front();
+    auto type = param.getType();
+
+    // If this is just a single parenthesized type,
+    // to save space in the mangled name, let's encode
+    // it as a single type dropping sugar.
+    if (!param.hasLabel() && !param.isVariadic() &&
+        !isa<TupleType>(type.getPointer())) {
+      appendType(type);
+      break;
     }
+
+    // If this is a tuple type with a single labeled element
+    // let's handle it as a general case.
+    LLVM_FALLTHROUGH;
   }
 
-  if (ParenType *Paren = dyn_cast<ParenType>(ParamsTy.getPointer()))
-    ParamsTy = Paren->getUnderlyingType();
+  default:
+    bool isFirstParam = true;
+    for (auto &param : params) {
+      appendTypeListElement(param.getLabel(), getParamType(param),
+                            param.getParameterFlags());
+      appendListSeparator(isFirstParam);
+    }
+    appendOperator("t");
+    break;
+  }
+}
 
-  appendType(ParamsTy);
+void ASTMangler::appendFunctionResultType(Type resultType) {
+  return resultType->isVoid() ? appendOperator("y") : appendType(resultType);
 }
 
 void ASTMangler::appendTypeList(Type listTy) {
@@ -1520,15 +1537,8 @@ void ASTMangler::appendTypeList(Type listTy) {
       return appendOperator("y");
     bool firstField = true;
     for (auto &field : tuple->getElements()) {
-      appendType(field.getType()->getInOutObjectType());
-      if (field.isInOut())
-        appendOperator("z");
-      if (field.getParameterFlags().isShared())
-        appendOperator("h");
-      if (field.hasName())
-        appendIdentifier(field.getName().str());
-      if (field.isVararg())
-        appendOperator("d");
+      appendTypeListElement(field.getName(), field.getType(),
+                            field.getParameterFlags());
       appendListSeparator(firstField);
     }
   } else {
@@ -1537,6 +1547,19 @@ void ASTMangler::appendTypeList(Type listTy) {
   }
 }
 
+void ASTMangler::appendTypeListElement(Identifier name, Type elementType,
+                                       ParameterTypeFlags flags) {
+  appendType(elementType->getInOutObjectType());
+  if (flags.isInOut())
+    appendOperator("z");
+  if (flags.isShared())
+    appendOperator("h");
+  if (!name.empty())
+    appendIdentifier(name.str());
+  if (flags.isVariadic())
+    appendOperator("d");
+}
+
 bool ASTMangler::appendGenericSignature(const GenericSignature *sig,
                                         GenericSignature *contextSig) {
   auto canSig = sig->getCanonicalSignature();
@@ -1828,23 +1851,10 @@ void ASTMangler::appendDeclType(const ValueDecl *decl, bool isFunctionMangling)
   auto type = getDeclTypeForMangling(decl, genericSig, parentGenericSig);
 
   if (AnyFunctionType *FuncTy = type->getAs<AnyFunctionType>()) {
-
-    const ParameterList *Params = nullptr;
-    if (const auto *FDecl = dyn_cast<AbstractFunctionDecl>(decl)) {
-      unsigned PListIdx = isMethodDecl(decl) ? 1 : 0;
-      if (PListIdx < FDecl->getNumParameterLists()) {
-        Params = FDecl->getParameterList(PListIdx);
-      }
-    } else if (const auto *SDecl = dyn_cast<SubscriptDecl>(decl)) {
-        Params = SDecl->getIndices();
-    }
-    bool forceSingleParam = Params && (Params->size() == 1);
-          
-
     if (isFunctionMangling) {
-      appendFunctionSignature(FuncTy, forceSingleParam);
+      appendFunctionSignature(FuncTy);
     } else {
-      appendFunctionType(FuncTy, forceSingleParam);
+      appendFunctionType(FuncTy);
     }
   } else {
     appendType(type);

test/SILGen/arguments_as_tuple_overloads.swift

@@ -39,10 +39,30 @@ public func test(_ a: Int, _ b: Int) {
 public func test(_ t: (Int, Int)) {
 }
 
+// CHECK: sil @_T04test0A7NoLabelySi_Sit_tF :
+public func testNoLabel(_: (Int, Int)) {
+}
+
+// CHECK: sil @_T04test0A5FnArgyySi_SitcF :
+public func testFnArg(_: (Int, Int) -> Void) {
+}
+
+// CHECK: sil @_T04test0A5FnArgyySi_Sit_tcF :
+public func testFnArg(_: ((Int, Int)) -> Void) {
+}
+
 // CHECK: sil @_T04test3fooyyt_tF :
 public func foo(_: ()) {
 }
 
 // CHECK: sil @_T04test3fooyyF :
 public func foo() {
 }
+
+public func baz() {
+  // CHECK: function_ref @_T04test3bazyyFySi_Sit_tcfU_ :
+  let _: ((Int, Int)) -> Void = { x in }
+
+  // CHECK: function_ref @_T04test3bazyyFySi_SitcfU0_ :
+  let _: (Int, Int) -> Void = { x, y in }
+}