Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[cxx-interop] Support function templates. #33053

Merged
merged 1 commit into from
Oct 22, 2020
Merged

[cxx-interop] Support function templates. #33053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion include/swift/AST/ASTContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@
#include "swift/AST/Import.h"
#include "swift/AST/SearchPathOptions.h"
#include "swift/AST/Type.h"
#include "swift/AST/Types.h"
#include "swift/AST/TypeAlignments.h"
#include "swift/AST/Types.h"
#include "swift/Basic/LangOptions.h"
#include "swift/Basic/Located.h"
#include "swift/Basic/Malloc.h"
#include "clang/AST/DeclTemplate.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
Expand Down Expand Up @@ -628,6 +629,13 @@ class ASTContext final {
ArrayRef<SILParameterInfo> params, Optional<SILResultInfo> result,
SILFunctionType::Representation trueRep);

/// Instantiates "Impl.Converter" if needed, then calls
/// ClangTypeConverter::getClangTemplateArguments.
zoecarver marked this conversation as resolved.
Show resolved Hide resolved
std::unique_ptr<TemplateInstantiationError> getClangTemplateArguments(
const clang::TemplateParameterList *templateParams,
ArrayRef<Type> genericArgs,
SmallVectorImpl<clang::TemplateArgument> &templateArgs);

/// Get the Swift declaration that a Clang declaration was exported from,
/// if applicable.
const Decl *getSwiftDeclForExportedClangDecl(const clang::Decl *decl);
Expand Down
14 changes: 14 additions & 0 deletions include/swift/AST/ClangModuleLoader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
#define SWIFT_AST_CLANG_MODULE_LOADER_H

#include "swift/AST/ModuleLoader.h"
#include "swift/AST/SubstitutionMap.h"
#include "swift/Basic/TaggedUnion.h"
#include "clang/AST/DeclTemplate.h"

namespace clang {
class ASTContext;
Expand Down Expand Up @@ -219,6 +221,18 @@ class ClangModuleLoader : public ModuleLoader {
/// based on subtleties like the target module interface format.
virtual bool isSerializable(const clang::Type *type,
bool checkCanonical) const = 0;

virtual clang::FunctionDecl *
instantiateCXXFunctionTemplate(ASTContext &ctx,
clang::FunctionTemplateDecl *func,
SubstitutionMap subst) = 0;
};

/// Used to describe a template instantiation error.
struct TemplateInstantiationError {
/// Generic types that could not be converted to QualTypes using the
/// ClangTypeConverter.
SmallVector<Type, 4> failedTypes;
};

} // namespace swift
Expand Down
7 changes: 4 additions & 3 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5960,9 +5960,10 @@ class FuncDecl : public AbstractFunctionDecl {
DeclContext *Parent);

static FuncDecl *createImported(ASTContext &Context, SourceLoc FuncLoc,
DeclName Name, SourceLoc NameLoc,
bool Async, bool Throws,
ParameterList *BodyParams, Type FnRetType,
DeclName Name, SourceLoc NameLoc, bool Async,
bool Throws, ParameterList *BodyParams,
Type FnRetType,
GenericParamList *GenericParams,
DeclContext *Parent, ClangNode ClangN);

