From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x841.google.com (mail-qt1-x841.google.com [IPv6:2607:f8b0:4864:20::841]) by sourceware.org (Postfix) with ESMTPS id CD8DA3942014 for ; Thu, 12 Mar 2020 17:03:26 +0000 (GMT) Received: by mail-qt1-x841.google.com with SMTP id n5so4949724qtv.7 for ; Thu, 12 Mar 2020 10:03:26 -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=vpy5iYeG7UJSD7alWKqCptaNkv5qb288nJKh8Z6teVA=; b=lRnBYc3iBMuAK6bXkK2DTcQcy0HvqY8pDy7Z9DwdiaJtXO8xerCJzfvU0j6XogcREF aigH6MNGxinujD0vXq5iQ7mQe4bpYhJ6w6fZL5evgzkRmuhb1owrTx2Uo5K4fVyHxP5m 1uSHQzjc5gO+cxHwBViCG3AAkHtXQIqV9NdN0Sai7PJhMF82Icdrn7kCmZY2Wd7KXVQW 9QdcUX1TTqT1o5Noc4exGSfEygSZ+n+LEdI4ey7HeqkE2S3/VR8KpAswXNC88BEUCBIi G/3cl8FSlGtPHYKMCrJ4GTnpa+WXH3vpfChQ5xuVfcinpeZHQMbexqo1f5u6/pOfq55p dy7g== X-Gm-Message-State: ANhLgQ21OZvQB/RTEE4639TDyWin8SZtT7uYhZjGAGXOUoFaP58U55ys ZHilleU26u8mKBa8l5302p0vvuXI X-Google-Smtp-Source: ADFU+vszkpZ/J0aQYmb2wyk7K8HCJez8TyeVCmpA8qt/ogr6pWkzL3HjeRoj+nCUdCxkn59I6LSpLw== X-Received: by 2002:ac8:3f62:: with SMTP id w31mr834309qtk.171.1584032604942; Thu, 12 Mar 2020 10:03:24 -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 o57sm7915980qtf.42.2020.03.12.10.03.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 12 Mar 2020 10:03:24 -0700 (PDT) Subject: Re: [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> Message-ID: <2744364d-e705-06f8-064a-5425ade11669@gmail.com> Date: Thu, 12 Mar 2020 11:03:22 -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=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: Thu, 12 Mar 2020 17:03:28 -0000 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. 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 >>>> >>> >> >