Skip to content

[cxx-interop] Allow importing templated functions when template args do not appear in function signature #39535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
50 changes: 47 additions & 3 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down
8 changes: 7 additions & 1 deletion lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
116 changes: 104 additions & 12 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we could use auto [a, b] = 😢

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think various methods/static methods might take a meta type. We need to verify if this will break that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are passing, so I guess this isn't an issue.

}
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(
Expand All @@ -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.
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this patch, but at some point we should move all synthesizers to their own file. They just take up space right now and clutter the files they're scattered across.

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
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions test/Interop/Cxx/templates/Inputs/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading