On 3/20/20 3:53 PM, Jason Merrill wrote:
> On 3/19/20 7:55 PM, Martin Sebor wrote:
>> On 3/18/20 9:07 PM, Jason Merrill wrote:
>>> On 3/12/20 6:38 PM, Martin Sebor wrote:
>> ...
>>>> +     declarations of a class from its uses doesn't work for type 
>>>> aliases
>>>> +     (as in using T = class C;).  */
>>>
>>> Good point.  Perhaps we could pass flags to 
>>> cp_parser_declares_only_class_p and have it return false if 
>>> CP_PARSER_FLAGS_TYPENAME_OPTIONAL, since that is set for an alias but 
>>> not for a normal type-specifier.
>>
>> I wondered if there was a way to identify that we're dealing with
>> an alias.  CP_PARSER_FLAGS_TYPENAME_OPTIONAL is set not just for
>> those but also for template declarations (in
>> cp_parser_single_declaration) but I was able to make it work by
>> tweaking cp_parser_type_specifier.  It doesn't feel very clean
>> (it seems like either the bit or all of cp_parser_flags could be
>> a member of the parser class or some subobject of it) but it does
>> the job.  Thanks for pointing me in the right direction!
> 
> Hmm, true, relying on that flag is probably too fragile.  And now that I 
> look closer, I see that we already have is_declaration in 
> cp_parser_elaborated_type_specifier, we just need to check that before 
> cp_parser_declares_only_class_p like we do earlier in the function.

I changed it to use is_declaration instead.

>>>> +  if (!decl_p && !def_p && TREE_CODE (decl) == TEMPLATE_DECL)
>>>>      {
>>>> +      /* When TYPE is the use of an implicit specialization of a 
>>>> previously
>>>> +     declared template set TYPE_DECL to the type of the primary 
>>>> template
>>>> +     for the specialization and look it up in CLASS2LOC below.  For 
>>>> uses
>>>> +     of explicit or partial specializations TYPE_DECL already 
>>>> points to
>>>> +     the declaration of the specialization.  */
>>>> +      type_decl = specialization_of (type_decl);
>>>
>>> Here shouldn't is_use be true?
>>
>> If it were set to true here we would find the partial specialization
>> corresponding to its specialization in the use when what we want is
>> the latter.  As a result, for the following:
>>
>>    template <class>   struct S;
>>    template <class T> struct S<T*>;
>>
>>    extern class  S<int*> s1;   // expect -Wmismatched-tags
>>    extern struct S<int*> s2;
>>
>> we'd end up with a warning for s2 pointing to the instantiation of
>> s1 as the "guiding declaration:"
>>
>> z.C:5:15: warning: ‘template<class T> struct S<T*>’ declared with a 
>> mismatched class-key ‘struct’ [-Wmismatched-tags]
>>      5 | extern struct S<int*> s2;
>>        |               ^~~~~~~
>> z.C:5:15: note: remove the class-key or replace it with ‘class’
>> z.C:4:15: note: ‘template<class T> struct S<T*>’ first declared as 
>> ‘class’ here
>>      4 | extern class  S<int*> s1;   // expect -Wmismatched-tags
>>        |               ^~~~~~~
> 
> I found this puzzling and wanted to see why that would be, but I can't 
> reproduce it; compiling with -Wmismatched-tags produces only

I'm not sure what you did differently.  With the patch (the last one
or the one in the attachment) we get the expected warning below.

> 
> wa2.C:4:17: warning: ‘S<T*>’ declared with a mismatched class-key 
> ‘class’ [-Wmismatched-tags]
>      4 |   extern class  S<int*> s1;   // expect -Wmismatched-tags
>        |                 ^~~~~~~
> wa2.C:4:17: note: remove the class-key or replace it with ‘struct’
> wa2.C:2:29: note: ‘S<T*>’ first declared as ‘struct’ here
>      2 |   template <class T> struct S<T*>;
> 
> So the only difference is whether we talk about S<T*> or S<int*>.  I 
> agree that the latter is probably better, which is what you get without 
> my suggested change.  But since specialization_of does nothing if is_use 
> is false, how about removing the call here and removing the is_use 
> parameter?

Sure.

> 
>> +  if (tree spec = most_specialized_partial_spec (ret, tf_none))
>> +    if (spec != error_mark_node)
>> +      ret = TREE_VALUE (spec);
> 
> I think you want to take the TREE_TYPE of the template here, so you 
> don't need to do it here:
> 
>> +      tree pt = specialization_of (TYPE_MAIN_DECL (type), true);
>> +      if (TREE_CODE (pt) == TEMPLATE_DECL)
>> +       pt = TREE_TYPE (pt);
>> +      pt = TYPE_MAIN_DECL (pt);
> 
> And also, since it takes a TYPE_DECL, it would be better to return 
> another TYPE_DECL rather than a _TYPE, especially since the only caller 
> immediately extracts a TYPE_DECL.

Okay.  Attached is an revised patch with these changes.

Martin