From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe2e.google.com (mail-vs1-xe2e.google.com [IPv6:2607:f8b0:4864:20::e2e]) by sourceware.org (Postfix) with ESMTPS id 0D6E33858D34 for ; Fri, 24 Sep 2021 00:49:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0D6E33858D34 Received: by mail-vs1-xe2e.google.com with SMTP id k10so8331204vsp.12 for ; Thu, 23 Sep 2021 17:49:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=DV1bpUote96J/JYKIvejbMmK/GvAjVNy5fJ+nNNe93U=; b=iNfvm9U8E9SmXgr9g0J9PnG0TgBF0QNq/PrWjjLW7SUvm5m5BrXqMPCmle+KaPiPbI RkE4067+XXPcB6GmswWo14FmZMIZI4U1klzwkS62imUy9/JbA6NQmqjLYj/TXODkkgIU 14g9He7kSjpUvcjI4R3boKnsIvhiKrhzjKu6ULR9ZHO/DuJtcoY1lhc/vm5RXh9QeEfW meKr25Ctv8vlujz14wGJtEOoJRAPUJ8r9028GQJntIu3RrUJjFN18XN02cw4xn/J5xnp T+bUhXZ0Q0KVaYbY/Q/XLE8TIj3eYzx3f6ETAgEuXWuHPtnWFWiqChjHvL0aBicJ5ce5 RuUg== X-Gm-Message-State: AOAM5302tSAZyj66yXuW2WrwWiJn3qJckBYAuxfO2bP3JMSjlFVYTih6 kAuHQV7PHAfH9Hg5VBzj3zqlpi1Z0RDI+gXyerT7GpOG2ZmCkA== X-Google-Smtp-Source: ABdhPJxgXUd1JtStcK7McQtBXQL6gdv9d9H612ISH9IlV/oA1B+ugtREoH2o5OaP7Pijsxoa14MyEkxPGkHzVfRXjOk= X-Received: by 2002:a67:ee12:: with SMTP id f18mr7648794vsp.20.1632444587472; Thu, 23 Sep 2021 17:49:47 -0700 (PDT) MIME-Version: 1.0 References: <63d2c5da-2089-2a41-9427-9c0d7d786272@redhat.com> <5ebab71b-b8a3-0420-df1a-362cce6893ff@redhat.com> <0c128bcf-d67b-9ef3-94cf-3ced028cc51c@redhat.com> <5f447526-0a28-4a42-c3c8-445d23dcf561@redhat.com> In-Reply-To: <5f447526-0a28-4a42-c3c8-445d23dcf561@redhat.com> From: Michel Morin Date: Fri, 24 Sep 2021 09:49:36 +0900 Message-ID: Subject: Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] To: Jason Merrill Cc: David Malcolm , gcc-patches List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: Fri, 24 Sep 2021 00:49:49 -0000 On Fri, Sep 24, 2021 at 5:29 AM Jason Merrill wrote: > > On Wed, Sep 22, 2021 at 7:05 PM Michel Morin wrote: > > On Thu, Sep 23, 2021 at 5:09 AM Jason Merrill wrot= e: > > > > 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 =E2=80=94 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 =E2=80=94 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 tr= eat > > >> 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. What a coincidence ;) > > 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"). Thank you for the commit! Next time, I'll change my git user.name. Regards, Michel > Thanks! > > Jason >