Skip to content

CTAD: parameter pack assertion crash when synthesizing deduction guides for alias #90209

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

Closed
hokein opened this issue Apr 26, 2024 · 9 comments · Fixed by #98013
Closed

CTAD: parameter pack assertion crash when synthesizing deduction guides for alias #90209

hokein opened this issue Apr 26, 2024 · 9 comments · Fixed by #98013
Assignees
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party crash Prefer [crash-on-valid] or [crash-on-invalid]

Comments

@hokein
Copy link
Collaborator

hokein commented Apr 26, 2024

Example:

struct Key1 {}; 

template <int i>
struct Item {};

template <class... Ts>
struct List {};

template <class Key, class L>
struct TemplatedClass {
  template <class... Items>
  TemplatedClass(Key, Items...);
};

template <class Key, class...Items>
  requires(sizeof(Key) > 0 && (sizeof(Items) && ...))
TemplatedClass(Key, Items...) -> TemplatedClass<Key, List<Items...>>;

template <class T>
struct HasTemplateParam {
  template <class Key, class... Items>
  using Alias = TemplatedClass<Key, List<Items...>>;
};

struct Mod1 : HasTemplateParam<Mod1> {
  using Foo1 = decltype(Alias{Key1()});
};

https://godbolt.org/z/fssjafzYe.

@hokein hokein added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2024

@llvm/issue-subscribers-c-20

Author: Haojian Wu (hokein)

Example:
struct Key1 {}; 

template &lt;int i&gt;
struct Item {};

template &lt;class... Ts&gt;
struct List {};

template &lt;class Key, class L&gt;
struct TemplatedClass {
  template &lt;class... Items&gt;
  TemplatedClass(Key, Items...);
};

template &lt;class Key, class...Items&gt;
  requires(sizeof(Key) &gt; 0 &amp;&amp; (sizeof(Items) &amp;&amp; ...))
TemplatedClass(Key, Items...) -&gt; TemplatedClass&lt;Key, List&lt;Items...&gt;&gt;;

template &lt;class T&gt;
struct HasTemplateParam {
  template &lt;class Key, class... Items&gt;
  using Alias = TemplatedClass&lt;Key, List&lt;Items...&gt;&gt;;
};

struct Mod1 : HasTemplateParam&lt;Mod1&gt; {
  using Foo1 = decltype(Alias{Key1()});
};

https://godbolt.org/z/fssjafzYe.

@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2024

@llvm/issue-subscribers-clang-frontend

Author: Haojian Wu (hokein)

Example:
struct Key1 {}; 

template &lt;int i&gt;
struct Item {};

template &lt;class... Ts&gt;
struct List {};

template &lt;class Key, class L&gt;
struct TemplatedClass {
  template &lt;class... Items&gt;
  TemplatedClass(Key, Items...);
};

template &lt;class Key, class...Items&gt;
  requires(sizeof(Key) &gt; 0 &amp;&amp; (sizeof(Items) &amp;&amp; ...))
TemplatedClass(Key, Items...) -&gt; TemplatedClass&lt;Key, List&lt;Items...&gt;&gt;;

template &lt;class T&gt;
struct HasTemplateParam {
  template &lt;class Key, class... Items&gt;
  using Alias = TemplatedClass&lt;Key, List&lt;Items...&gt;&gt;;
};

struct Mod1 : HasTemplateParam&lt;Mod1&gt; {
  using Foo1 = decltype(Alias{Key1()});
};

https://godbolt.org/z/fssjafzYe.

@EugeneZelenko EugeneZelenko added the crash Prefer [crash-on-valid] or [crash-on-invalid] label Apr 26, 2024
@shafik
Copy link
Collaborator

shafik commented Apr 26, 2024

Confirmed, it looks like this bug is an addition since clang-18 release. Do you know which change introduced it?

@shafik shafik added the confirmed Verified by a second party label Apr 26, 2024
@hokein
Copy link
Collaborator Author

hokein commented Apr 29, 2024

I think it is a new issue introduced by the recent changes I made to implement the alias CTAD feature as the crash occurs in that code path.

@shafik
Copy link
Collaborator

shafik commented Apr 29, 2024

I think it is a new issue introduced by the recent changes I made to implement the alias CTAD feature as the crash occurs in that code path.

Do you understand the underlying issues? Do you have plan to fix it?

@hokein
Copy link
Collaborator Author

hokein commented May 15, 2024

I haven't delved into it deeply yet, but I suspect we may have some issues with handling the parameter pack. I plan to address this before the next clang release.

@hokein hokein self-assigned this May 28, 2024
@hokein
Copy link
Collaborator Author