bool isStatic() const;
Expand Down
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1674,6 +1674,11 @@ ERROR(where_nongeneric_ctx,none,
ERROR(where_nongeneric_toplevel,none,
"'where' clause cannot be applied to a non-generic top-level "
"declaration", ())
ERROR(unable_to_convert_generic_swift_types,none,
"could not generate C++ types from the generic Swift types provided. "
"The following Swift type(s) provided to '%0' were unable to be "
zoecarver marked this conversation as resolved.
Show resolved Hide resolved
"converted: %1.",
(StringRef, StringRef))

// Type aliases
ERROR(type_alias_underlying_type_access,none,
Expand Down
5 changes: 5 additions & 0 deletions include/swift/ClangImporter/ClangImporter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,11 @@ class ClangImporter final : public ClangModuleLoader {

bool isSerializable(const clang::Type *type,
bool checkCanonical) const override;

clang::FunctionDecl *
instantiateCXXFunctionTemplate(ASTContext &ctx,
clang::FunctionTemplateDecl *func,
SubstitutionMap subst) override;
};

ImportDecl *createImportDecl(ASTContext &Ctx, DeclContext *DC, ClangNode ClangN,
Expand Down
23 changes: 19 additions & 4 deletions lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
#include "ClangTypeConverter.h"
#include "ForeignRepresentationInfo.h"
#include "SubstitutionMapStorage.h"
#include "swift/AST/ForeignAsyncConvention.h"
#include "swift/AST/ClangModuleLoader.h"
#include "swift/AST/ConcreteDeclRef.h"
#include "swift/AST/DiagnosticEngine.h"
#include "swift/AST/DiagnosticsSema.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/FileUnit.h"
#include "swift/AST/ForeignAsyncConvention.h"
#include "swift/AST/ForeignErrorConvention.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/GenericSignature.h"
Expand All @@ -41,19 +41,19 @@
#include "swift/AST/PropertyWrappers.h"
#include "swift/AST/ProtocolConformance.h"
#include "swift/AST/RawComment.h"
#include "swift/AST/SILLayout.h"
#include "swift/AST/SemanticAttrs.h"
#include "swift/AST/SourceFile.h"
#include "swift/AST/SubstitutionMap.h"
#include "swift/AST/SILLayout.h"
#include "swift/AST/TypeCheckRequests.h"
#include "swift/Basic/Compiler.h"
#include "swift/Basic/SourceManager.h"
#include "swift/Basic/Statistic.h"
#include "swift/Basic/StringExtras.h"
#include "swift/Syntax/References.h"
#include "swift/Syntax/SyntaxArena.h"
#include "swift/Strings.h"
#include "swift/Subsystems.h"
#include "swift/Syntax/References.h"
#include "swift/Syntax/SyntaxArena.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/StringMap.h"
Expand Down Expand Up @@ -4563,6 +4563,21 @@ ASTContext::getCanonicalClangFunctionType(
return ty ? ty->getCanonicalTypeInternal().getTypePtr() : nullptr;
}

std::unique_ptr<TemplateInstantiationError>
ASTContext::getClangTemplateArguments(
const clang::TemplateParameterList *templateParams,
ArrayRef<Type> genericArgs,
SmallVectorImpl<clang::TemplateArgument> &templateArgs) {
auto &impl = getImpl();
if (!impl.Converter) {
auto *cml = getClangModuleLoader();
impl.Converter.emplace(*this, cml->getClangASTContext(), LangOpts.Target);
}

return impl.Converter->getClangTemplateArguments(templateParams, genericArgs,
templateArgs);
}

const Decl *
ASTContext::getSwiftDeclForExportedClangDecl(const clang::Decl *decl) {
auto &impl = getImpl();
Expand Down
30 changes: 30 additions & 0 deletions lib/AST/ClangTypeConverter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,3 +807,33 @@ Decl *ClangTypeConverter::getSwiftDeclForExportedClangDecl(
auto it = ReversedExportMap.find(decl);
return (it != ReversedExportMap.end() ? it->second : nullptr);
}

std::unique_ptr<TemplateInstantiationError>
ClangTypeConverter::getClangTemplateArguments(
const clang::TemplateParameterList *templateParams,
ArrayRef<Type> genericArgs,
SmallVectorImpl<clang::TemplateArgument> &templateArgs) {
zoecarver marked this conversation as resolved.
Show resolved Hide resolved
// Keep track of the types we failed to convert so we can return a useful
// error.
SmallVector<Type, 2> failedTypes;
for (clang::NamedDecl *param : *templateParams) {
// Note: all template parameters must be template type parameters. This is
// verified when we import the clang decl.
zoecarver marked this conversation as resolved.
Show resolved Hide resolved
auto templateParam = cast<clang::TemplateTypeParmDecl>(param);
auto replacement = genericArgs[templateParam->getIndex()];
auto qualType = convert(replacement);
if (qualType.isNull()) {
failedTypes.push_back(replacement);
// Find all the types we can't convert.
continue;
zoecarver marked this conversation as resolved.
Show resolved Hide resolved
}
templateArgs.push_back(clang::TemplateArgument(qualType));
}
if (failedTypes.empty())
return nullptr;
auto errorInfo = std::make_unique<TemplateInstantiationError>();
llvm::for_each(failedTypes, [&errorInfo](auto type) {
errorInfo->failedTypes.push_back(type);
});
return errorInfo;
}
10 changes: 10 additions & 0 deletions lib/AST/ClangTypeConverter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,18 @@ class ClangTypeConverter :
/// Swift declaration.
Decl *getSwiftDeclForExportedClangDecl(const clang::Decl *decl) const;

/// Translate Swift generic arguments to template arguments.
zoecarver marked this conversation as resolved.
Show resolved Hide resolved
///
/// \returns nullptr if successful. If an error occors, returns a unique_ptr
/// to a `TemplateInstantiationError` with a list of the failed types.
zoecarver marked this conversation as resolved.
Show resolved Hide resolved
std::unique_ptr<TemplateInstantiationError> getClangTemplateArguments(
const clang::TemplateParameterList *templateParams,
ArrayRef<Type> genericArgs,
SmallVectorImpl<clang::TemplateArgument> &templateArgs);

private:
varungandhi-apple marked this conversation as resolved.
Show resolved Hide resolved
clang::QualType convert(Type type);

clang::QualType convertMemberType(NominalTypeDecl *DC,
StringRef memberName);

Expand Down
13 changes: 6 additions & 7 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7227,16 +7227,15 @@ FuncDecl *FuncDecl::createImplicit(ASTContext &Context,
}

FuncDecl *FuncDecl::createImported(ASTContext &Context, SourceLoc FuncLoc,
DeclName Name, SourceLoc NameLoc,
bool Async, bool Throws,
ParameterList *BodyParams,
Type FnRetType, DeclContext *Parent,
ClangNode ClangN) {
DeclName Name, SourceLoc NameLoc, bool Async,
bool Throws, ParameterList *BodyParams,
Type FnRetType,
GenericParamList *GenericParams,
DeclContext *Parent, ClangNode ClangN) {
assert(ClangN && FnRetType);
auto *const FD = FuncDecl::createImpl(
Context, SourceLoc(), StaticSpellingKind::None, FuncLoc, Name, NameLoc,
Async, SourceLoc(), Throws, SourceLoc(),
/*GenericParams=*/nullptr, Parent, ClangN);
Async, SourceLoc(), Throws, SourceLoc(), GenericParams, Parent, ClangN);
FD->setParameters(BodyParams);
FD->setResultInterfaceType(FnRetType);
return FD;
Expand Down
34 changes: 31 additions & 3 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@
#include "swift/AST/ClangModuleLoader.h"
#include "swift/AST/DiagnosticEngine.h"
#include "swift/AST/DiagnosticsClangImporter.h"
#include "swift/AST/ImportCache.h"
#include "swift/AST/DiagnosticsSema.h"
#include "swift/AST/IRGenOptions.h"
#include "swift/AST/ImportCache.h"
#include "swift/AST/LinkLibrary.h"
#include "swift/AST/Module.h"
#include "swift/AST/NameLookup.h"
Expand All @@ -35,8 +36,6 @@
#include "swift/ClangImporter/ClangModule.h"
#include "swift/Config.h"
#include "swift/Demangling/Demangle.h"
#include "swift/ClangImporter/ClangModule.h"
#include "swift/Config.h"
#include "swift/Parse/Lexer.h"
#include "swift/Parse/Parser.h"
#include "swift/Strings.h"
Expand Down Expand Up @@ -4082,3 +4081,32 @@ swift::getModuleCachePathFromClang(const clang::CompilerInstance &Clang) {
return llvm::sys::path::parent_path(SpecificModuleCachePath).str();
}

clang::FunctionDecl *ClangImporter::instantiateCXXFunctionTemplate(
ASTContext &ctx, clang::FunctionTemplateDecl *func, SubstitutionMap subst) {
SmallVector<clang::TemplateArgument, 4> templateSubst;
std::unique_ptr<TemplateInstantiationError> error =
ctx.getClangTemplateArguments(func->getTemplateParameters(),
subst.getReplacementTypes(), templateSubst);
if (error) {
std::string failedTypesStr;
llvm::raw_string_ostream failedTypesStrStream(failedTypesStr);
llvm::interleaveComma(error->failedTypes, failedTypesStrStream);
// TODO: Use the location of the apply here.
// TODO: This error message should not reference implementation details.
// See: https://github.com/apple/swift/pull/33053#discussion_r477003350
ctx.Diags.diagnose(SourceLoc(),
diag::unable_to_convert_generic_swift_types.ID,
{func->getName(), StringRef(failedTypesStr)});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@varungandhi-apple can I get your opinion on error handling again here? The issue is, on Windows, clang will crash if we don't exit here (because of an assertion: TPL->size() == TemplateArgs.size() && "size mismatch between args and parms!"). That's only on Windows, though. On other platforms, we're allowed to continue which I think is good because it allows us to report more errors before aborting compilation. I see a few paths forward, what do you think would be the best (others, please feel free to chime in as well)?

  1. Create an SR/TODO and don't test this particular error on Windows.
  2. Try to "fix" this in LLVM. I'm not sure this is a very good or practical solution, I think clang/llvm reviewers will probably be against removing this (completely valid) assertion for good reason.
  3. (somehow) Tell the compiler to abort compilation after emitting this error.

Copy link
Contributor

Choose a reason for hiding this comment

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

[I only briefly skimmed the Clang code...] The assertion you pointed out is in MicrosoftCXXNameMangler::mangleTemplateArgs. I don't understand why the name mangler is being called upon template instantiation. That seems strange... shouldn't name mangling happen much later in the pipeline? I would expect that if there's an instantiation error, and I hope there is one when there's a size mismatch, we won't even get to the name mangler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, your comment made me realize, I think the issue is completely my fault. What we need to do is return nullptr after emitting the diagnostic. I think that will fix this.

I think name mangling probably happens when we try to instantiate the function template.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. That's not what I was talking about (and I totally missed that we didn't return here in case of an error)... but glad that it helped you figured out the issue. 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All is well that ends well :) Now let's just hope the build succeeds on Windows.

// Return a valid FunctionDecl but, we'll never use it.
return func->getAsFunction();
}

// Instanciate a specialization of this template using the substitution map.
auto *templateArgList = clang::TemplateArgumentList::CreateCopy(
func->getASTContext(), templateSubst);
auto &sema = getClangInstance().getSema();
auto *spec = sema.InstantiateFunctionDeclaration(func, templateArgList,
clang::SourceLocation());
sema.InstantiateFunctionDefinition(clang::SourceLocation(), spec);
return spec;
}