Skip to content

[cxx-interop] Do not import function templates with a parameter pack - #91410

Open
patrykstefanski wants to merge 1 commit into
swiftlang:mainfrom
patrykstefanski:bail-on-function-template-parameter-packs
Open

[cxx-interop] Do not import function templates with a parameter pack#91410
patrykstefanski wants to merge 1 commit into
swiftlang:mainfrom
patrykstefanski:bail-on-function-template-parameter-packs

Conversation

@patrykstefanski

Copy link
Copy Markdown
Contributor

A pack typename... Ts is a clang::TemplateTypeParmDecl, so the existing "all template parameters must be template type parameters" guard in VisitFunctionTemplateDecl let it through. The pack collapsed into a single ordinary generic parameter and the pack expansion parameter was imported as Any:

func parameterPack<Ts>(_ ts: Any, Ts: Ts.Type)

Calling that from Swift triggers an assertion in getPackAsArray().

Bail on packs instead, the way non-type template parameters are already treated, so a call site gets "cannot find 'parameterPack' in scope" rather than an assertion failure.

rdar://184658782

A pack `typename... Ts` is a clang::TemplateTypeParmDecl, so the
existing "all template parameters must be template type parameters"
guard in VisitFunctionTemplateDecl let it through. The pack collapsed
into a single ordinary generic parameter and the pack expansion
parameter was imported as Any:

    func parameterPack<Ts>(_ ts: Any, Ts: Ts.Type)

Calling that from Swift triggers an assertion in getPackAsArray().

Bail on packs instead, the way non-type template parameters are already
treated, so a call site gets "cannot find 'parameterPack' in scope"
rather than an assertion failure.

rdar://184658782
@patrykstefanski

Copy link
Copy Markdown
Contributor Author

@swift-ci please test

@egorzhdan egorzhdan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +5015 to +5017
const auto *typeParam =
dyn_cast<clang::TemplateTypeParmDecl>(param);
return typeParam && !typeParam->isParameterPack();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could probably hoist this check into importNameImpl and save some cycles on name lookup. I don't think this is very important though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ interop Feature: Interoperability with C++

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants