Merge pull request #39535 from guitard0g/tparam-not-used-in-fsig

swift-ci · web-flow · commit b69eac963095 · 2021-10-06T18:06:29.000-07:00
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -4019,7 +4019,7 @@ namespace {
       }
       return Impl.importFunctionParameterList(
           dc, decl, nonSelfParams, decl->isVariadic(), allowNSUIntegerAsInt,
-          argNames, genericParams);
+          argNames, genericParams, /*resultType=*/nullptr);
     }
 
     Decl *importGlobalAsInitializer(const clang::FunctionDecl *decl,
@@ -4079,23 +4079,15 @@ namespace {
       DeclName name = accessorInfo ? DeclName() : importedName.getDeclName();
       auto selfIdx = importedName.getSelfIndex();
 
-      auto underlyingTypeIsSame = [](const clang::Type *a,
-                                     clang::TemplateTypeParmDecl *b) {
-        while (a->isPointerType() || a->isReferenceType())
-          a = a->getPointeeType().getTypePtr();
-        return a == b->getTypeForDecl();
-      };
-
       auto templateParamTypeUsedInSignature =
-          [underlyingTypeIsSame,
-           decl](clang::TemplateTypeParmDecl *type) -> bool {
+          [decl](clang::TemplateTypeParmDecl *type) -> bool {
         // TODO(SR-13809): we will want to update this to handle dependent
         // types when those are supported.
-        if (underlyingTypeIsSame(decl->getReturnType().getTypePtr(), type))
+        if (hasSameUnderlyingType(decl->getReturnType().getTypePtr(), type))
           return true;
 
         for (unsigned i : range(0, decl->getNumParams())) {
-          if (underlyingTypeIsSame(
+          if (hasSameUnderlyingType(
                   decl->getParamDecl(i)->getType().getTypePtr(), type))
             return true;
         }
@@ -4241,6 +4233,11 @@ namespace {
       if (!bodyParams)
         return nullptr;
 
+      if (name && name.getArgumentNames().size() != bodyParams->size()) {
+        // We synthesized additional parameters so rebuild the DeclName.
+        name = DeclName(Impl.SwiftContext, name.getBaseName(), bodyParams);
+      }
+
       auto loc = Impl.importSourceLoc(decl->getLocation());
 
       ClangNode clangNode = decl;
@@ -6619,7 +6616,7 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer(
   } else {
     parameterList = Impl.importFunctionParameterList(
         dc, decl, {decl->param_begin(), decl->param_end()}, decl->isVariadic(),
-        allowNSUIntegerAsInt, argNames, /*genericParams=*/{});
+        allowNSUIntegerAsInt, argNames, /*genericParams=*/{}, /*resultType=*/nullptr);
   }
   if (!parameterList)
     return nullptr;
@@ -8534,6 +8531,13 @@ bool importer::isImportedAsStatic(const clang::OverloadedOperatorKind op) {
   }
 }
 
+bool importer::hasSameUnderlyingType(const clang::Type *a,
+                                     const clang::TemplateTypeParmDecl *b) {
+  while (a->isPointerType() || a->isReferenceType())
+    a = a->getPointeeType().getTypePtr();
+  return a == b->getTypeForDecl();
+}
+
 unsigned ClangImporter::Implementation::getClangSwiftAttrSourceBuffer(
     StringRef attributeText) {
   auto known = ClangSwiftAttrSourceBuffers.find(attributeText);
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -1844,14 +1844,14 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(
       return {Type(), false};
   }
 
+  Type swiftResultTy = importedType.getType();
   ArrayRef<Identifier> argNames = name.getArgumentNames();
   parameterList = importFunctionParameterList(dc, clangDecl, params, isVariadic,
                                               allowNSUIntegerAsInt, argNames,
-                                              genericParams);
+                                              genericParams, swiftResultTy);
   if (!parameterList)
     return {Type(), false};
 
-  Type swiftResultTy = importedType.getType();
   if (clangDecl->isNoReturn())
     swiftResultTy = SwiftContext.getNeverType();
 
@@ -1862,7 +1862,7 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
     DeclContext *dc, const clang::FunctionDecl *clangDecl,
     ArrayRef<const clang::ParmVarDecl *> params, bool isVariadic,
     bool allowNSUIntegerAsInt, ArrayRef<Identifier> argNames,
-    ArrayRef<GenericTypeParamDecl *> genericParams) {
+    ArrayRef<GenericTypeParamDecl *> genericParams, Type resultType) {
   // Import the parameters.
   SmallVector<ParamDecl *, 4> parameters;
   unsigned index = 0;
@@ -1980,6 +1980,50 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
     ++index;
   }
 
+  auto genericParamTypeUsedInSignature =
+      [params, resultType](GenericTypeParamDecl *genericParam,
+                           bool shouldCheckResultType) -> bool {
+    auto paramDecl = genericParam->getClangDecl();
+    auto templateTypeParam = cast<clang::TemplateTypeParmDecl>(paramDecl);
+    // TODO(SR-13809): This won't work when we support importing dependent types.
+    // We'll have to change this logic to traverse the type tree of the imported
+    // Swift type (basically whatever ends up in the parameters variable).
+    // Check if genericParam's corresponding clang template type is used by
+    // the clang function's parameters.
+    for (auto param : params) {
+      if (hasSameUnderlyingType(param->getType().getTypePtr(),
+                                templateTypeParam)) {
+        return true;
+      }
+    }
+
+    // Check if genericParam is used as a type parameter in the result type.
+    return shouldCheckResultType &&
+        resultType.findIf([genericParam](Type typePart) -> bool {
+          return typePart->isEqual(genericParam->getDeclaredInterfaceType());
+        });
+  };
+  
+  // Make sure all generic parameters are accounted for in the function signature.
+  for (auto genericParam : genericParams) {
+    bool shouldCheckResultType = resultType && resultType->hasTypeParameter();
+    if (genericParamTypeUsedInSignature(genericParam, shouldCheckResultType))
+      continue;
+    
+    // If this generic parameter is not used in the function signature,
+    // add a new parameter that accepts a metatype corresponding to that
+    // generic parameter.
+    Identifier name = genericParam->getName();
+    auto param = new (SwiftContext)
+        ParamDecl(SourceLoc(), SourceLoc(), name, SourceLoc(),
+                  name, dc);
+    auto metatype =
+      cast<MetatypeType>(genericParam->getInterfaceType().getPointer());
+    param->setInterfaceType(metatype);
+    param->setSpecifier(ParamSpecifier::Default);
+    parameters.push_back(param);
+  }
+
   // Append an additional argument to represent varargs.
   if (isVariadic) {
     auto paramTy =
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
@@ -1223,7 +1223,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
       DeclContext *dc, const clang::FunctionDecl *clangDecl,
       ArrayRef<const clang::ParmVarDecl *> params, bool isVariadic,
       bool allowNSUIntegerAsInt, ArrayRef<Identifier> argNames,
-      ArrayRef<GenericTypeParamDecl *> genericParams);
+      ArrayRef<GenericTypeParamDecl *> genericParams, Type resultType);
 
   ImportedType importPropertyType(const clang::ObjCPropertyDecl *clangDecl,
                                   bool isFromSystemModule);
@@ -1560,6 +1560,12 @@ bool isSpecialUIKitStructZeroProperty(const clang::NamedDecl *decl);
 /// even if imported as a non-static member function.
 bool isImportedAsStatic(clang::OverloadedOperatorKind op);
 
+/// \returns true if \p a has the same underlying type as \p b after removing
+/// any pointer/reference specifiers. Note that this does not currently look through
+/// nested types other than pointers or references.
+bool hasSameUnderlyingType(const clang::Type *a,
+                           const clang::TemplateTypeParmDecl *b);
+
 /// Add command-line arguments for a normal import of Clang code.
 void getNormalInvocationArguments(std::vector<std::string> &invocationArgStrs,
                                   ASTContext &ctx);
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -109,16 +109,18 @@ Solution::computeSubstitutions(GenericSignature sig,
                               lookupConformanceFn);
 }
 
-static ConcreteDeclRef generateDeclRefForSpecializedCXXFunctionTemplate(
-    ASTContext &ctx, AbstractFunctionDecl *oldDecl, SubstitutionMap subst,
-    clang::FunctionDecl *specialized) {
+// Derive a concrete function type for fdecl by substituting the generic args
+// and use that to derive the corresponding function type and parameter list.
+static std::pair<FunctionType *, ParameterList *>
+substituteFunctionTypeAndParamList(ASTContext &ctx, AbstractFunctionDecl *fdecl,
+                                   SubstitutionMap subst) {
   FunctionType *newFnType = nullptr;
   // Create a new ParameterList with the substituted type.
   if (auto oldFnType = dyn_cast<GenericFunctionType>(
-          oldDecl->getInterfaceType().getPointer())) {
+          fdecl->getInterfaceType().getPointer())) {
     newFnType = oldFnType->substGenericArgs(subst);
   } else {
-    newFnType = cast<FunctionType>(oldDecl->getInterfaceType().getPointer());
+    newFnType = cast<FunctionType>(fdecl->getInterfaceType().getPointer());
   }
   // The constructor type is a function type as follows:
   //   (CType.Type) -> (Generic) -> CType
@@ -127,22 +129,49 @@ static ConcreteDeclRef generateDeclRefForSpecializedCXXFunctionTemplate(
   // In either case, we only want the result of that function type because that
   // is the function type with the generic params that need to be substituted:
   //   (Generic) -> CType
-  if (isa<ConstructorDecl>(oldDecl) || oldDecl->isInstanceMember() ||
-      oldDecl->isStatic())
+  if (isa<ConstructorDecl>(fdecl) || fdecl->isInstanceMember() ||
+      fdecl->isStatic())
     newFnType = cast<FunctionType>(newFnType->getResult().getPointer());
   SmallVector<ParamDecl *, 4> newParams;
   unsigned i = 0;
+
   for (auto paramTy : newFnType->getParams()) {
-    auto *oldParamDecl = oldDecl->getParameters()->get(i);
+    auto *oldParamDecl = fdecl->getParameters()->get(i);
     auto *newParamDecl =
-        ParamDecl::cloneWithoutType(oldDecl->getASTContext(), oldParamDecl);
+        ParamDecl::cloneWithoutType(fdecl->getASTContext(), oldParamDecl);
     newParamDecl->setInterfaceType(paramTy.getParameterType());
     newParams.push_back(newParamDecl);
     (void)++i;
   }
   auto *newParamList =
       ParameterList::create(ctx, SourceLoc(), newParams, SourceLoc());
 
+  return {newFnType, newParamList};
+}
+
+static ValueDecl *generateSpecializedCXXFunctionTemplate(
+    ASTContext &ctx, AbstractFunctionDecl *oldDecl, SubstitutionMap subst,
+    clang::FunctionDecl *specialized) {
+  auto newFnTypeAndParams = substituteFunctionTypeAndParamList(ctx, oldDecl, subst);
+  auto newFnType = newFnTypeAndParams.first;
+  auto paramList = newFnTypeAndParams.second;
+
+  SmallVector<ParamDecl *, 4> newParamsWithoutMetatypes;
+  for (auto param : *paramList ) {
+    if (isa<FuncDecl>(oldDecl) &&
+        isa<MetatypeType>(param->getType().getPointer())) {
+      // Metatype parameters are added synthetically to account for template
+      // params that don't make it to the function signature. These shouldn't
+      // exist in the resulting specialized FuncDecl. Note that this doesn't
+      // affect constructors because all template params for a constructor
+      // must be in the function signature by design.
+      continue;
+    }
+    newParamsWithoutMetatypes.push_back(param);
+  }
+  auto *newParamList =
+      ParameterList::create(ctx, SourceLoc(), newParamsWithoutMetatypes, SourceLoc());
+
   if (isa<ConstructorDecl>(oldDecl)) {
     DeclName ctorName(ctx, DeclBaseName::createConstructor(), newParamList);
     auto newCtorDecl = ConstructorDecl::createImported(
@@ -152,7 +181,7 @@ static ConcreteDeclRef generateDeclRefForSpecializedCXXFunctionTemplate(
         /*throws=*/false, /*throwsLoc=*/SourceLoc(), 
         newParamList, /*genericParams=*/nullptr,
         oldDecl->getDeclContext());
-    return ConcreteDeclRef(newCtorDecl);
+    return newCtorDecl;
   }
 
   // Generate a name for the specialized function.
@@ -176,7 +205,48 @@ static ConcreteDeclRef generateDeclRefForSpecializedCXXFunctionTemplate(
     newFnDecl->setImportAsStaticMember();
   }
   newFnDecl->setSelfAccessKind(cast<FuncDecl>(oldDecl)->getSelfAccessKind());
-  return ConcreteDeclRef(newFnDecl);
+  return newFnDecl;
+}
+
+// Synthesizes the body of a thunk that takes extra metatype arguments and
+// skips over them to forward them along to the FuncDecl contained by context.
+// This is used when importing a C++ templated function where the template params
+// are not used in the function signature. We supply the type params as explicit
+// metatype arguments to aid in typechecking, but they shouldn't be forwarded to
+// the corresponding C++ function.
+static std::pair<BraceStmt *, bool>
+synthesizeForwardingThunkBody(AbstractFunctionDecl *afd, void *context) {
+  ASTContext &ctx = afd->getASTContext();
+
+  auto thunkDecl = cast<FuncDecl>(afd);
+  auto specializedFuncDecl = static_cast<FuncDecl *>(context);
+
+  SmallVector<Argument, 8> forwardingParams;
+  for (auto param : *thunkDecl->getParameters()) {
+    if (isa<MetatypeType>(param->getType().getPointer())) {
+      continue;
+    }
+    auto paramRefExpr = new (ctx) DeclRefExpr(param, DeclNameLoc(),
+                                             /*Implicit=*/true);
+    paramRefExpr->setType(param->getType());
+    forwardingParams.push_back(Argument(SourceLoc(), Identifier(), paramRefExpr));
+  }
+
+  auto *specializedFuncDeclRef = new (ctx) DeclRefExpr(ConcreteDeclRef(specializedFuncDecl),
+                                                DeclNameLoc(), true);
+  specializedFuncDeclRef->setType(specializedFuncDecl->getInterfaceType());
+
+  auto argList = ArgumentList::createImplicit(ctx, forwardingParams);
+  auto *specializedFuncCallExpr = CallExpr::createImplicit(ctx, specializedFuncDeclRef, argList);
+  specializedFuncCallExpr->setType(thunkDecl->getResultInterfaceType());
+  specializedFuncCallExpr->setThrows(false);
+
+  auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), specializedFuncCallExpr,
+                                        /*implicit=*/true);
+
+  auto body = BraceStmt::create(ctx, SourceLoc(), {returnStmt}, SourceLoc(),
+                               /*implicit=*/true);
+  return {body, /*isTypeChecked=*/true};
 }
 
 ConcreteDeclRef
@@ -215,13 +285,35 @@ Solution::resolveConcreteDeclRef(ValueDecl *decl,
                 const_cast<clang::FunctionTemplateDecl *>(
                     cast<clang::FunctionTemplateDecl>(decl->getClangDecl())),
                 subst);
-    return generateDeclRefForSpecializedCXXFunctionTemplate(
+    auto newDecl = generateSpecializedCXXFunctionTemplate(
         decl->getASTContext(), cast<AbstractFunctionDecl>(decl), subst, newFn);
+    if (auto fn = dyn_cast<FuncDecl>(decl)) {
+      if (newFn->getNumParams() != fn->getParameters()->size()) {
+        // We added additional metatype parameters to aid template
+        // specialization, which are no longer now that we've specialized
+        // this function. Create a thunk that only forwards the original
+        // parameters along to the clang function.
+        auto thunkTypeAndParamList = substituteFunctionTypeAndParamList(decl->getASTContext(),
+                                                                        fn, subst);
+        auto thunk = FuncDecl::createImplicit(
+                 fn->getASTContext(), fn->getStaticSpelling(), fn->getName(),
+                 fn->getNameLoc(), fn->hasAsync(), fn->hasThrows(),
+                 /*genericParams=*/nullptr, thunkTypeAndParamList.second,
+                 thunkTypeAndParamList.first->getResult(), fn->getDeclContext());
+        thunk->copyFormalAccessFrom(fn);
+        thunk->setBodySynthesizer(synthesizeForwardingThunkBody, cast<FuncDecl>(newDecl));
+        thunk->setSelfAccessKind(fn->getSelfAccessKind());
+
+        return ConcreteDeclRef(thunk);
+      }
+    }
+    return ConcreteDeclRef(newDecl);
   }
 
   return ConcreteDeclRef(decl, subst);
 }
 
+
 ConstraintLocator *Solution::getCalleeLocator(ConstraintLocator *locator,
                                               bool lookThroughApply) const {
   auto &cs = getConstraintSystem();
diff --git a/test/Interop/Cxx/templates/Inputs/module.modulemap b/test/Interop/Cxx/templates/Inputs/module.modulemap
@@ -122,3 +122,8 @@ module DefaultedTemplateTypeParameter {
   header "defaulted-template-type-parameter.h"
   requires cplusplus
 }
+
+module TemplateTypeParameterNotInSignature {
+  header "template-type-parameter-not-in-signature.h"
+  requires cplusplus
+}
diff --git a/test/Interop/Cxx/templates/Inputs/template-type-parameter-not-in-signature.h b/test/Interop/Cxx/templates/Inputs/template-type-parameter-not-in-signature.h
@@ -0,0 +1,31 @@
+#ifndef TEST_INTEROP_CXX_TEMPLATES_INPUTS_TEMPLATE_TYPE_PARAMETER_NOT_IN_SIGNATURE_H
+#define TEST_INTEROP_CXX_TEMPLATES_INPUTS_TEMPLATE_TYPE_PARAMETER_NOT_IN_SIGNATURE_H
+
+template <typename T>
+void templateTypeParamNotUsedInSignature() {}
+
+template <typename T, typename U>
+void multiTemplateTypeParamNotUsedInSignature() {}
+
+template <typename T, typename U>
+U multiTemplateTypeParamOneUsedInSignature(U u) { return u; }
+
+template <typename T, typename U>
+void multiTemplateTypeParamNotUsedInSignatureWithUnrelatedParams(int x, long y) {}
+
+template <typename T>
+T templateTypeParamUsedInReturnType(int x) { return x; }
+
+template <typename T>
+T templateTypeParamUsedInReferenceParam(T &t) { return t; }
+
+template <typename T, typename U>
+void templateTypeParamNotUsedInSignatureWithVarargs(...) {}
+
+template <typename T, typename U, typename V>
+void templateTypeParamNotUsedInSignatureWithVarargsAndUnrelatedParam(int x, ...) {}
+
+template <typename T, int N>
+void templateTypeParamNotUsedInSignatureWithNonTypeParam() {}
+
+#endif // TEST_INTEROP_CXX_TEMPLATES_INPUTS_TEMPLATE_TYPE_PARAMETER_NOT_IN_SIGNATURE_H
diff --git a/test/Interop/Cxx/templates/function-template-module-interface.swift b/test/Interop/Cxx/templates/function-template-module-interface.swift
@@ -5,7 +5,7 @@
 // CHECK: func passThrough<T>(_ value: T) -> T
 // CHECK: func passThroughConst<T>(_ value: T) -> T
 // CHECK: func templateParameterReturnType<R, T, U>(_ a: T, _ b: U) -> R
-// CHECK: func cannotInferTemplate<T>()
+// CHECK: func cannotInferTemplate<T>(T: T.Type)
 
 // CHECK: struct HasVariadicMemeber {
 // CHECK:   @available(*, unavailable, message: "Variadic function is unavailable")
diff --git a/test/Interop/Cxx/templates/template-type-parameter-not-in-signature-module-interface.swift b/test/Interop/Cxx/templates/template-type-parameter-not-in-signature-module-interface.swift
diff --git a/test/Interop/Cxx/templates/template-type-parameter-not-in-signature.swift b/test/Interop/Cxx/templates/template-type-parameter-not-in-signature.swift

Original file line number	Diff line number	Diff line change
`@@ -122,3 +122,8 @@ module DefaultedTemplateTypeParameter {`
`122`	`122`	`header "defaulted-template-type-parameter.h"`
`123`	`123`	`requires cplusplus`
`124`	`124`	`}`
	`125`	`+`
	`126`	`+module TemplateTypeParameterNotInSignature {`
	`127`	`+ header "template-type-parameter-not-in-signature.h"`
	`128`	`+ requires cplusplus`
	`129`	`+}`