Merge pull request #41394 from zoecarver/revert-revert-cleanup-and-fixes-for-function-templates

Author: zoecarver

lib/ClangImporter/ClangImporter.cpp

@@ -4672,8 +4672,7 @@ clang::FunctionDecl *ClangImporter::instantiateCXXFunctionTemplate(
     ctx.Diags.diagnose(SourceLoc(),
                        diag::unable_to_convert_generic_swift_types.ID,
                        {func->getName(), StringRef(failedTypesStr)});
-    // Return a valid FunctionDecl but, we'll never use it.
-    return func->getAsFunction();
+    return nullptr;
   }
 
   // Instanciate a specialization of this template using the substitution map.

lib/ClangImporter/ImportType.cpp

@@ -1945,7 +1945,7 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
     Type swiftParamTy;
     bool isParamTypeImplicitlyUnwrapped = false;
     bool isInOut = false;
-    if ((isa<clang::ReferenceType>(paramTy) || isa<clang::PointerType>(paramTy)) &&
+    if (isa<clang::PointerType>(paramTy) &&
         isa<clang::TemplateTypeParmType>(paramTy->getPointeeType())) {
       auto pointeeType = paramTy->getPointeeType();
       auto templateParamType = cast<clang::TemplateTypeParmType>(pointeeType);
@@ -1957,6 +1957,13 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
       swiftParamTy = genericType->wrapInPointer(pointerKind);
       if (!swiftParamTy)
         return nullptr;
+    } else if (isa<clang::ReferenceType>(paramTy) &&
+               isa<clang::TemplateTypeParmType>(paramTy->getPointeeType())) {
+      auto templateParamType =
+          cast<clang::TemplateTypeParmType>(paramTy->getPointeeType());
+      swiftParamTy =
+          findGenericTypeInGenericDecls(templateParamType, genericParams);
+      isInOut = true;
     } else if (auto *templateParamType =
                    dyn_cast<clang::TemplateTypeParmType>(paramTy)) {
       swiftParamTy =

lib/Sema/CSApply.cpp

@@ -109,6 +109,76 @@ Solution::computeSubstitutions(GenericSignature sig,
                               lookupConformanceFn);
 }
 
+// On Windows and 32-bit platforms we need to force "Int" to actually be
+// re-imported as "Int." This is needed because otherwise, we cannot round-trip
+// "Int" and "UInt". For example, on Windows, "Int" will be imported into C++ as
+// "long long" and then back into Swift as "Int64" not "Int." 
+static ValueDecl *rewriteIntegerTypes(SubstitutionMap subst, ValueDecl *oldDecl,
+                                      AbstractFunctionDecl *newDecl) {
+  auto originalFnSubst = cast<AbstractFunctionDecl>(oldDecl)
+                             ->getInterfaceType()
+                             ->getAs<GenericFunctionType>()
+                             ->substGenericArgs(subst);
+  // The constructor type is a function type as follows:
+  //   (CType.Type) -> (Generic) -> CType
+  // And a method's function type is as follows:
+  //   (inout CType) -> (Generic) -> Void
+  // 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())
+    originalFnSubst = cast<FunctionType>(originalFnSubst->getResult().getPointer());
+
+  SmallVector<ParamDecl *, 4> fixedParameters;
+  unsigned parameterIndex = 0;
+  for (auto *newFnParam : *newDecl->getParameters()) {
+    // If the user substituted this param with an (U)Int, use (U)Int.
+    auto substParamType =
+        originalFnSubst->getParams()[parameterIndex].getParameterType();
+    if (substParamType->isEqual(newDecl->getASTContext().getIntType()) ||
+        substParamType->isEqual(newDecl->getASTContext().getUIntType())) {
+      auto intParam =
+          ParamDecl::cloneWithoutType(newDecl->getASTContext(), newFnParam);
+      intParam->setInterfaceType(substParamType);
+      fixedParameters.push_back(intParam);
+    } else {
+      fixedParameters.push_back(newFnParam);
+    }
+    parameterIndex++;
+  }
+
+  auto fixedParams =
+      ParameterList::create(newDecl->getASTContext(), fixedParameters);
+  newDecl->setParameters(fixedParams);
+
+  // Now fix the result type:
+  if (originalFnSubst->getResult()->isEqual(
+          newDecl->getASTContext().getIntType()) ||
+      originalFnSubst->getResult()->isEqual(
+          newDecl->getASTContext().getUIntType())) {
+    // Constructors don't have a result.
+    if (auto func = dyn_cast<FuncDecl>(newDecl)) {
+      // We have to rebuild the whole function.
+      auto newFnDecl = FuncDecl::createImported(
+          func->getASTContext(), func->getNameLoc(),
+          func->getName(), func->getNameLoc(),
+          func->hasAsync(), func->hasThrows(),
+          fixedParams, originalFnSubst->getResult(),
+          /*genericParams=*/nullptr, func->getDeclContext(), newDecl->getClangDecl());
+      if (func->isStatic()) newFnDecl->setStatic();
+      if (func->isImportAsStaticMember()) newFnDecl->setImportAsStaticMember();
+      if (func->getImportAsMemberStatus().isInstance()) {
+        newFnDecl->setSelfAccessKind(func->getSelfAccessKind());
+        newFnDecl->setSelfIndex(func->getSelfIndex());
+      }
+      return newFnDecl;
+    }
+  }
+
+  return newDecl;
+}
+
 // 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 *>
@@ -149,65 +219,6 @@ substituteFunctionTypeAndParamList(ASTContext &ctx, AbstractFunctionDecl *fdecl,
   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(
-        ctx, specialized, ctorName, oldDecl->getLoc(), 
-        /*failable=*/false, /*failabilityLoc=*/SourceLoc(),
-        /*Async=*/false, /*AsyncLoc=*/SourceLoc(),
-        /*throws=*/false, /*throwsLoc=*/SourceLoc(), 
-        newParamList, /*genericParams=*/nullptr,
-        oldDecl->getDeclContext());
-    return newCtorDecl;
-  }
-
-  // Generate a name for the specialized function.
-  std::string newNameStr;
-  llvm::raw_string_ostream buffer(newNameStr);
-  std::unique_ptr<clang::MangleContext> mangler(
-      specialized->getASTContext().createMangleContext());
-  mangler->mangleName(specialized, buffer);
-  buffer.flush();
-  // Add all parameters as empty parameters.
-  auto newName = DeclName(
-      ctx, DeclName(ctx.getIdentifier(newNameStr)).getBaseName(), newParamList);
-
-  auto newFnDecl = FuncDecl::createImported(
-      ctx, oldDecl->getLoc(), newName, oldDecl->getNameLoc(),
-      /*Async=*/false, oldDecl->hasThrows(), newParamList,
-      newFnType->getResult(), /*GenericParams=*/nullptr,
-      oldDecl->getDeclContext(), specialized);
-  if (oldDecl->isStatic()) {
-    newFnDecl->setStatic();
-    newFnDecl->setImportAsStaticMember();
-  }
-  newFnDecl->setSelfAccessKind(cast<FuncDecl>(oldDecl)->getSelfAccessKind());
-  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
@@ -249,20 +260,32 @@ synthesizeForwardingThunkBody(AbstractFunctionDecl *afd, void *context) {
   return {body, /*isTypeChecked=*/true};
 }
 
-ConcreteDeclRef
-Solution::resolveConcreteDeclRef(ValueDecl *decl,
-                                 ConstraintLocator *locator) const {
-  if (!decl)
-    return ConcreteDeclRef();
-
-  // Get the generic signatue of the decl and compute the substitutions.
-  auto sig = decl->getInnermostDeclContext()->getGenericSignatureOfContext();
-  auto subst = computeSubstitutions(sig, locator);
+static ValueDecl *generateThunkForExtraMetatypes(SubstitutionMap subst,
+                                                 FuncDecl *oldDecl,
+                                                 FuncDecl *newDecl) {
+  // 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(oldDecl->getASTContext(),
+                                                                  oldDecl, subst);
+  auto thunk = FuncDecl::createImplicit(
+      oldDecl->getASTContext(), oldDecl->getStaticSpelling(), oldDecl->getName(),
+      oldDecl->getNameLoc(), oldDecl->hasAsync(), oldDecl->hasThrows(),
+      /*genericParams=*/nullptr, thunkTypeAndParamList.second,
+      thunkTypeAndParamList.first->getResult(), oldDecl->getDeclContext());
+  thunk->copyFormalAccessFrom(oldDecl);
+  thunk->setBodySynthesizer(synthesizeForwardingThunkBody, newDecl);
+  thunk->setSelfAccessKind(oldDecl->getSelfAccessKind());
+
+  return thunk;
+}
 
-  // Lazily instantiate function definitions for class template specializations.
-  // Members of a class template specialization will be instantiated here (not
-  // when imported). If this method has already be instantiated, then this is a
-  // no-op.
+// Lazily instantiate function definitions for class template specializations.
+// Members of a class template specialization will be instantiated here (not
+// when imported). If this method has already be instantiated, then this is a
+// no-op.
+static void maybeInstantiateCXXMethodDefinition(ValueDecl *decl) {
   if (const auto *constMethod =
           dyn_cast_or_null<clang::CXXMethodDecl>(decl->getClangDecl())) {
     auto method = const_cast<clang::CXXMethodDecl *>(constMethod);
@@ -273,41 +296,62 @@ Solution::resolveConcreteDeclRef(ValueDecl *decl,
           ->getClangSema()
           .InstantiateFunctionDefinition(method->getLocation(), method);
   }
+}
+
+static ConcreteDeclRef getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
+                                                            ValueDecl *decl) {
+  assert(isa<clang::FunctionTemplateDecl>(decl->getClangDecl()) &&
+      "This API should only be used with function templates.");
+
+  auto *newFn =
+      decl->getASTContext()
+          .getClangModuleLoader()
+          ->instantiateCXXFunctionTemplate(
+              decl->getASTContext(),
+              const_cast<clang::FunctionTemplateDecl *>(
+                  cast<clang::FunctionTemplateDecl>(decl->getClangDecl())),
+              subst);
+  // We failed to specialize this function template. The compiler is going to
+  // exit soon. Return something valid in the meantime.
+  if (!newFn)
+    return ConcreteDeclRef(decl);
+
+  auto newDecl = cast_or_null<ValueDecl>(
+      decl->getASTContext().getClangModuleLoader()->importDeclDirectly(
+          newFn));
+
+  if (auto fn = dyn_cast<AbstractFunctionDecl>(newDecl)) {
+    if (!subst.empty()) {
+      newDecl = rewriteIntegerTypes(subst, decl, fn);
+    }
+  }
+
+  if (auto fn = dyn_cast<FuncDecl>(decl)) {
+    if (newFn->getNumParams() != fn->getParameters()->size()) {
+      newDecl = generateThunkForExtraMetatypes(subst, fn,
+                                               cast<FuncDecl>(newDecl));
+    }
+  }
+
+  return ConcreteDeclRef(newDecl);
+}
+
+ConcreteDeclRef
+Solution::resolveConcreteDeclRef(ValueDecl *decl,
+                                 ConstraintLocator *locator) const {
+  if (!decl)
+    return ConcreteDeclRef();
+
+  // Get the generic signatue of the decl and compute the substitutions.
+  auto sig = decl->getInnermostDeclContext()->getGenericSignatureOfContext();
+  auto subst = computeSubstitutions(sig, locator);
+
+  maybeInstantiateCXXMethodDefinition(decl);
 
   // If this is a C++ function template, get it's specialization for the given
   // substitution map and update the decl accordingly.
   if (isa_and_nonnull<clang::FunctionTemplateDecl>(decl->getClangDecl())) {
-    auto *newFn =
-        decl->getASTContext()
-            .getClangModuleLoader()
-            ->instantiateCXXFunctionTemplate(
-                decl->getASTContext(),
-                const_cast<clang::FunctionTemplateDecl *>(
-                    cast<clang::FunctionTemplateDecl>(decl->getClangDecl())),
-                subst);
-    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 getCXXFunctionTemplateSpecialization(subst, decl);
   }
 
   return ConcreteDeclRef(decl, subst);

test/Interop/Cxx/class/constructors-irgen.swift

@@ -11,6 +11,9 @@ typealias Void = ()
 struct UnsafePointer<T> { }
 struct UnsafeMutablePointer<T> { }
 
+struct Int { }
+struct UInt { }
+
 public func createHasVirtualBase() -> HasVirtualBase {
   // ITANIUM_X64: define swiftcc void @"$ss20createHasVirtualBaseSo0bcD0VyF"(%TSo14HasVirtualBaseV* noalias nocapture sret({{.*}}) %0)
   // ITANIUM_X64-NOT: define

test/Interop/Cxx/class/constructors-silgen.swift

@@ -65,7 +65,7 @@ public func deletedConstructor(a: UnsafeMutablePointer<Int32>) {
 // CHECK: apply [[FN]]([[TEMPL]], [[ARG_VAL]]) : $@convention(c) (ArgType) -> @out TemplatedConstructor
 // CHECK-LABEL: end sil function '$s4main20templatedConstructoryyF'
 
-// CHECK-LABEL: sil hidden_external [clang TemplatedConstructor.init] @{{_ZN20TemplatedConstructorC1I7ArgTypeEET_|\?\?\$\?0UArgType@@@TemplatedConstructor@@QEAA@UArgType@@@Z}} : $@convention(c) (ArgType) -> @out TemplatedConstructor
+// CHECK-LABEL: sil [clang TemplatedConstructor.init] @{{_ZN20TemplatedConstructorC1I7ArgTypeEET_|\?\?\$\?0UArgType@@@TemplatedConstructor@@QEAA@UArgType@@@Z}} : $@convention(c) (ArgType) -> @out TemplatedConstructor
 public func templatedConstructor() {
   let templated = TemplatedConstructor(ArgType())
 }

test/Interop/Cxx/templates/defaulted-template-type-parameter-module-interface.swift

@@ -14,7 +14,7 @@
 // CHECK: func defaultedTemplateTypeParamAndUnrealtedParam(_: Int32)
 // CHECK: func overloadedDefaultedTemplate<T>(_: T)
 // CHECK: func overloadedDefaultedTemplate(_: Int32)
-// CHECK: func defaultedTemplateReferenceTypeParam<T>(_ t: UnsafeMutablePointer<T>)
+// CHECK: func defaultedTemplateReferenceTypeParam<T>(_ t: inout T)
 // The following types aren't imported correctly, but that does not have to do
 // with the fact that the template type paramaters are defaulted.
 // CHECK: func defaultedTemplatePointerTypeParam<T>(_ t: UnsafeMutablePointer<T>)

test/Interop/Cxx/templates/function-template-irgen-objc.swift

@@ -14,13 +14,13 @@
 // CHECK:   mutating func test2(_: Int32, _ varargs: Any...)
 // CHECK: }
 
-// CHECK: func lvalueReference<T>(_ ref: UnsafeMutablePointer<T>)
-// CHECK: func constLvalueReference<T>(_: UnsafePointer<T>)
-// CHECK: func forwardingReference<T>(_: UnsafeMutablePointer<T>)
+// CHECK: func lvalueReference<T>(_ ref: inout T)
+// CHECK: func constLvalueReference<T>(_: inout T)
+// CHECK: func forwardingReference<T>(_: inout T)
 // CHECK: func PointerTemplateParameter<T>(_: UnsafeMutablePointer<T>)
 
 // CHECK: enum Orbiters {
 // CHECK:   static func galileo<T>(_: T)
 // CHECK:   static func cassini<T, U>(_: T, _: U)
-// CHECK:   static func magellan<T>(_: UnsafeMutablePointer<T>)
+// CHECK:   static func magellan<T>(_: inout T)
 // CHECK: }

test/Interop/Cxx/templates/function-template-module-interface.swift

@@ -28,15 +28,6 @@ FunctionTemplateTestSuite.test("lvalueReference<T> where T == Int") {
   expectEqual(value, 42)
 }
 
-// TODO: currently "Any" is imported as an Objective-C "id".
-// This doesn't work without the Objective-C runtime. 
-#if _runtime(_ObjC)
-FunctionTemplateTestSuite.test("passThrough<T> where T == Any") {
-  let result = passThrough(42 as Any)
-  expectEqual(42, result as! Int)
-}
-#endif
-
 // TODO: Generics, Any, and Protocols should be tested here but need to be
 // better supported in ClangTypeConverter first.