Require canonical type comparison for typedefs again.

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:

~=~
~=~

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

Thanks.

On 09/28/2010 07:25 AM, Dodji Seketeli wrote:
> 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.

Right.

> Hence A and A' will is considered structurally
> equivalent. No?

I don't see why; the template parms of A and A' should have different 
canonical types.

> 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?

The former.  Or rather, that fixup_template_parms_canonical_types should 
work this way.  Incidentally, I notice that it still does two passes 
over the parm list.

> 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:

Looks good.

Jason

Jason Merrill <jason@redhat.com> writes:

> On 09/28/2010 07:25 AM, Dodji Seketeli wrote:
>> 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.
>
> Right.
>
>> Hence A and A' will is considered structurally
>> equivalent. No?
>
> I don't see why; the template parms of A and A' should have different
> canonical types.

Ah, I wasn't clear. During the fixup, at some point, A and A' have the
same template parm actually. Because A's template parm (let's call it T)
saw its canonical type updated, and then, T got substituted into A to
create A'.

But then A kept its canonical type, even thoug T's canonical type
changed. And A' has another canonical type, different from the one of A.

>
>> 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?
>
> The former.  Or rather, that fixup_template_parms_canonical_types
> should work this way.

Okay, thanks, working on that now.

>  Incidentally, I notice that it still does two
> passes over the parm list.

Oops, okay I will address that.

On 09/29/2010 03:05 PM, Dodji Seketeli wrote:
> Ah, I wasn't clear. During the fixup, at some point, A and A' have the
> same template parm actually. Because A's template parm (let's call it T)
> saw its canonical type updated

Well, that's the problem then.  You shouldn't be modifying the template 
parms that you started out with, you should be replacing them with new ones.

Jason