[PATCH] Canonicalize the variadic template alias with multiple ellipsis.

[Richard Smith]

On Mon, Jul 7, 2014 at 6:01 AM, Logan Chien <tzuhsiang.chien at gmail.com>
wrote:

> Ping?
>
> This patch tries to fix an assertion failure related to the template alias
> with two (or more) parameter packs.  For example,
>
>
> template <typename... T> struct tuple;
> template <typename... T> struct extract_;
> template <typename... T> using extract = typename extract_<T...>::type;
>
> template <typename... A, typename... B>
> inline auto test(tuple<A...>&& xs, B&&... ys) -> extract<A&&..., B...> { }
>
> Please have a look, and feel free to let me know if there is any problem.
> Thanks!
>

Testcase reduced to:

template <typename... T> struct extract_;
template <typename... T> using extract = typename extract_<T...>::type;
template <typename... A, typename B> void test(extract<A..., B>);

Sincerely,
> Logan
>
>
> On Mon, Jun 30, 2014 at 1:10 AM, Logan Chien <tzuhsiang.chien at gmail.com>
> wrote:
>
>> Hi rsmith,
>>
>> If the template has two variadic formal parameters, then the type
>> might not be canonical.  We have call ASTContext.getCanonicalType()
>> to canonicalize the type; otherwise, an assertion failure will be
>> raised.
>>
>> This patch fix this issue by adding getCanonicalType() in
>> TransformTemplateTypeParmType.
>>
>
This seems to be blindly addressing a symptom rather than going after the
actual problem.

When we match the template arguments <A..., B> against the parameters of
'extract', we form <template-parameter-0-0..., B>. Note we canonicalize the
first but not the second. This is because Sema::CheckTemplateArgumentList
does no checking for template arguments after the first pack expansion:

      // If we just saw a pack expansion, then directly convert the
remaining
      // arguments, because we don't know what parameters they'll match up
      // with.

This means we fail to check or canonicalize any arguments after a pack
expansion. With that knowledge in hand, here's a testcase for the same bug
that your patch won't fix:

  template <typename... T> struct extract_;
  template <typename... T> using extract = typename extract_<T...>::type;
  template <typename... A, typename B> void test(extract<A..., 123>);

The right fix here would be for CheckTemplateArgumentList to distinguish
between the case where more substitution and expansion will be performed
before the template is instantiated, and the case where it actually needs
to match up the parameters and arguments properly.

And then in a case like this:

  template<typename ...T> struct A {
    template<T ...U, typename V> using B = V;
    void f(B<T()..., int>);
  };

(for which we currently reject-valid), if CheckTemplateArgumentList is
called for an alias template, it needs to tell its caller to create a
dependent type rather than performing a substitution.

http://reviews.llvm.org/D4343
>>
>> Files:
>>   lib/Sema/SemaTemplateInstantiate.cpp
>>   test/SemaTemplate/alias-templates.cpp
>>
>> Index: lib/Sema/SemaTemplateInstantiate.cpp
>> ===================================================================
>> --- lib/Sema/SemaTemplateInstantiate.cpp
>> +++ lib/Sema/SemaTemplateInstantiate.cpp
>> @@ -1376,7 +1376,7 @@
>>      assert(Arg.getKind() == TemplateArgument::Type &&
>>             "Template argument kind mismatch");
>>
>> -    QualType Replacement = Arg.getAsType();
>> +    QualType Replacement =
>> getSema().Context.getCanonicalType(Arg.getAsType());
>>
>>      // TODO: only do this uniquing once, at the start of instantiation.
>>      QualType Result
>> Index: test/SemaTemplate/alias-templates.cpp
>> ===================================================================
>> --- test/SemaTemplate/alias-templates.cpp
>> +++ test/SemaTemplate/alias-templates.cpp
>> @@ -201,3 +201,17 @@
>>    template <typename T, typename U, typename V>
>>    using derived2 = ::PR16904::base<T, U>::template derived<V>; //
>> expected-error {{expected a type}} expected-error {{expected ';'}}
>>  }
>> +
>> +namespace VariadicTemplateAlias {
>> +  template <typename... T> struct tuple;
>> +  template <typename... T> struct extract_;
>> +
>> +  // Note: Both the template alias and the concatenation of variadic
>> template
>> +  // arguments A and B are required to trigger the assertion failure.
>> +
>> +  template <typename... T>
>> +  using extract = typename extract_<T...>::type;
>> +
>> +  template <typename... A, typename... B>
>> +  inline auto test(tuple<A...>&& xs, B&&... ys) -> extract<A&&..., B...>
>> { }
>> +}
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140708/1f5ab6c7/attachment.html>