[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

Great examples.

> Check this example out: https://godbolt.org/z/rsGsM6GrM
>
>   template <class ...Ts> struct A {
>     template <class ...Us> struct B {
>         using type1 = void ((void (*...fps)(Ts, Us)));
>     };
>   };
>   using type2 = A<int, char>::B<short, bool>::type1;
>
>   TypeAliasDecl 0x55ffe8b45368 <line:8:1, col:45> col:7 type2 'A<int, char>::B<short, bool>::type1':'void ((void (*)(int, short), void (*)(char, bool)))'
>     `-ElaboratedType 0x55ffe8b452f0 'A<int, char>::B<short, bool>::type1' sugar
>       `-TypedefType 0x55ffe8b452d0 'A<int, char>::B<short, bool>::type1' sugar
>         |-TypeAlias 0x55ffe8b45258 'type1'
>         `-FunctionProtoType 0x55ffe8b451e0 'void ((void (*)(int, short), void (*)(char, bool)))' cdecl
>           |-ParenType 0x55ffe8b12150 'void' sugar
>           | `-BuiltinType 0x55ffe8ac6370 'void'
>           |-PointerType 0x55ffe8b44550 'void (*)(int, short)'
>           | `-ParenType 0x55ffe8b444f0 'void (int, short)' sugar
>           |   `-FunctionProtoType 0x55ffe8b444b0 'void (int, short)' cdecl
>           |     |-BuiltinType 0x55ffe8ac6370 'void'
>           |     |-SubstTemplateTypeParmType 0x55ffe8b44310 'int' sugar
>           |     | |-TemplateTypeParmType 0x55ffe8b11440 'Ts' dependent contains_unexpanded_pack depth 0 index 0 pack
>           |     | | `-TemplateTypeParm 0x55ffe8b113c0 'Ts'
>           |     | `-BuiltinType 0x55ffe8ac6410 'int'
>           |     `-SubstTemplateTypeParmType 0x55ffe8b443c0 'short' sugar
>           |       |-TemplateTypeParmType 0x55ffe8b11960 'Us' dependent contains_unexpanded_pack depth 1 index 0 pack
>           |       | `-TemplateTypeParm 0x55ffe8b118d8 'Us'
>           |       `-BuiltinType 0x55ffe8ac63f0 'short'
>           `-PointerType 0x55ffe8b450c0 'void (*)(char, bool)'
>             `-ParenType 0x55ffe8b45060 'void (char, bool)' sugar
>               `-FunctionProtoType 0x55ffe8b447d0 'void (char, bool)' cdecl
>                 |-BuiltinType 0x55ffe8ac6370 'void'
>                 |-SubstTemplateTypeParmType 0x55ffe8b44630 'char' sugar
>                 | |-TemplateTypeParmType 0x55ffe8b11440 'Ts' dependent contains_unexpanded_pack depth 0 index 0 pack
>                 | | `-TemplateTypeParm 0x55ffe8b113c0 'Ts'
>                 | `-BuiltinType 0x55ffe8ac63b0 'char'
>                 `-SubstTemplateTypeParmType 0x55ffe8b446e0 'bool' sugar
>                   |-TemplateTypeParmType 0x55ffe8b11960 'Us' dependent contains_unexpanded_pack depth 1 index 0 pack
>                   | `-TemplateTypeParm 0x55ffe8b118d8 'Us'
>                   `-BuiltinType 0x55ffe8ac6390 'bool'

If it were as simple as the above case the Resugarer TreeTransform could say keep a map of `TemplateTypeParmType`s to current pack indices, and iterate those during each each `TransformSubstTemplateTypeParmType` call, but...

> And in fact, a given parameter pack might be referenced more than one time in a given pack expansion pattern:
>
>   template <class ...Ts> struct A {
>     template <class ...Us> struct B {
>         using type1 = void ((void (*...fps)(Ts, Ts, Us)));
>     };
>   };
>
> Ie those two Ts are referencing the same argument within the pack, we can't confuse that with an expansion.
>
> So think also about a case like the above, but where you are performing the expansion just one time. It doesn't look like to me you can figure that out from what Clang leaves behind in the AST at the moment.

...the different `Ts` are the same `TemplateTypeParmType`, so that wouldn't work.  You're right, more info is needed.

> We may need to create a new AST node, which is like a Subst variant of a PackExpansionType, at the expansion loci.

Or, maybe `TemplateTypeParmType` could store a number that would make it unique for each *instance* of it in a given expansion?  Or just `SubstTemplateTypeParmType` could store this number in addition to its `TemplateTypeParmType`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128113/new/

https://reviews.llvm.org/D128113