This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++ PATCH] Fix ICE in unify with variadic templates (PRs c++/32565, c++/33943)


On Nov 19, 2007 4:10 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Nov 15, 2007 at 07:40:42AM -0500, Jakub Jelinek wrote:
> > As shown on these I believe valid testcases, when unifying
> > BOUND_TEMPLATE_TEMPLATE_PARMs, we really can't assume if one {parm,arg}vec
> > argument is a parameter pack, the other will be as well.  Fixed by using
> > expand_template_argument_pack on both vectors so that we unify corresponding
> > parameters.
>
> This updated patch also handles PACK_EXPANSION_P if present at the end of
> this way expanded parmvec.

Thanks for working on this.

> With this PR33943 no longer ICEs, the errors are
> now:
> /usr/src/gcc/gcc/testsuite/g++.dg/cpp0x/variadic86.C:18: error: cannot expand 'U ...' into a fixed-length argument list
> /usr/src/gcc/gcc/testsuite/g++.dg/cpp0x/variadic86.C:18: error: ambiguous class template instantiation for 'struct baz<foo<int, short int> >'
> /usr/src/gcc/gcc/testsuite/g++.dg/cpp0x/variadic86.C:12: error: candidates are: struct baz<T<U ...> >
> /usr/src/gcc/gcc/testsuite/g++.dg/cpp0x/variadic86.C:16: error:                 struct baz<T<U, V ...> >
> /usr/src/gcc/gcc/testsuite/g++.dg/cpp0x/variadic86.C:18: error: aggregate 'baz<foo<int, short int> > b1' has incomplete type and cannot be defined
> /usr/src/gcc/gcc/testsuite/g++.dg/cpp0x/variadic86.C:19: error: cannot expand 'U ...' into a fixed-length argument list
> (though they aren't issued by the unify with BOUND_TEMPLATE_TEMPLATE_PARM
> that has been changed, but later on).  Now whether the errors are
> correct or not I don't know.

Hmmm, that's not quite right. This should be valid code, with baz<
foo<int, short> > matching the first partial specialization and baz<
bar<int, short> > matching the second. I think the fundamental problem
is that this call in the BOUND_TEMPLATE_TEMPLATE_PARM case of unify()
does the wrong thing when variadics are involved:

	    if (coerce_template_parms (argtmplvec, parmvec,
				       TYPE_TI_TEMPLATE (parm),
				       tf_none,
				       /*require_all_args=*/true,
				       /*use_default_args=*/false)
		== error_mark_node)
	      return 1;

That call is really meant to check that the parameters of the template
template parameter match the template parameters of the argument; the
require_all_args and !use_default_args settings make sure that we're
checking for exact matches in the template parameter lists. The
problem with variadic templates is that we don't end up looking for an
exact match, because we're still allowing zero or more arguments to
match a pack expansion in coerce_template_parms.To make things more
concrete, here's an example from PR c++/32565:

  template<typename...> struct A1;
  template<template<int...> class T> struct A1<T<0> > {};
  template<int> struct B1 {};
  A1<B1<0> > a1;

This code is ill-formed, because T in the partial specialization of A1
can only refer to a template that contains a single template type
parameter pack; B1, on the other hand, contains a single template type
parameter (that is not a pack). So T cannot unify to B1, and we end up
picking the general template for A1, which has no definition.

The straw-man patch below replaces the coerce_template_parms call with
a call to coerce_template_template_parms, which I think more
accurately describes what we want. Now, things get more interesting...
this patch actually tightens up checking a bit in non-variadics cases.
For example, the following example is ill-formed, but we accept it:

  template<typename T, template<T> class C>
  void f(T, C<5>);

  template<int N> struct X {};
  void g() {
    f(5l, X<5>());
  }

We deduce T=long and C=X, then accept this code, but this is wrong: C
can't bind to X, because in f<long>, C has a non-type template
parameter with type 'long', whereas 'X' has a template parameter of
type 'int'. With the patch below, we correctly reject the example.

Now, the dark side of this patch is that it is checking the bindings
of template-template parameters too early. Twisting the example above
just a little bit, we have the following well-formed example:

  template<typename T, template<T> class C>
  void f(C<5>, T);

  template<int N> struct X {};
  void g() {
    f(X<5>(), 5);
  }

Without my patch, we accept this code. With my patch, we try to check
that C can bind to X as soon as we see X<5> in unification. However,
at this point we don't have a binding for the type 'T' (and we're not
allowed to deduce one from the non-type template argument). So, we
fail to bind C to X, and reject this well-formed code.

So, to try to summarize:

  - I believe your patch to the C++ FE is correct, but there's a
little more work to be done to get the semantics right.

  - The use of coerce_template_parms in the
BOUND_TEMPLATE_TEMPLATE_PARM case in unify() isn't exactly correct;
it's a heuristic that works in most cases, but fails to reject some
incorrect code. We could probably construct a case where it picks the
wrong partial specialization somewhere.

  - coerce_template_template_parms is the right way to check whether
we can unify a template template parameter to a template, but it has
to be used after we have deduced all of the template arguments, e.g.,
in get_class_bindings and type_unification_real.


As an aside: we're putting together a proposal that loosens the
matching semantics for template template parameters with parameter
packs in their template parameter lists. It would, among other things,
make variadic84.C, variadic85.C, and variadic86.C all valid C++0x
code. However, that patch affects mainly the
coerce_template_template_parms definition, and should not conflict
much with this patch.

  - Doug

Index: pt.c
===================================================================
--- pt.c        (revision 130204)
+++ pt.c        (working copy)
@@ -5187,12 +5187,15 @@ coerce_template_parms (tree parms,
                  template parameter pack (if it were, we would have
                  handled it above), we're trying to expand into a
                  fixed-length argument list.  */
-              if (TREE_CODE (arg) == EXPR_PACK_EXPANSION)
-                error ("cannot expand %<%E%> into a fixed-length "
-                       "argument list", arg);
-              else
-                error ("cannot expand %<%T%> into a fixed-length "
-                       "argument list", arg);
+              if (complain & tf_error)
+                {
+                  if (TREE_CODE (arg) == EXPR_PACK_EXPANSION)
+                    error ("cannot expand %<%E%> into a fixed-length "
+                           "argument list", arg);
+                  else
+                    error ("cannot expand %<%T%> into a fixed-length "
+                           "argument list", arg);
+                }
              return error_mark_node;
             }
         }
@@ -12709,13 +12712,14 @@ unify (tree tparms, tree targs, tree par
              Here, if Lvalue_proxy is permitted to bind to View, then
              the global operator+ will be used; if they are not, the
              Lvalue_proxy will be converted to float.  */
-           if (coerce_template_parms (argtmplvec, parmvec,
-                                      TYPE_TI_TEMPLATE (parm),
-                                      tf_none,
-                                      /*require_all_args=*/true,
-                                      /*use_default_args=*/false)
-               == error_mark_node)
-             return 1;
+            if (!coerce_template_template_parms
+                  (DECL_INNERMOST_TEMPLATE_PARMS (TYPE_TI_TEMPLATE (parm)),
+                   argtmplvec,
+                   tf_none,
+                   TYPE_TI_TEMPLATE (parm),
+                   targs))
+              return 1;
+

            /* Deduce arguments T, i from TT<T> or TT<i>.
               We check each element of PARMVEC and ARGVEC individually


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]