From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 22B3A3858D29 for ; Thu, 23 Sep 2021 20:29:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 22B3A3858D29 Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-259-mmO1KiXuOTi9x9W-xq34Aw-1; Thu, 23 Sep 2021 16:29:15 -0400 X-MC-Unique: mmO1KiXuOTi9x9W-xq34Aw-1 Received: by mail-qt1-f198.google.com with SMTP id x28-20020ac8701c000000b0029f4b940566so19140786qtm.19 for ; Thu, 23 Sep 2021 13:29:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:from :subject:to:cc:references:content-language:in-reply-to :content-transfer-encoding; bh=cvdYhwZ48obxCr0gitiu5l+MrZW1VXTLL2iegCxF5+Q=; b=PSiCxHo/yFDF+0TIYWw4q0hplLXdduQkwXmjgzaddGFke05GezWvTqVpvwZhRoSKud V+OxyJkuaZ3tWAtSuhKChGa4gOq3Tl1y1FzivXIWzcgATTDNTDFfwLlh4j1Qb+NjfwoO 7vj1JhHNIYE8OOHbropSOAZB/9vDO4ALrLKsT3ks2Djly89GgmE5pN7sxBpZkHBLJKSh sgOfuhRGXI//DlS84tvPJDWSqW5yG+arrg9wg82UhRTck1L3oMmCimBX2ibIVOm04UU1 TcfndDJuqT18jQAvVMS7eFl5dgoSJ0uNvw6OWVqfw7VS1x2YZx6iyMxhNGubt9ti1ZJv mdHQ== X-Gm-Message-State: AOAM531DPcTWxg+rocZvpzomukEmxj/nvosIEyTvfLE+s2XfpIzrqwLW lHMZjf354f6AriUhOLb5p5pxvWxhDmacQXDeDJcpiFafP1X7ZuJkpvpgC1SGUWsOEUGhuGnvHXA RKolvOF3vc11hghajtQ== X-Received: by 2002:ad4:5630:: with SMTP id cb16mr6263339qvb.45.1632428954457; Thu, 23 Sep 2021 13:29:14 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzrih+EoX60sXDLHDr5REQVgQJ4rffksAgR8rTh1ktqo2b9GfgS+S/kBWIVJggfxOw1xtkp+Q== X-Received: by 2002:ad4:5630:: with SMTP id cb16mr6263314qvb.45.1632428954108; Thu, 23 Sep 2021 13:29:14 -0700 (PDT) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id a189sm4963188qkf.114.2021.09.23.13.29.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Sep 2021 13:29:13 -0700 (PDT) Message-ID: <5f447526-0a28-4a42-c3c8-445d23dcf561@redhat.com> Date: Thu, 23 Sep 2021 16:29:12 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.1 From: Jason Merrill Subject: Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] To: Michel Morin Cc: David Malcolm , gcc-patches List References: <63d2c5da-2089-2a41-9427-9c0d7d786272@redhat.com> <5ebab71b-b8a3-0420-df1a-362cce6893ff@redhat.com> <0c128bcf-d67b-9ef3-94cf-3ced028cc51c@redhat.com> In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, CTE_8BIT_MISMATCH, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 23 Sep 2021 20:29:20 -0000 On Wed, Sep 22, 2021 at 7:05 PM Michel Morin wrote: On Thu, Sep 23, 2021 at 5:09 AM Jason Merrill wrote: > > On 9/21/21 20:53, Michel Morin wrote: > > On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill wrote: > >> > >> On 9/17/21 13:31, Michel Morin wrote: > >>> On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill wrote: > >>>> > >>>> On 9/16/21 11:50, Michel Morin wrote: > >>>>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill wrote: > >>>>>> > >>>>>> On 9/14/21 04:29, Michel Morin via Gcc-patches wrote: > >>>>>>> On Tue, Sep 14, 2021 at 7:14 AM David Malcolm wrote: > >>>>>>>> > >>>>>>>> On Tue, 2021-09-14 at 03:35 +0900, Michel Morin via Gcc-patches wrote: > >>>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> PR77565 reports that, with the code `typdef int Int;`, GCC emits > >>>>>>>>> "did you mean 'typeof'?" instead of "did you mean 'typedef'?". > >>>>>>>>> > >>>>>>>>> This happens because the typo corrector determines that `typeof` is a > >>>>>>>>> candidate for suggestion (through > >>>>>>>>> `cp_keyword_starts_decl_specifier_p`), > >>>>>>>>> but `typedef` is not. > >>>>>>>>> > >>>>>>>>> This patch fixes the issue by adding `typedef` as a candidate. The > >>>>>>>>> patch > >>>>>>>>> additionally adds the `inline` specifier and cv-specifiers as a > >>>>>>>>> candidate. > >>>>>>>>> Here is a patch (tests `make check-gcc` pass on darwin): > >>>>>>>> > >>>>>>>> Thanks for this patch (and for reporting the bug in the first place). > >>>>>>>> > >>>>>>>> I notice that, as well as being used for fix-it hints by > >>>>>>>> lookup_name_fuzzy (indirectly via suggest_rid_p), > >>>>>>>> cp_keyword_starts_decl_specifier_p is also used by > >>>>>>>> cp_lexer_next_token_is_decl_specifier_keyword, which is used by > >>>>>>>> cp_parser_lambda_declarator_opt and cp_parser_constructor_declarator_p. > >>>>>>> > >>>>>>> Ah, you're right! Thank you for pointing this out. > >>>>>>> I failed to grep those functions somehow. > >>>>>>> > >>>>>>> One thing that confuses me is that cp_keyword_starts_decl_specifier_p > >>>>>>> misses many keywords that can start decl-specifiers (e.g. > >>>>>>> typedef/inline/cv-qual and friend/explicit/virtual). > >>>>>>> So let's wait C++ frontend maintainers ;) > >>>>>> > >>>>>> That is strange.  Let's add all the rest of them as well. > >>>>> > >>>>> Done. Thanks for your help! > >>>>> > >>>>> One more thing — cp_keyword_starts_decl_specifier_p includes RID_ATTRIBUTE > >>>>> (from the beginning; see https://gcc.gnu.org/PR28261 ), but attributes are > >>>>> not decl-specifiers. Would it be reasonable to remove this? > >>>> > >>>> It looks like the place that PR28261 used > >>>> cp_lexer_next_token_is_decl_specifier_keyword specifically exempts > >>>> attributes: > >>>> > >>>>>             && (!cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer) > >>>>>                 /* GNU attributes can actually appear both at the start of > >>>>>                    a parameter and parenthesized declarator. > >>>>>                    S (__attribute__((unused)) int); > >>>>>                    is a constructor, but > >>>>>                    S (__attribute__((unused)) foo) (int); > >>>>>                    is a function declaration.  */ > >>>>>                 || (cp_parser_allow_gnu_extensions_p (parser) > >>>>>                     && cp_next_tokens_can_be_gnu_attribute_p (parser))) > >>>> > >>>> So yes, let's remove RID_ATTRIBUTE and the || clause there.  I'd keep > >>>> the comment, but move it to go with the test for C++11 attributes below. > >>> > >>> Done. No regressions introduced. > >>> > >>>>> One more thing — cp_keyword_starts_decl_specifier_p includes RID_ATTRIBUTE > >>>>> (from the beginning; see https://gcc.gnu.org/PR28261 ), but attributes are > >>>>> not decl-specifiers. > >>> > >>> Oh, this is wrong. I thought that, since C++11 attributes are not a > >>> decl-specifier, neither are GNU attributes. But the comment just before > >>> cp_parser_decl_specifier_seq says that GNU attributes are considered as a > >>> decl-specifier. So I'm not confident about the removal of RID_ATTRIBUTE in > >>> cp_keyword_starts_decl_specifier_p... > >> > >> GNU attributes can appear in lots of places, and the only two callers of > >> cp_parser_next_token_is_decl_specifier_keyword don't want to treat > >> attributes accordingly. > > > > Makes sense. > > > >> Let's go with both your patches, and also > >> remove the consequently-unnecessary attributes check in > >> cp_parser_lambda_declarator_opt: > >> > >>>    if (cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer) > >>>        && !cp_next_tokens_can_be_gnu_attribute_p (parser)) > >> > >> OK with that change. > > > > Updated and rebased the patch. No regressions on x86_64-apple-darwin. > > > > Thank you for your help! > > Looks good, thanks.  You can push the patches yourself, right? This is my first patch contribution to GCC, and I don't have write access. So it'd be great if someone pushes the patches. Ah!  Sorry, I was confusing you with Mikael Morin , who does have write access. > I assume these patches are small enough that copyright assignment or > DCO certification are not needed. (If needed, I'll prepare one.) Agreed. I applied the patches, adjusting the name in the commit to match the name on your email (rather than "morinmorin"). Thanks! Jason