CTAD: incorrect transformed require-clause for the alias deduction guide #90177

hokein · 2024-04-26T08:23:01Z

Example:

struct Key1  {};

template <class Key>
struct Foo {
  Foo(Key);
};

template <class D>
constexpr bool C = sizeof(D);

template <class Key>
  requires (C<Key>)
Foo(Key) -> Foo<Key>;

template <class T>
struct HasTemplateParam {
  template <class Key>
  using Alias = Foo<Key>;
};

class Forward;
struct Test : HasTemplateParam<Forward> {
  using Foo1 = decltype(Alias{
    Key1(),
  });
};

clang rejects the above valid code with the following diagnostics:

<source>:9:20: error: invalid application of 'sizeof' to an incomplete type 'Forward'
    9 | constexpr bool C = sizeof(D);
      |                    ^~~~~~~~~
<source>:12:13: note: in instantiation of variable template specialization 'C<Forward>' requested here
   12 |   requires (C<Key>)
      |             ^
<source>:12:13: note: while substituting template arguments into constraint expression here
   12 |   requires (C<Key>)
      |             ^~~~~~
<source>:23:25: note: while checking constraint satisfaction for template '<deduction guide for Alias><Forward>' required here
   23 |   using Foo1 = decltype(Alias{
      |                         ^~~~~
<source>:23:25: note: in instantiation of function template specialization 'HasTemplateParam<Forward>::<deduction guide for Alias><Key1>' requested here
<source>:21:7: note: forward declaration of 'Forward'
   21 | class Forward;

It seems to be an issue in the transformed require-clause for the alias deduction guide. The require expression should be evaluated on the type Key1, rather than the outer type Forward.

The text was updated successfully, but these errors were encountered:

llvmbot · 2024-04-26T08:23:18Z

@llvm/issue-subscribers-c-20

Author: Haojian Wu (hokein)

Example:

struct Key1  {};

template &lt;class Key&gt;
struct Foo {
  Foo(Key);
};

template &lt;class D&gt;
constexpr bool C = sizeof(D);

template &lt;class Key&gt;
  requires (C&lt;Key&gt;)
Foo(Key) -&gt; Foo&lt;Key&gt;;

template &lt;class T&gt;
struct HasTemplateParam {
  template &lt;class Key&gt;
  using Alias = Foo&lt;Key&gt;;
};

class Forward;
struct Test : HasTemplateParam&lt;Forward&gt; {
  using Foo1 = decltype(Alias{
    Key1(),
  });
};

clang rejects the above valid code with the following diagnostics:

&lt;source&gt;:9:20: error: invalid application of 'sizeof' to an incomplete type 'Forward'
    9 | constexpr bool C = sizeof(D);
      |                    ^~~~~~~~~
&lt;source&gt;:12:13: note: in instantiation of variable template specialization 'C&lt;Forward&gt;' requested here
   12 |   requires (C&lt;Key&gt;)
      |             ^
&lt;source&gt;:12:13: note: while substituting template arguments into constraint expression here
   12 |   requires (C&lt;Key&gt;)
      |             ^~~~~~
&lt;source&gt;:23:25: note: while checking constraint satisfaction for template '&lt;deduction guide for Alias&gt;&lt;Forward&gt;' required here
   23 |   using Foo1 = decltype(Alias{
      |                         ^~~~~
&lt;source&gt;:23:25: note: in instantiation of function template specialization 'HasTemplateParam&lt;Forward&gt;::&lt;deduction guide for Alias&gt;&lt;Key1&gt;' requested here
&lt;source&gt;:21:7: note: forward declaration of 'Forward'
   21 | class Forward;

It seems to be an issue in the transformed require-clause for the alias deduction guide. The require expression should be evaluated on the type Key1, rather than the outer type Forward.

llvmbot · 2024-04-26T08:23:29Z

@llvm/issue-subscribers-clang-frontend

Author: Haojian Wu (hokein)

Example:

struct Key1  {};

template &lt;class Key&gt;
struct Foo {
  Foo(Key);
};

template &lt;class D&gt;
constexpr bool C = sizeof(D);

template &lt;class Key&gt;
  requires (C&lt;Key&gt;)
Foo(Key) -&gt; Foo&lt;Key&gt;;

template &lt;class T&gt;
struct HasTemplateParam {
  template &lt;class Key&gt;
  using Alias = Foo&lt;Key&gt;;
};

class Forward;
struct Test : HasTemplateParam&lt;Forward&gt; {
  using Foo1 = decltype(Alias{
    Key1(),
  });
};

clang rejects the above valid code with the following diagnostics:

&lt;source&gt;:9:20: error: invalid application of 'sizeof' to an incomplete type 'Forward'
    9 | constexpr bool C = sizeof(D);
      |                    ^~~~~~~~~
&lt;source&gt;:12:13: note: in instantiation of variable template specialization 'C&lt;Forward&gt;' requested here
   12 |   requires (C&lt;Key&gt;)
      |             ^
&lt;source&gt;:12:13: note: while substituting template arguments into constraint expression here
   12 |   requires (C&lt;Key&gt;)
      |             ^~~~~~
&lt;source&gt;:23:25: note: while checking constraint satisfaction for template '&lt;deduction guide for Alias&gt;&lt;Forward&gt;' required here
   23 |   using Foo1 = decltype(Alias{
      |                         ^~~~~
&lt;source&gt;:23:25: note: in instantiation of function template specialization 'HasTemplateParam&lt;Forward&gt;::&lt;deduction guide for Alias&gt;&lt;Key1&gt;' requested here
&lt;source&gt;:21:7: note: forward declaration of 'Forward'
   21 | class Forward;

It seems to be an issue in the transformed require-clause for the alias deduction guide. The require expression should be evaluated on the type Key1, rather than the outer type Forward.

hokein · 2024-04-26T10:18:36Z

OK, I can confirm that the synthesized deduction guide (and its require-clause) for the alias is correct

FunctionTemplateDecl  <deduction guide for Alias>
|-TemplateTypeParmDecl class depth 0 index 0 Key
|-ParenExpr  '<dependent type>' lvalue
| `-UnresolvedLookupExpr  '<dependent type>' lvalue (no ADL) = 'C'
|   `-TemplateArgument type 'type-parameter-0-0'
|     `-TemplateTypeParmType  'type-parameter-0-0' dependent depth 0 index 0
`-CXXDeductionGuideDecl  <deduction guide for Alias> 'auto (type-parameter-0-0) -> Foo<type-parameter-0-0>'
  `-ParmVarDecl  'type-parameter-0-0'

Up to the function overload resolution point at https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplateDeduction.cpp#L3769-L3772, everything appears to be in order. The instantiated deduction guide looks like:

CXXDeductionGuideDecl <deduction guide for Alias> 'auto (Key1) -> Foo<Key1>' implicit_instantiation
|-TemplateArgument type 'Key1'
| `-RecordType 'Key1'
|   `-CXXRecord 'Key1'
`-ParmVarDecl 'Key1'

The issue seems to be in the CheckInstantiatedFunctionTemplateConstraints(...).

shafik · 2024-04-26T20:49:34Z

If you switch Key1 and Forward it fails as expected: https://godbolt.org/z/Pbzs7E3YY

this seems to have landed after clang-18, do you know which change brought this in?

hokein · 2024-04-29T10:02:28Z

this seems to have landed after clang-18, do you know which change brought this in?

I think this is caused by the recent changes to support CTAD for alias.

OK, I can confirm that the synthesized deduction guide (and its require-clause) for the alias is correct

FunctionTemplateDecl  <deduction guide for Alias>
|-TemplateTypeParmDecl class depth 0 index 0 Key
|-ParenExpr  '<dependent type>' lvalue
| `-UnresolvedLookupExpr  '<dependent type>' lvalue (no ADL) = 'C'
|   `-TemplateArgument type 'type-parameter-0-0'
|     `-TemplateTypeParmType  'type-parameter-0-0' dependent depth 0 index 0
`-CXXDeductionGuideDecl  <deduction guide for Alias> 'auto (type-parameter-0-0) -> Foo<type-parameter-0-0>'
  `-ParmVarDecl  'type-parameter-0-0'

Turns out it was wrong, the depth of the TemplateTypeParam in the require-clause is mixed up, and it seems like clang expects it to be 1 instead of 0.

Taking on a working case (normal CTAD):

template <class D>
constexpr bool C = sizeof(D);

template <class T>
struct HasTemplateParam {
  template <class Key>
  struct Foo {
    Foo(Key);
  };

  template <class Key>
  requires (C<Key>)
  Foo(Key) -> Foo<Key>;
};

class Forward;
class Key1 {};
void kkk() {
  HasTemplateParam<Forward>::Foo k = Key1();
}

The explicit deduction guide for Foo inside the template specialization HasTemplateParam<Forward> is:

|-FunctionTemplateDecl 0x5560902690c0 <line:11:3, line:13:22> col:3 <deduction guide for Foo>
|   | |-TemplateTypeParmDecl 0x556090267b28 <line:11:13, col:19> col:19 class depth 0 index 0 Key
|   | |-ParenExpr 0x556090266d70 <line:12:12, col:19> '<dependent type>' lvalue
|   | | `-UnresolvedLookupExpr 0x556090266cf8 <col:13, col:18> '<dependent type>' lvalue (no ADL) = 'C' 0x556090245da0
|   | |   `-TemplateArgument type 'Key'
|   | |     `-TemplateTypeParmType 0x556090266ca0 'Key' dependent depth 1 index 0
|   | |       `-TemplateTypeParm 0x556090266c48 'Key'
|   | `-CXXDeductionGuideDecl 0x556090268ff8 <line:13:3, col:22> col:3 <deduction guide for Foo> 'auto (Key) -> Foo<Key>'
|   |   `-ParmVarDecl 0x556090268df8 <col:7> col:10 'Key' 'Key'
|   |     `-TemplateTypeParmType 0x556090267b80 'Key' dependent depth 0 index 0
|   |       `-TemplateTypeParm 0x556090267b28 'Key'

Note the difference in the occurrence of template parameter Key in the require expression (depth 1) and the function parameter (depth 0), even though they refer to the same template parameter.

My understanding is that this subtle difference is intentional. When clang evaluates the constraint, it requires the entire template argument list (even the outer template is instantiated). @erichkeane can you confirm?

zyn0217 · 2024-04-29T11:17:02Z

My understanding is that this subtle difference is intentional. When clang evaluates the constraint, it requires the entire template argument list (even the outer template is instantiated).

I think your understanding is correct because in TemplateDeclInstantiator::VisitTemplateTypeParmDecl, we won't update the depth of the parameter to which a constraint refers unless EvaluateConstraints is set on.

llvm-project/clang/lib/Sema/SemaTemplateInstantiate.cpp

Lines 3100 to 3104 in 55c6bda

    
           if (!EvaluateConstraints) { 
        
               Inst->setTypeConstraint(TC->getConceptReference(), 
        
                                       TC->getImmediatelyDeclaredConstraint()); 
        
               return false; 
        
           }

erichkeane · 2024-04-29T13:20:47Z

this seems to have landed after clang-18, do you know which change brought this in?

I think this is caused by the recent changes to support CTAD for alias.
OK, I can confirm that the synthesized deduction guide (and its require-clause) for the alias is correct
FunctionTemplateDecl  <deduction guide for Alias>
|-TemplateTypeParmDecl class depth 0 index 0 Key
|-ParenExpr  '<dependent type>' lvalue
| `-UnresolvedLookupExpr  '<dependent type>' lvalue (no ADL) = 'C'
|   `-TemplateArgument type 'type-parameter-0-0'
|     `-TemplateTypeParmType  'type-parameter-0-0' dependent depth 0 index 0
`-CXXDeductionGuideDecl  <deduction guide for Alias> 'auto (type-parameter-0-0) -> Foo<type-parameter-0-0>'
  `-ParmVarDecl  'type-parameter-0-0'
Turns out it was wrong, the depth of the TemplateTypeParam in the require-clause is mixed up, and it seems like clang expects it to be 1 instead of 0.

Taking on a working case (normal CTAD):
template <class D>
constexpr bool C = sizeof(D);

template <class T>
struct HasTemplateParam {
  template <class Key>
  struct Foo {
    Foo(Key);
  };

  template <class Key>
  requires (C<Key>)
  Foo(Key) -> Foo<Key>;
};

class Forward;
class Key1 {};
void kkk() {
  HasTemplateParam<Forward>::Foo k = Key1();
}
The explicit deduction guide for Foo inside the template specialization HasTemplateParam<Forward> is:
|-FunctionTemplateDecl 0x5560902690c0 <line:11:3, line:13:22> col:3 <deduction guide for Foo>
|   | |-TemplateTypeParmDecl 0x556090267b28 <line:11:13, col:19> col:19 class depth 0 index 0 Key
|   | |-ParenExpr 0x556090266d70 <line:12:12, col:19> '<dependent type>' lvalue
|   | | `-UnresolvedLookupExpr 0x556090266cf8 <col:13, col:18> '<dependent type>' lvalue (no ADL) = 'C' 0x556090245da0
|   | |   `-TemplateArgument type 'Key'
|   | |     `-TemplateTypeParmType 0x556090266ca0 'Key' dependent depth 1 index 0
|   | |       `-TemplateTypeParm 0x556090266c48 'Key'
|   | `-CXXDeductionGuideDecl 0x556090268ff8 <line:13:3, col:22> col:3 <deduction guide for Foo> 'auto (Key) -> Foo<Key>'
|   |   `-ParmVarDecl 0x556090268df8 <col:7> col:10 'Key' 'Key'
|   |     `-TemplateTypeParmType 0x556090267b80 'Key' dependent depth 0 index 0
|   |       `-TemplateTypeParm 0x556090267b28 'Key'
Note the difference in the occurrence of template parameter Key in the require expression (depth 1) and the function parameter (depth 0), even though they refer to the same template parameter.

My understanding is that this subtle difference is intentional. When clang evaluates the constraint, it requires the entire template argument list (even the outer template is instantiated). @erichkeane can you confirm?

Right, that is correct. Constraints should NOT be instantiated at all unless they are being evaluated, this is so that they can refer to changes in types that happen after instantiation (and are useful for using them in CRTP in particular).

Thus, we don't substitute them at all if at all possible (and the AST always stores the uninstantiated version). As a by-product of this, we end up having to recollect the entire set of template argument lists when we check constraints.

hokein · 2024-04-30T11:30:28Z

Thanks for the confirmation and clarification, @zyn0217, @erichkeane.

Thus, we don't substitute them at all if at all possible (and the AST always stores the uninstantiated version). As a by-product of this, we end up having to recollect the entire set of template argument lists when we check constraints.

And in the implementation, we take a shortcut and reuse the require-clause of the primary (uninstantiated) template. In this example, the require-clause of the deduction guide Foo in HasTemplateParam<Forward> is the one in the primary template HasTemplateParam.

Given the way constraints work in clang, the transformRequireClause for alias CTAD is broken.

When we synthesize the alias deduction guide from the underlying deduction guide, we "replace" all occurrences of template parameters of the underlying deduction guide with the alias RHS template arguments. The RHS arguments come from the alias template in the instantiated HasTemplateParam<Forward> (note that the template argument Key in using Alias = Foo<Key> is type-parameter-0-0 (depth:0, index: 0)). Thus, we end up having type-parameter-0-0 in the require-clause of the final alias deduction guide.

One way to fix the require-clause depth issue is to use the alias template in the primary HasTemplateParam template when doing the transform specifically for require-clause, as this alias template has original depth information.

#90961) In the clang AST, constraint nodes are deliberately not instantiated unless they are actively being evaluated. Consequently, occurrences of template parameters in the require-clause expression have a subtle "depth" difference compared to normal occurrences in places, such as function parameters. When transforming the require-clause, we must take this distinction into account. The existing implementation overlooks this consideration. This patch is to rewrite the implementation of the require-clause transformation to address this issue. Fixes #90177

llvm#90961) In the clang AST, constraint nodes are deliberately not instantiated unless they are actively being evaluated. Consequently, occurrences of template parameters in the require-clause expression have a subtle "depth" difference compared to normal occurrences in places, such as function parameters. When transforming the require-clause, we must take this distinction into account. The existing implementation overlooks this consideration. This patch is to rewrite the implementation of the require-clause transformation to address this issue. Fixes llvm#90177

hokein added clang Clang issues not falling into any other category c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 26, 2024

EugeneZelenko removed the clang Clang issues not falling into any other category label Apr 26, 2024

shafik added the confirmed Verified by a second party label Apr 26, 2024

hokein mentioned this issue Apr 29, 2024

[AST] Dump explicit template arguments for UnreslovedLookupExpr. #90466

Merged

hokein mentioned this issue May 3, 2024

[clang] CTAD alias: fix the transformation for the require-clause expr #90961

Merged

hokein closed this as completed in #90961 May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CTAD: incorrect transformed require-clause for the alias deduction guide #90177

CTAD: incorrect transformed require-clause for the alias deduction guide #90177

hokein commented Apr 26, 2024

llvmbot commented Apr 26, 2024

llvmbot commented Apr 26, 2024

hokein commented Apr 26, 2024

shafik commented Apr 26, 2024

hokein commented Apr 29, 2024

zyn0217 commented Apr 29, 2024

erichkeane commented Apr 29, 2024

hokein commented Apr 30, 2024

CTAD: incorrect transformed require-clause for the alias deduction guide #90177

CTAD: incorrect transformed require-clause for the alias deduction guide #90177

Comments

hokein commented Apr 26, 2024

llvmbot commented Apr 26, 2024

llvmbot commented Apr 26, 2024

hokein commented Apr 26, 2024

shafik commented Apr 26, 2024

hokein commented Apr 29, 2024

zyn0217 commented Apr 29, 2024

erichkeane commented Apr 29, 2024

hokein commented Apr 30, 2024