Re: Require canonical type comparison for typedefs again.

[Dodji Seketeli <dodji at redhat dot com>]

Jason Merrill <jason@redhat.com> writes:

[...]
>>>> +  if (total_num_parms == 0)
>>>> +    return NULL_TREE;
>>>
>>> Since you're already treating parms from a list of unknown length as
>>> distinct from parms from a list of known length, we can give them a
>>> canonical type, too.
>>
>> Doing that doesn't seem to work. The problem is not with the comparison
>> of type template parms but rather with the comparison of dependent types
>> created during the template parms parsing.
>>
>> If type template parms don't require structural comparison then a type A
>> (created during template parms parsing) dependent on those parms will
>> have a canonical type.
>
> Yep.
>
>> The fixup then creates a type A' that is
>> structurally equivalent to A, but has a different canonical type.
>
> It shouldn't be structurally equivalent, since A's template parms come
> from a list of non-0 length, which is not true of A'.
>

I must be missing something. In structural_comptype, in the RECORD_TYPE
case, comp_template_args is called and I think this eventually compares
the template parms of A and A' using their canonical types, even though
we are calling from structural_comptype and are supposed to do
structural comparison. Hence A and A' will is considered structurally
equivalent. No?

>> * Template parm level reduction.
>>
>> When at least one of the parms is itself a template [like in
>> testsuite/g++.old-deja/g++.pt/nttp1.C] then at some point we end up in
>> tsubst (at least) with an argument set that has a depth that is less
>> than the depth of the parm of the template template parm we are
>> susbtituting into.
>
> Rather, the problem comes when we don't have any argument at all; in
> that case we reduce the level.
>
>> My understanding is that the tsubsting code then thinks that we are
>> doing the substitution for the purpose of instantiation and then reduces
>> the level of the resulting parms.
>
> Partial instantiation, yes.
>
> I would expect that when we get to this point we've already done the
> fixup for these parms, so that they don't need to be substituted
> directly; we only need to fixup the type of any non-type parms (and so
> on recursively for template template parms).  That ought to avoid this
> problem.

Just to be sure I understand; do you mean I should write a function that
recursively does type substitution for non-type parms (only) of template
template parms and use that in the fixup? Or does such a function exists
already?

[...]

>
>> Incidentally I noticed that the tf_no_class_instantiation is not used by
>> any client code anymore. Would you accept a patchlet to remove it?
>
> Yep.
>
>> During unification of C<5>  with X<5>  the argument 5 of X is converted
>> into the type of the parameter of C, i.e 5 is converted into typename
>> metafun<T>::type. convert_template_argument then tries to substitute
>> into typename metafun<T>::type. This should just create another
>> equivalent typename becase metafun<T>  is dependent and thus non
>> complete. But when tsubst calls make_typename_type, the later fails to
>> detect that metafun<T>  is dependent [because processing_template_decl is
>> 0] and goes crazy.
>
> Why don't we have an argument for T by this point?

Good question. I seems T's argument is "lost" somehow even in current
trunk without my patch. I think what happens is that unify (in the
BOUND_TEMPLATE_TEMPLATE_PARM case) calls coerce_template_parms with an
argvec that is just the innermost template arguments of C<5>, which is
{5}. I think it should call it with the full set of template arguments
including the arguments deduced so far i.e, {{5}, {5}}. The depth of
that full set would be 2. The first level of arguments is the argument
for T (the first {5}), and the second set (the second {5}) would be
to unified with "metafun<5>::type" to deduce "int".

This hunk -- similar to what is done in lookup_template_class in the
"if (DECL_TEMPLATE_TEMPLATE_PARM_P (templ))" branch -- seems to fix the
issue:

~=~
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index e060cc7..bf13978 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14914,6 +14914,7 @@ unify (tree tparms, tree targs, tree parm, tree arg, int strict)
 	  {
 	    tree parmvec = TYPE_TI_ARGS (parm);
 	    tree argvec = INNERMOST_TEMPLATE_ARGS (TYPE_TI_ARGS (arg));
+	    tree full_argvec = add_to_template_args (targs, argvec);
 	    tree parm_parms 
               = DECL_INNERMOST_TEMPLATE_PARMS
 	          (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (parm));
@@ -14948,7 +14949,7 @@ unify (tree tparms, tree targs, tree parm, tree arg, int strict)
 	      the global operator+ will be used; if they are not, the
 	      Lvalue_proxy will be converted to float.  */
 	    if (coerce_template_parms (parm_parms,
-                                       argvec,
+                                       full_argvec,
 				       TYPE_TI_TEMPLATE (parm),
 				       tf_none,
 				       /*require_all_args=*/true,
~=~

I have removed the use of use_template_parms in make_typename_type from
make_typename_type thus. Does this make more sense?

Thanks.
-- 
	Dodji