From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 75984 invoked by alias); 14 Dec 2018 21:43:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 75966 invoked by uid 89); 14 Dec 2018 21:43:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=HTo:D*oracle.com X-HELO: mail-qt1-f193.google.com Received: from mail-qt1-f193.google.com (HELO mail-qt1-f193.google.com) (209.85.160.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Dec 2018 21:43:09 +0000 Received: by mail-qt1-f193.google.com with SMTP id l12so5962833qtf.8 for ; Fri, 14 Dec 2018 13:43:09 -0800 (PST) Return-Path: Received: from [192.168.1.132] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id b134sm2673097qkg.78.2018.12.14.13.43.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 14 Dec 2018 13:43:06 -0800 (PST) Subject: Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") To: Paolo Carlini Cc: gcc-patches List , Nathan Sidwell References: <4c6fe735-5435-1ede-a85a-787262a7e63c@oracle.com> <37c9ffea-5929-a209-7818-67e05d44edcc@oracle.com> <1db43e23-22da-0f4a-d711-fe6c54244564@oracle.com> <6d56abf3-93a0-d1ad-0072-2e0d9922b492@redhat.com> <6d889ffa-607f-853e-4b87-68bcf5c1028c@oracle.com> From: Jason Merrill Message-ID: <0c2cb9af-f572-6d03-c82d-c8c25320b0bf@redhat.com> Date: Fri, 14 Dec 2018 21:43:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg01112.txt.bz2 On 12/14/18 4:33 PM, Paolo Carlini wrote: > Hi, > > On 14/12/18 21:19, Jason Merrill wrote: >> On 12/14/18 1:44 PM, Paolo Carlini wrote: >>> Hi, >>> >>> On 13/12/18 22:03, Jason Merrill wrote: >>>> On 10/30/18 9:22 PM, Paolo Carlini wrote: >>>>> Hi, >>>>> >>>>> On 30/10/18 21:37, Jason Merrill wrote: >>>>>> On 10/26/18 2:02 PM, Paolo Carlini wrote: >>>>>>> On 26/10/18 17:18, Jason Merrill wrote: >>>>>>>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >>>>>>>> wrote: >>>>>>>>> On 24/10/18 22:41, Jason Merrill wrote: >>>>>>>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>>>>>>>          && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>>>>>>>> +       && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>>>>>>>           && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>>>>>>>> I would think that the MAYBE_CLASS_TYPE_P here should be >>>>>>>>>> CLASS_TYPE_P, >>>>>>>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>>>>>>>> allow template type parameters for some reason? >>>>>>>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However >>>>>>>>> it seems >>>>>>>>> we at least want to let through TEMPLATE_TYPE_PARMs >>>>>>>>> representing 'auto' >>>>>>>>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>>>>>>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, >>>>>>>>> otherwise >>>>>>>>> we regress on template/spec32.C and template/ttp22.C because we >>>>>>>>> don't >>>>>>>>> diagnose the shadowing anymore. Thus, I would say either we >>>>>>>>> keep on >>>>>>>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we >>>>>>>>> add a comment? >>>>>>>> Aha.  I guess the answer is not to restrict that test any more, but >>>>>>>> instead to fix the code further down so it gives a proper >>>>>>>> diagnostic >>>>>>>> rather than call warn_misplaced_attr_for_class_type. >>>>>>> >>>>>>> I see. Thus something like the below? It passes testing on >>>>>>> x86_64-linux. >>>>>> >>>>>>> +  if ((!declared_type || TREE_CODE (declared_type) == >>>>>>> DECLTYPE_TYPE) >>>>>>> +      && ! saw_friend && !error_p) >>>>>>>      permerror (input_location, "declaration does not declare >>>>>>> anything"); >>>>>> >>>>>> I see no reason to make this specific to decltype.  Maybe move >>>>>> this diagnostic into the final 'else' block with the other >>>>>> declspec diagnostics and not look at declared_type at all? >>>>> >>>>> I'm not sure to fully understand: if we do that we still want to at >>>>> least minimally check that declared_type is null, like we already >>>>> do, and then we simply accept the new testcase. Is that Ok? >>>>> Because, as I probably mentioned at some point, all the other >>>>> compilers I have at hand issue a "does not declare anything" >>>>> diagnostic, and we likewise do that for the legacy __typeof. Not >>>>> looking into declared_type *at all* doesn't work with plain class >>>>> types and enums, of course. Or you meant something entirely >>>>> different?? >>>>> >>>>>>> +  if (declspecs->attributes && warn_attributes && declared_type >>>>>>> +      && TREE_CODE (declared_type) != DECLTYPE_TYPE) >>>>>> >>>>>> I think we do want to give a diagnostic about useless attributes, >>>>>> not skip it. >>>>> >>>>> Agreed. FWIW the attached tests fine. >>>> >>>> The problem here is that the code toward the bottom expects >>>> "declared_type" to be the tagged type declared by a declaration with >>>> no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE. >>>> >>>> I think once we've checked for 'auto' we don't want declared_type to >>>> be anything that isn't OVERLOAD_TYPE_P.  We can arrange that either >>>> by checking for 'auto' first and then changing the code that sets >>>> declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type >>>> after checking for 'auto' if it isn't OVERLOAD_TYPE_P. >>> >>> Thanks. I'm slowly catching up on this issue... Any suggestion about >>> BOUND_TEMPLATE_TEMPLATE_PARM? If we don't let through such tree nodes >>> - which are MAYBE_CLASS_TYPE_P and aren't OVERLOAD_TYPE_P - we >>> regress on template/spec32.C, we don't reject it anymore. >> If we clear declared_type for a BOUND_TEMPLATE_TEMPLATE_PARM, we >> should get the "does not declare anything" error. > > Ah, now I see, I didn't realize that we would also change the errors we > issue for those testcases. Thus the below is finishing testing, appears > to work fine. OK. Jason