On 3/12/20 11:03 AM, Martin Sebor wrote:
> On 3/11/20 3:30 PM, Martin Sebor wrote:
>> On 3/11/20 2:10 PM, Jason Merrill wrote:
>>> On 3/11/20 12:57 PM, Martin Sebor wrote:
>>>> On 3/9/20 6:08 PM, Jason Merrill wrote:
>>>>> On 3/9/20 5:39 PM, Martin Sebor wrote:
>>>>>> On 3/9/20 1:40 PM, Jason Merrill wrote:
>>>>>>> On 3/9/20 12:31 PM, Martin Sebor wrote:
>>>>>>>> On 2/28/20 1:24 PM, Jason Merrill wrote:
>>>>>>>>> On 2/28/20 12:45 PM, Martin Sebor wrote:
>>>>>>>>>> On 2/28/20 9:58 AM, Jason Merrill wrote:
>>>>>>>>>>> On 2/24/20 6:58 PM, Martin Sebor wrote:
>>>>>>>>>>>> -Wredundant-tags doesn't consider type declarations that are 
>>>>>>>>>>>> also
>>>>>>>>>>>> the first uses of the type, such as in 'void f (struct S);' and
>>>>>>>>>>>> issues false positives for those.  According to the reported 
>>>>>>>>>>>> that's
>>>>>>>>>>>> making it harder to use the warning to clean up LibreOffice.
>>>>>>>>>>>>
>>>>>>>>>>>> The attached patch extends -Wredundant-tags to avoid these 
>>>>>>>>>>>> false
>>>>>>>>>>>> positives by relying on the same class_decl_loc_t::class2loc 
>>>>>>>>>>>> mapping
>>>>>>>>>>>> as -Wmismatched-tags.  The patch also somewhat improves the 
>>>>>>>>>>>> detection
>>>>>>>>>>>> of both issues in template declarations (though more work is 
>>>>>>>>>>>> still
>>>>>>>>>>>> needed there).
>>>>>>>>>>>
>>>>>>>>>>>> +         a new entry for it and return unless it's a 
>>>>>>>>>>>> declaration
>>>>>>>>>>>> +         involving a template that may need to be diagnosed by
>>>>>>>>>>>> +         -Wredundant-tags.  */
>>>>>>>>>>>>        *rdl = class_decl_loc_t (class_key, false, def_p);
>>>>>>>>>>>> -      return;
>>>>>>>>>>>> +      if (TREE_CODE (decl) != TEMPLATE_DECL)
>>>>>>>>>>>> +        return;
>>>>>>>>>>>
>>>>>>>>>>> How can the first appearance of a class template be redundant?
>>>>>>>>>>
>>>>>>>>>> I'm not sure I correctly understand the question.  The comment 
>>>>>>>>>> says
>>>>>>>>>> "involving a template" (i.e., not one of the first declaration of
>>>>>>>>>> a template).  The test case that corresponds to this test is:
>>>>>>>>>>
>>>>>>>>>>    template <class> struct S7 { };
>>>>>>>>>>    struct S7<void> s7v;  // { dg-warning "\\\[-Wredundant-tags" }
>>>>>>>>>>
>>>>>>>>>> where DECL is the TEPLATE_DECL of S7<void>.
>>>>>>>>>>
>>>>>>>>>> As I mentioned, more work is still needed to handle templates 
>>>>>>>>>> right
>>>>>>>>>> because some redundant tags are still not diagnosed.  For 
>>>>>>>>>> example:
>>>>>>>>>>
>>>>>>>>>>    template <class> struct S7 { };
>>>>>>>>>>    template <class T>
>>>>>>>>>>    using U = struct S7<T>;   // missing warning
>>>>>>>>>
>>>>>>>>> When we get here for an instance of a template, it doesn't make 
>>>>>>>>> sense to treat it as a new type.
>>>>>>>>>
>>>>>>>>> If decl is a template and type_decl is an instance of that 
>>>>>>>>> template, do we want to (before the lookup) change type_decl to 
>>>>>>>>> the template or the corresponding generic TYPE_DECL, which 
>>>>>>>>> should already be in the table?
>>>>>>>>
>>>>>>>> I'm struggling with how to do this.  Given type (a RECORD_TYPE) and
>>>>>>>> type_decl (a TEMPLATE_DECL) representing the use of a template, how
>>>>>>>> do I get the corresponding template (or its explicit or partial
>>>>>>>> specialization) in the three cases below?
>>>>>>>>
>>>>>>>>    1) Instance of the primary:
>>>>>>>>       template <class> class A;
>>>>>>>>       struct A<int> a;
>>>>>>>>
>>>>>>>>    2) Instance of an explicit specialization:
>>>>>>>>       template <class> class B;
>>>>>>>>       template <> struct B<int>;
>>>>>>>>       class B<int> b;
>>>>>>>>
>>>>>>>>    3) Instance of a partial specialization:
>>>>>>>>       template <class> class C;
>>>>>>>>       template <class T> struct C<T*>;
>>>>>>>>       class C<int*> c;
>>>>>>>>
>>>>>>>> By trial and (lots of) error I figured out that in both (1) and 
>>>>>>>> (2),
>>>>>>>> but not in (3), TYPE_MAIN_DECL (TYPE_TI_TEMPLATE (type)) returns
>>>>>>>> the template's type_decl.
>>>>>>>>
>>>>>>>> Is there some function to call to get it in (3), or even better,
>>>>>>>> in all three cases?
>>>>>>>
>>>>>>> I think you're looking for most_general_template.
>>>>>>
>>>>>> I don't think that's quite what I'm looking for.  At least it doesn't
>>>>>> return the template or its specialization in all three cases above.
>>>>>
>>>>> Ah, true, that function stops at specializations.  Oddly, I don't 
>>>>> think there's currently a similar function that looks through them. 
>>>>> You could create one that does a simple loop through 
>>>>> DECL_TI_TEMPLATE like is_specialization_of.
>>>>
>>>> Thanks for the tip.  Even with that I'm having trouble with partial
>>>> specializations.  For example in:
>>>>
>>>>    template <class>   struct S;
>>>>    template <class T> class S<const T>;
>>>>    extern class  S<const int> s1;
>>>>    extern struct S<const int> s2;  // expect -Wmismatched-tags
>>>>
>>>> how do I find the declaration of the partial specialization when given
>>>> the type in the extern declaration?  A loop in my find_template_for()
>>>> function (similar to is_specialization_of) only visits the implicit
>>>> specialization S<const int> (i.e., its own type) and the primary.
>>>
>>> Is that a problem?  The name is from the primary template, so does it 
>>> matter for this warning whether there's an explicit specialization 
>>> involved?
>>
>> I don't understand the question.  S<const int> is an instance of
>> the partial specialization.  To diagnose the right mismatch the warning
>> needs to know how to find the template (i.e., either the primary, or
>> the explicit or partial specialization) the instance corresponds to and
>> the class-key it was declared with.  As it is, while GCC does diagnose
>> the right declaration (that of s2), it does that thanks to a bug:
>> because it finds and uses the type and class-key used to declare s1.
>> If we get rid of s1 it doesn't diagnose anything.
>>
>> I tried using DECL_TEMPLATE_SPECIALIZATIONS() to get the list of
>> the partial specializations but it doesn't like any of the arguments
>> I've given it (it ICEs).
> 
> With this fixed, here's the algorithm I tried:
> 
> 1) for a type T of a template instantiation (s1 above), get the primary
>     P that T was instantiated from using
>     P = TYPE_MAIN_DECL (CLASSTYPE_PRIMARY_TEMPLATE_TYPE (T)),
> 2) from P, get the chain of its specializations using
>     SC = DECL_TEMPLATE_SPECIALIZATIONS (P)
> 3) for each (partial) specialization S on the SC chain get the chain
>     of its instantiations IC using DECL_TEMPLATE_INSTANTIATIONS, if
>     is_specialization_of (T, TREE_VALUE (IC)) is non-zero take
>     TREE_VALUE (SC) as the declaration of the partial specialization
>     that the template instanstantiaton T was generated from.
> 
> Unfortunately, in the example above, DECL_TEMPLATE_INSTANTIATIONS for
> the partial specialization 'class S<const T>' is null (even after all
> the declarations have been parsed) so I'm at a dead end.

