Re: [PATCH] PR c++/26693

[Dodji Seketeli <dodji at redhat dot com>]

Jason Merrill a Ãcrit :
> Dodji Seketeli wrote:
> > 2/ When tsubst() encounters a typedef variant type, it tries to 
> > "reuse" its specialization, instead of tsubst-ing its main variant 
> > type. This works great. But when the main variant type is a 
> > TYPENAME_TYPE, I think we need to make sure the main variant type can 
> > be properly tsubst-ed before attempting to reuse the specialization of 
> > the typedef variant type.
>
> This sounds wrong to me: if tsubsting the TYPENAME_TYPE was going to be 
> a problem, we ought to get that error when we instantiate the class of 
> which the typedef is a member.

In this code example:
     1  template<typename> struct A
     2  {
     3    typedef int X;
     4    static const int i = 0;
     5  };
     6
     7  template<typename> struct B
     8  {
     9    B(const B&); // { dg-message "candidate" }
    10    typedef typename A<char[A<B>::i]>::X Y;
    11    template<typename T> B(T, Y); // { dg-error "call" }
    12  };
    13
    14  B<int> b(0,0);

There are two sets of error g++ should give us.

The first one is:
: In instantiation of 'B<int>':
:14:   instantiated from here
:10: warning: ISO C++ forbids zero-size array
: In instantiation of 'B<int>':
:14:   instantiated from here

I think it's the one you are talking about. We get that one, even if I 
don't add what I was refering to in the point 2/ of my previous email.

The second set of error g++ should give us is:
:11: error: no matching function for call to 'B<int>::B(int, int)'
:9: note: candidates are: B< <template-parameter-1-1> >::B(const B< 
<template-parameter-1-1> >&) [with <template-parameter-1-1> = int]

This second set of error is missing if I don't handle the particular 
case of typedef variants of TYPENAME_TYPE as was talking about in 2/.

I believe this is one is generated because during instantiation of the 
last parameter (namely the Y typedef) of B<int>::B(int, int), the 
function tsubst() receives a typedef variant of a TYPENAME_TYPE.

Before this patch, the condition:
 if (TYPE_P (t)
      && TYPE_NAME (t)
      && TYPE_NAME (t) != TYPE_MAIN_DECL (t))
    {

was not true, because t was not a typedef variant. So t was handled by 
the TYPENAME_TYPE case that comes later in that function and an error 
was raised.

But after the patch, this condition becomes true and later this if:

    if (DECL_CLASS_SCOPE_P (decl)
        && CLASSTYPE_TEMPLATE_INFO (DECL_CONTEXT (decl))
        && uses_template_parms (DECL_CONTEXT (decl)))
      {
        tree tmpl = most_general_template (DECL_TI_TEMPLATE (decl));
        tree gen_args = tsubst (DECL_TI_ARGS (decl), args, complain, 
in_decl);
        r = retrieve_specialization (tmpl, gen_args, false);
      }

becomes true as well, and later r is returned. This code path doesn't 
raise the error that was raised previously.

So my understanding is the typedef variant of TYPENAME_TYPE triggered a 
different code path that lead to some errors not being raised. I might 
be missing something at this point, but I can't see what :-(.

> > +      if (DECL_IS_BUILTIN (x)
> > +          || (TREE_TYPE (x) != error_mark_node
> > +          && TYPE_NAME (type) != x
> > +          /* We don't want to copy the type when all we're
> > +             doing is making a TYPE_DECL for the purposes of
> > +             inlining.  */
> > +          && (!TYPE_NAME (type)
> > +              || TYPE_NAME (type) != DECL_ABSTRACT_ORIGIN (x))))
> > +        clone_underlying_type (x);
>
> This is wrong: if DECL_IS_BUILTIN (x) we don't want to create a typedef 
> variant, we just want to set TYPE_NAME (type) to x.

Ah. I did this because clone_underlying_type handles that case of 
DECL_IS_BUILTING (x). I thought duplicating that here would not be 
desirable. So yes, the initial clone_underlying_type does more than just 
creating a typedef variant.

Thanks,

Dodji.