[PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

Jason Merrill jason@redhat.com
Fri Mar 20 21:53:40 GMT 2020


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.

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

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?

> +  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.

Jason



More information about the Gcc-patches mailing list