After a lot more trial and error I discovered
most_specialized_partial_spec in pt.c with whose help I have been able
to get templates to work the way I think they should (at least the cases
I've tested do).

Besides fixing the original problem that motivated it, the attached
patch also corrects how template specializations are handled: the first
declaration of either a primary template or its specialization (either
explicit or partial) determines the class-key in subsequent uses of
the type or its instantiations.  To do this for uses of first-time
template instantiations such as in the declaration of s1 in the test
case above, class_decl_loc_t::diag_mismatched_tags looks up the template
(either the primary or the partial specialization) in the CLASS2LOC map
and uses it and its class-key as the guide when issuing diagnostics.
This also means that the first instance of every template needs to
have a record in the CLASS2LOC map (it also needs it to compare its
class-key to subsequent redeclarations of the template).

This has been unexpectedly difficult.  A big part of it is that I've
never before worked with templates in the front-end so I had to learn
how they're organized (I'm far from having mastered it).  What's made
the learning curve especially steep, besides the sparse documentation,
is the problems hinted at in the paragraph below below.  This whole
area could really stand to be documented in some sort of a writeup:
a high-level overview of how templates are put together (i.e., what
hangs off what in the tree representation) and what APIs to use to
work with them.

The revised patch has been tested on x86_64-linux.

Martin


> The other recurring problem I'm running into is that many of the C++
> FE macros (and APIs) don't always return the expected/documented result.
> I think this is at least in part because until a declaration has been
> fully parsed not all the bits are in place to make it possible to tell
> if it's an implicit or explicit specialization, for example (so
> CLASSTYPE_USE_TEMPLATE (T) is 1 for the first-time declaration of
> an explicit specialization, for example).  But in view of the problem
> above I'm not sure that's the only reason.
> 
>>
>> Martin
>>
>> PS As an aside, both Clang and VC++ have trouble with templates as
>> well, just slightly different kinds of trouble.   Clang diagnoses
>> the declaration of the partial specialization while VC++ diagnoses
>> s1.  In other similar cases like the one below VC++ does the right
>> thing and Clang is silent.
>>
>>    template <class>   struct S { };
>>    template <class T> class S<const T> { };
>>
>>    template <class T> void f (class S<T>);          // VC++ warns
>>    template <class T> void g (struct S<const T>);   // GCC & VC++ warn
>>
>>>
>>>> Martin
>>>>
>>>>>
>>>>>> In (2) and (3) it won't distinguish between specializations of B or
>>>>>> C on different types.  In (2), the function returns the same result
>>>>>> for both:
>>>>>>
>>>>>>    template <> struct B<int>;
>>>>>>    template <> struct B<char>;
>>>>>>
>>>>>> In (3), it similarly returns the same result for both of
>>>>>>
>>>>>>    template <class T> struct C<T*>;
>>>>>>    template <class T> struct C<const T>;
>>>>>>
>>>>>> even though they are declarations of distinct types.
>>>>>
>>>>>
>>>>> Jason
>>>>>
>>>>
>>>
>>
>