From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-xc41.google.com (mail-yw1-xc41.google.com [IPv6:2607:f8b0:4864:20::c41]) by sourceware.org (Postfix) with ESMTPS id 4F2493947C09 for ; Wed, 11 Mar 2020 21:30:05 +0000 (GMT) Received: by mail-yw1-xc41.google.com with SMTP id d206so3487734ywa.12 for ; Wed, 11 Mar 2020 14:30:05 -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:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=jLCmloT7wwzIrgXxSY1D0lRl+Ybq6dZM0cn55+gehzc=; b=gJy5WePOvzv0UXSnd34abNx2qENn5FhCZYDrL5B6lDHHNTB7RaGJTvqjiD3ZRuJglT 3PF4PrpVvqcmIJbKxzv70jcToCo3e/Y8K9OYm1P/o5IOEgaqslrgXw+Du2AccywvpQUZ E5kCDbTo9cS22/Ui/G2yO+ds62gZH8qzX2jSBB261ST7OphxOrjMvo3S2aRFMlk6dFgl xHVFOG1oT+8HRl1xaEBLloKRiJdZVnrgWkh8qBW5LFWahF9SK6Du0+YyKkGRme16oCfF ch1tUydcbAvdFDhMJX4qaijTnvzG6dkzS8iBsE6aeKdvu01VOP0X0Gxs6lFEviixg9XC wyxA== X-Gm-Message-State: ANhLgQ1vmmUcKRDgx1oekv2j7zBP+7Uo02qsCfBTG4J6fq2YFMq7L7j3 6r9Cgfu9n0EcdXTzJHsD+wztrMVq X-Google-Smtp-Source: ADFU+vuoV8npMRALOlQlMBdvwcTLEJxoOftU7ObtmRq/pVT/mHHFvbjDaedr+5TNwnf8M4EFHF6RrA== X-Received: by 2002:a81:25d6:: with SMTP id l205mr5267892ywl.139.1583962204457; Wed, 11 Mar 2020 14:30:04 -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 k195sm13914669ywk.104.2020.03.11.14.30.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 11 Mar 2020 14:30:03 -0700 (PDT) Subject: Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824) 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> From: Martin Sebor Message-ID: Date: Wed, 11 Mar 2020 15:30:02 -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: <2c553c0d-c0ac-88ba-2a85-12c523a9a164@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=0.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GARBLED_BODY, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.2 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, 11 Mar 2020 21:30:06 -0000 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). 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 >>> >> >