[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