hokein commented May 28, 2024

A more simplified test case:

template <class Ts>
struct List {
  List(int);
};

template <class T1>
struct TemplatedClass {
  TemplatedClass(T1);
};

template <class T1>
TemplatedClass(T1) -> TemplatedClass<List<T1>>;

template <class T2>
using Alias = TemplatedClass<List<T2>>;

Alias test(1);

The alias deduction guide synthesized from the written TemplatedClass deduction guide is ill-formed because its template parameter list is empty:

FunctionTemplateDecl 0x55c2a051f440 </tmp/t8.cpp:14:1, line:15:38> col:1 <deduction guide for Alias>
|-TypeTraitExpr 0x55c2a051f330 <col:1> 'bool' __is_deducible
| |-DeducedTemplateSpecializationType 0x55c2a051dc00 'Alias' dependent
| `-ElaboratedType 0x55c2a051f150 'TemplatedClass<List<type-parameter-0-0>>' sugar dependent
|   `-TemplateSpecializationType 0x55c2a051f100 'TemplatedClass<List<type-parameter-0-0>>' dependent TemplatedClass
|     `-TemplateArgument type 'List<type-parameter-0-0>'
|       `-ElaboratedType 0x55c2a051f0b0 'List<type-parameter-0-0>' sugar dependent
|         `-TemplateSpecializationType 0x55c2a051f060 'List<type-parameter-0-0>' dependent List
|           `-TemplateArgument type 'type-parameter-0-0'
|             `-SubstTemplateTypeParmType 0x55c2a051efa0 'type-parameter-0-0' sugar dependent class depth 0 index 0 T1
|               |-FunctionTemplate 0x55c2a051ba20 '<deduction guide for TemplatedClass>'
|               `-TemplateTypeParmType 0x55c2a04fa090 'type-parameter-0-0' dependent depth 0 index 0
`-CXXDeductionGuideDecl 0x55c2a051f378 <line:14:1, line:15:38> col:1 <deduction guide for Alias> 'auto (type-parameter-0-0) -> TemplatedClass<List<type-parameter-0-0>>'
  `-ParmVarDecl 0x55c2a051efe8 <line:12:16> col:18 'type-parameter-0-0'

When deducing the template arguments of the return type of the TemplatedClass deduction guide from the right-hand side of Alias, we expect T1 to be deduced to T2, and the deduced argument to hold sufficient information to retrieve the T2 template parameter declaration of Alias. However, we don't have enough information in this case (specifically, with TemplateSpecializationType). The deduced argument is merely a canonicalTemplateTypeParmType, which only has the index and depth. Consequently, TemplateParamsReferencedInTemplateArgumentList cannot determine that T2 is referenced in the deduced argument, resulting in an empty template parameter list.

This seems working as intended, as clang loses this kind of information during the template argument deduction, see the FIXME.

@hokein
Copy link
Collaborator Author

hokein commented May 28, 2024

This seems working as intended, as clang loses this kind of information during the template argument deduction, see the FIXME.

Ideally, addressing the FIXME should fix this issue. @mizvekov, how hard would it be to address it? If it is hard or requires significant work, I think we can use fix this crash by using the index and depth to retrieve the corresponding decl from the template parameter list of the alias.

@mizvekov
Copy link
Contributor

This seems working as intended, as clang loses this kind of information during the template argument deduction, see the FIXME.

Ideally, addressing the FIXME should fix this issue. @mizvekov, how hard would it be to address it? If it is hard or requires significant work, I think we can use fix this crash by using the index and depth to retrieve the corresponding decl from the template parameter list of the alias.

That FIXME is partially addressed in #93433, regarding the TemplateName.
It will be fully addressed, regarding the arguments, by mizvekov@252f292 when I get the time to rebase and submit a PR.

But I don't think we should depend on type sugar to be correct here, figuring out the correct parameter from depth and index alone is more consistent.

@hokein hokein added this to the LLVM 19.X Release milestone Jun 25, 2024
@hokein hokein closed this as completed in 8b7263b Jul 11, 2024
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this issue Jul 14, 2024
…TemplateParamsReferencedInTemplateArgumentList (llvm#98013)

As described in llvm#90209 (comment),
Clang may not preserve enough information during template argument
deduction. This can result in a merely canonical `TemplateTypeParmType`
with a null `Decl`, leading to an incomplete template parameter list for
the synthesized deduction guide.

This patch addresses the issue by using the index and depth information
to retrieve the corresponding template parameter, rather than relying on
`TTP->getDecl()`.

Fixes llvm#90209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party crash Prefer [crash-on-valid] or [crash-on-invalid]
Projects
None yet
5 participants