Re: [C++ Patch] PR 34776

[Mark Mitchell <mark at codesourcery dot com>]

Paolo Carlini wrote:
> Hi,
>> It's not a requirement for acceptance, but I think it would be even
>> better to also (a) add a "gcc_assert (CLASS_TYPE_P (type))" to
>> constructor_name_p, and (b) and change the documentation for
>> constructor_name_p to say:
>>
>>  ... TYPE, which must be a class type.
>>
>> Those changes are pre-approved, provided they pass testing.
>>   
> Thanks. Actually, testing these changes revealed a nit: we can also have
> TEMPLATE_TYPE_PARM, for example. I think IS_AGGR_TYPE is the proper
> concept, right? I tested the below, on x86_64_linux.

I think IS_AGGR_TYPE is probably the right predicate, but:

!    which must be an aggregate type.  */

is not the right comment -- because IS_AGGR_TYPE is misnamed.  (There's
a comment on IS_AGGR_TYPE that explains that it should be thought of as
MAYBE_CLASS_TYPE_P.  Renaming the macro is a fine change -- for Stage 1
or 2.)  I still think "class type" is the right thing to say in the
comment.  In particular, it doesn't make sense to pass in an array type
-- which is an aggregate type, but not a class type.

Also:

!   if (IS_AGGR_TYPE (scope) && constructor_name_p (name, scope))

is an interesting case.  I thought about this a little while looking at
the previous patch.  In particular, using IS_AGGR_TYPE means we will
check -- and issue an error about -- "using T::T;" where "T" is a
template type parameter.  If we were to use CLASS_TYPE_P we would still
issue an error at the point of instantiation, but not at the point of
declaration.  Your version is OK, I think; it doesn't make sense to
allow the using declaration if no instantiation of the template is valid.

So, patch approved with change to comment mentioned above.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713