From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x743.google.com (mail-qk1-x743.google.com [IPv6:2607:f8b0:4864:20::743]) by sourceware.org (Postfix) with ESMTPS id 61914385E003 for ; Wed, 25 Mar 2020 20:54:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 61914385E003 Received: by mail-qk1-x743.google.com with SMTP id h14so4203250qke.5 for ; Wed, 25 Mar 2020 13:54:39 -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=vlAnKEbN+O8xD/77VtBGFBzjjl7lTKkbdkcys5TnSbk=; b=T8yVmyFU6U4NOZaGWTeLYV5v6ShYTuAAkfkfsEm/zAwAAenEb65lQi//PyvA50nIng K+Itz8u1/k8oxyyeCpgI66vOY/GHmUAHkT0safgrQhjKFXyzy4An0WJeVSF9xO6RcMlq bYIWyoCpoubLbDYAJ/GWOtreuEVyplhPebqw5ATIcjnuCdMEY4vL8xbjpo5/+XlxLxfg IKhfokLWjD4EgpjZED8pK6Vzdsb7np1PHrvsYh7JpuSlby6qLHxjjfflJ7wTJD/iWIRz kGRoxk9ol+E9Ucgx/hI/Uz2fG7moKAZOS9svvGXT7Ic9jRWYM82Og5csIUbB7Z2LVCYn u/Jg== X-Gm-Message-State: ANhLgQ1M0KazJTkNDL8lbKWdhf6qe8ykdVhtNJfGhk94eQpp1XsSjKhP 9XmUIGwKLa/ngwAcIutpLRql3uFh X-Google-Smtp-Source: ADFU+vu78Ev0YZf5VVz4EAkTvpbg6ZpqMhHKoiITy1LDpWiKK+L892Fl4za/JEHEkUR35vD4oVOjAA== X-Received: by 2002:a37:4bd3:: with SMTP id y202mr5011807qka.32.1585169678609; Wed, 25 Mar 2020 13:54:38 -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 c12sm99375qtb.49.2020.03.25.13.54.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Mar 2020 13:54:38 -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> <2744364d-e705-06f8-064a-5425ade11669@gmail.com> <1239c5f0-7ea7-e760-0b1c-1fe725a5e129@redhat.com> <48d42e59-64c4-6674-40a4-c786f9e504f4@gmail.com> <6775e196-9f97-6651-bc6a-ee6d4b7d16bc@gmail.com> Message-ID: Date: Wed, 25 Mar 2020 14:54:37 -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: <6775e196-9f97-6651-bc6a-ee6d4b7d16bc@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, BODY_8BITS, 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, TXREP 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, 25 Mar 2020 20:54:41 -0000 Ping: Jason, is the latest patch acceptable or are there any other changes you want me to make? https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542538.html On 3/21/20 3:59 PM, Martin Sebor wrote: > On 3/20/20 3:53 PM, Jason Merrill wrote: >> 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. > > I changed it to use is_declaration instead. > >>>>> +  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    struct S; >>>    template struct S; >>> >>>    extern class  S s1;   // expect -Wmismatched-tags >>>    extern struct S 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 struct S’ declared with a >>> mismatched class-key ‘struct’ [-Wmismatched-tags] >>>      5 | extern struct S s2; >>>        |               ^~~~~~~ >>> z.C:5:15: note: remove the class-key or replace it with ‘class’ >>> z.C:4:15: note: ‘template struct S’ first declared as >>> ‘class’ here >>>      4 | extern class  S 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 > > I'm not sure what you did differently.  With the patch (the last one > or the one in the attachment) we get the expected warning below. > >> >> wa2.C:4:17: warning: ‘S’ declared with a mismatched class-key >> ‘class’ [-Wmismatched-tags] >>      4 |   extern class  S s1;   // expect -Wmismatched-tags >>        |                 ^~~~~~~ >> wa2.C:4:17: note: remove the class-key or replace it with ‘struct’ >> wa2.C:2:29: note: ‘S’ first declared as ‘struct’ here >>      2 |   template struct S; >> >> So the only difference is whether we talk about S or S.  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? > > Sure. > >> >>> +  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. > > Okay.  Attached is an revised patch with these changes. > > Martin