From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82a.google.com (mail-qt1-x82a.google.com [IPv6:2607:f8b0:4864:20::82a]) by sourceware.org (Postfix) with ESMTPS id 05628394881E for ; Wed, 18 Mar 2020 22:09:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 05628394881E Received: by mail-qt1-x82a.google.com with SMTP id f20so106984qtq.6 for ; Wed, 18 Mar 2020 15:09:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=osHHhOO7c4+XS3u3Jb/hlMZYC9zeXx3Q8vkggfjs7rI=; b=hizdAS/yA2okVO1NLCtNjQu9dJgDGBXP+t4zVcpHD8dr8Eklqz9On+xUOAi8K76SY/ NDXvo0Za8v8VmSWzVmv+Cjxyi8uBaUii1rtf0CAmKtAodTOLV2lCdvogkhYB8KrNiyVJ +f4Phv0StoG4LqwN9vLoWEVVXSJu0hKKylDklcrUlU60QYpt03pQXSZJ0uaWmPh2Y34h RdNm2RXL0l5cmLfzAwJOsy2oc5hqqNJsEQ/nbSsMn4nd6BkALvjZzKTDqBDAixcJByVh 51AayYSstEZ7hupjHNR3uAZN3eV5y/tmyuxK3U+SrJ8UEqREooAoD9baUdOvCvz4VaLi J+0g== X-Gm-Message-State: ANhLgQ1AVIo76jUgAxMGjBER6+tuQR3mAN+Yfq8S5K0ehXrzzaNtFPXH KsAf41v0WzIR7ACPR6bAUSLF3lET X-Google-Smtp-Source: ADFU+vtj7u44TxAD7EnXrHBq8tTWQOo+uMOZLeH0xn049Jb3rXElMJOcP+KnycerypyG05Vu5TTLpw== X-Received: by 2002:aed:370a:: with SMTP id i10mr7025992qtb.114.1584569396170; Wed, 18 Mar 2020 15:09:56 -0700 (PDT) Received: from [192.168.0.41] (97-118-124-45.hlrn.qwest.net. [97.118.124.45]) by smtp.gmail.com with ESMTPSA id f13sm235991qkm.19.2020.03.18.15.09.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 18 Mar 2020 15:09:55 -0700 (PDT) Subject: [PING][PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824) From: Martin Sebor To: Jason Merrill , gcc-patches References: <010f6d68-a64e-45a4-744e-c040a4ea94d6@redhat.com> <65013bfc-23c3-47fb-a58d-9ab802d1febb@gmail.com> <2698a399-4176-2b5f-a134-52a0d82c2121@redhat.com> <13b8faa9-74b1-6131-5dda-d4f34dfa8af0@gmail.com> <77dc94af-40f1-46fd-f3f4-ceb7e3afc4b6@gmail.com> <00f21176-ea92-937c-5c8a-c04d5d813257@redhat.com> <90a6aebc-8fef-0b96-cd2b-3234bd82b9a0@gmail.com> <2c553c0d-c0ac-88ba-2a85-12c523a9a164@redhat.com> <2744364d-e705-06f8-064a-5425ade11669@gmail.com> Message-ID: Date: Wed, 18 Mar 2020 16:09:54 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=2.3 required=5.0 tests=DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GARBLED_BODY, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Level: ** X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Mar 2020 22:09:58 -0000 Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/541962.html On 3/12/20 4:38 PM, Martin Sebor wrote: > 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 struct S7 { }; >>>>>>>>>>>    struct S7 s7v;  // { dg-warning >>>>>>>>>>> "\\\[-Wredundant-tags" } >>>>>>>>>>> >>>>>>>>>>> where DECL is the TEPLATE_DECL of S7. >>>>>>>>>>> >>>>>>>>>>> As I mentioned, more work is still needed to handle templates >>>>>>>>>>> right >>>>>>>>>>> because some redundant tags are still not diagnosed.  For >>>>>>>>>>> example: >>>>>>>>>>> >>>>>>>>>>>    template struct S7 { }; >>>>>>>>>>>    template >>>>>>>>>>>    using U = struct S7;   // 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 A; >>>>>>>>>       struct A a; >>>>>>>>> >>>>>>>>>    2) Instance of an explicit specialization: >>>>>>>>>       template class B; >>>>>>>>>       template <> struct B; >>>>>>>>>       class B b; >>>>>>>>> >>>>>>>>>    3) Instance of a partial specialization: >>>>>>>>>       template class C; >>>>>>>>>       template struct C; >>>>>>>>>       class C 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    struct S; >>>>>    template class S; >>>>>    extern class  S s1; >>>>>    extern struct S 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 (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 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' 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    struct S { }; >>>    template class S { }; >>> >>>    template void f (class S);          // VC++ warns >>>    template void g (struct S);   // 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; >>>>>>>    template <> struct B; >>>>>>> >>>>>>> In (3), it similarly returns the same result for both of >>>>>>> >>>>>>>    template struct C; >>>>>>>    template struct C; >>>>>>> >>>>>>> even though they are declarations of distinct types. >>>>>> >>>>>> >>>>>> Jason >>>>>> >>>>> >>>> >>> >> >