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 46B193858411 for ; Wed, 22 Sep 2021 20:09:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 46B193858411 Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-351-Ibu4DEeaPAOYtk9z6DGoYA-1; Wed, 22 Sep 2021 16:08:57 -0400 X-MC-Unique: Ibu4DEeaPAOYtk9z6DGoYA-1 Received: by mail-qk1-f198.google.com with SMTP id bj32-20020a05620a192000b00433162e24d3so12606592qkb.8 for ; Wed, 22 Sep 2021 13:08:57 -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:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=OQWSf3h3SvxaSAJo8P4FdsEn8bBju+cP8pZVj5LDWAc=; b=NDH5Hv6vm52tpOYIiTignn1zgVfhr5uoL4LkrqqzNGQfCJLgLYEUx3jWW0i9PSK9NV JJ0aczeLE7gT862JbTvY73Q+nkQp0rm6MDv+AxEqdISGCKrfH48cFjVGwvuldniDh6jB sAWwwApHlYZ2HqblgxuWvbZv7Y+nLcmQYqU5EH8eqXOX1Xlhw7/T8IYkdaME4TSM3XG4 nsu+23/vSSUI3LHWBZnlzXuF+Gb/ADioXTUaiwH6ij6UXg7BtsbbixkbzDGns61AyITU r/gm8u6QHPecvgdRfNue9kbfruWnzwo9DATCIPgLErNA66NI1/vjnPzHf7FWdGAK02mB ZAnA== X-Gm-Message-State: AOAM530vSKQcMRmIB27dmmQErLLQ7mzX+g7OaInkfY9Oo25tYzZYc/jS 9kr3Hkq4BhkhNai0k35QjaeTjNtjJKCkg8Tq9Uf4gHqNKgHQBCbG3goap1iWlkfDGjS+OPQ8650 TwgzF6FBCZ/auzu7xYg== X-Received: by 2002:a05:620a:799:: with SMTP id 25mr1207516qka.119.1632341337168; Wed, 22 Sep 2021 13:08:57 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwTtVmIADIHOKGn8Z3cgpPU8992BQDXKkEAaORO36in8g1x0Fp4g67jZSdg+NHQCevS5kVgsg== X-Received: by 2002:a05:620a:799:: with SMTP id 25mr1207478qka.119.1632341336827; Wed, 22 Sep 2021 13:08:56 -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 g12sm2180867qtm.59.2021.09.22.13.08.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Sep 2021 13:08:56 -0700 (PDT) Message-ID: <0c128bcf-d67b-9ef3-94cf-3ced028cc51c@redhat.com> Date: Wed, 22 Sep 2021 16:08:55 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.1 Subject: Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] To: Michel Morin Cc: David Malcolm , gcc-patches@gcc.gnu.org References: <63d2c5da-2089-2a41-9427-9c0d7d786272@redhat.com> <5ebab71b-b8a3-0420-df1a-362cce6893ff@redhat.com> From: Jason Merrill 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=-13.1 required=5.0 tests=BAYES_00, CTE_8BIT_MISMATCH, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, 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: Wed, 22 Sep 2021 20:09:03 -0000 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? >>> I've split the patch into two. The first one is for adding missing keywords to >>> fix PR77565 and the second one is for removing the "attribute" keyword. >>> Here is the second patch (if this is not applied, that's no problem ;) ) >>> >>> ====================================================== >>> c++: adjust the handling of RID_ATTRIBUTE. >>> >>> gcc/cp/ChangeLog: >>> >>> * parser.c (cp_keyword_starts_decl_specifier_p): Do not >>> handle RID_ATTRIBUTE. >>> (cp_parser_constructor_declarator_p): Remove now-redundant >>> checks. >>> >>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c >>> index 40308d0d33f..d184a3aca7e 100644 >>> --- a/gcc/cp/parser.c >>> +++ b/gcc/cp/parser.c >>> @@ -1062,7 +1062,6 @@ cp_keyword_starts_decl_specifier_p (enum rid keyword) >>> case RID_TYPEDEF: >>> case RID_INLINE: >>> /* GNU extensions. */ >>> - case RID_ATTRIBUTE: >>> case RID_TYPEOF: >>> /* C++11 extensions. */ >>> case RID_DECLTYPE: >>> @@ -30798,23 +30797,22 @@ cp_parser_constructor_declarator_p >>> (cp_parser *parser, cp_parser_flags flags, >>> /* A parameter declaration begins with a decl-specifier, >>> which is either the "attribute" keyword, a storage class >>> specifier, or (usually) a type-specifier. */ >>> - && (!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))) >>> - /* A parameter declaration can also begin with [[attribute]]. */ >>> + && !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. [[attribute]] can appear in the >>> + first form too, but not in the second form. */ >>> && !cp_next_tokens_can_be_std_attribute_p (parser)) >>> { >>> tree type; >>> tree pushed_scope = NULL_TREE; >>> unsigned saved_num_template_parameter_lists; >>> >>> - if (cp_next_tokens_can_be_gnu_attribute_p (parser)) >>> + if (cp_parser_allow_gnu_extensions_p (parser) >>> + && cp_next_tokens_can_be_gnu_attribute_p (parser)) >>> { >>> unsigned int n = cp_parser_skip_gnu_attributes_opt (parser, 1); >>> while (--n) >>> ====================================================== >>> >>> Regards, >>> Michel >>> >>> >>>>> Both patches (with and without removal of RID_ATTRIBUTE) attached. >>>>> No regressions on x86_64-apple-darwin. >>>>> >>>>> Regards, >>>>> Michel >>>>> >>>>> >>>>> >>>>>>>> So I'm not sure if this fix is exactly correct - hopefully one of the >>>>>>>> C++ frontend maintainers can chime in. If >>>>>>>> cp_keyword_starts_decl_specifier_p isn't quite the right place for >>>>>>>> this, the fix could probably go in suggest_rid_p instead, which *is* >>>>>>>> specific to spelling corrections. >>>>>>>> >>>>>>>> Hope this is constructive; thanks again for the patch >>>>>>>> Dave >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> ============================================ >>>>>>>>> c++: add typo corrections for typedef/inline/cv-qual [PR77565] >>>>>>>>> >>>>>>>>> PR c++/77565 >>>>>>>>> >>>>>>>>> gcc/cp/ChangeLog: >>>>>>>>> >>>>>>>>> * parser.c (cp_keyword_starts_decl_specifier_p): Handle >>>>>>>>> typedef/inline specifiers and cv-qualifiers. >>>>>>>>> >>>>>>>>> gcc/testsuite/ChangeLog: >>>>>>>>> >>>>>>>>> * g++.dg/spellcheck-typenames.C: Add tests for decl-specs. >>>>>>>>> >>>>>>>>> --- a/gcc/cp/parser.c >>>>>>>>> +++ b/gcc/cp/parser.c >>>>>>>>> @@ -1051,6 +1051,12 @@ cp_keyword_starts_decl_specifier_p (enum rid >>>>>>>>> keyword) >>>>>>>>> case RID_FLOAT: >>>>>>>>> case RID_DOUBLE: >>>>>>>>> case RID_VOID: >>>>>>>>> + /* CV qualifiers. */ >>>>>>>>> + case RID_CONST: >>>>>>>>> + case RID_VOLATILE: >>>>>>>>> + /* typedef/inline specifiers. */ >>>>>>>>> + case RID_TYPEDEF: >>>>>>>>> + case RID_INLINE: >>>>>>>>> /* GNU extensions. */ >>>>>>>>> case RID_ATTRIBUTE: >>>>>>>>> case RID_TYPEOF: >>>>>>>>> --- a/gcc/testsuite/g++.dg/spellcheck-typenames.C >>>>>>>>> +++ b/gcc/testsuite/g++.dg/spellcheck-typenames.C >>>>>>>>> @@ -76,3 +76,38 @@ singed char ch; // { dg-error "1: 'singed' does >>>>>>>>> not >>>>>>>>> name a type; did you mean 's >>>>>>>>> ^~~~~~ >>>>>>>>> signed >>>>>>>>> { dg-end-multiline-output "" } */ >>>>>>>>> + >>>>>>>>> +typdef int my_int; // { dg-error "1: 'typdef' does not name a type; >>>>>>>>> did you mean 'typedef'?" } >>>>>>>>> +/* { dg-begin-multiline-output "" } >>>>>>>>> + typdef int my_int; >>>>>>>>> + ^~~~~~ >>>>>>>>> + typedef >>>>>>>>> + { dg-end-multiline-output "" } */ >>>>>>>>> + >>>>>>>>> +inlien int inline_func(); // { dg-error "1: 'inlien' does not name a >>>>>>>>> type; did you mean 'inline'?" } >>>>>>>>> +/* { dg-begin-multiline-output "" } >>>>>>>>> + inlien int inline_func(); >>>>>>>>> + ^~~~~~ >>>>>>>>> + inline >>>>>>>>> + { dg-end-multiline-output "" } */ >>>>>>>>> + >>>>>>>>> +coonst int ci = 0; // { dg-error "1: 'coonst' does not name a type; >>>>>>>>> did you mean 'const'?" } >>>>>>>>> +/* { dg-begin-multiline-output "" } >>>>>>>>> + coonst int ci = 0; >>>>>>>>> + ^~~~~~ >>>>>>>>> + const >>>>>>>>> + { dg-end-multiline-output "" } */ >>>>>>>>> + >>>>>>>>> +voltil int vi; // { dg-error "1: 'voltil' does not name a type; did >>>>>>>>> you mean 'volatile'?" } >>>>>>>>> +/* { dg-begin-multiline-output "" } >>>>>>>>> + voltil int vi; >>>>>>>>> + ^~~~~~ >>>>>>>>> + volatile >>>>>>>>> + { dg-end-multiline-output "" } */ >>>>>>>>> + >>>>>>>>> +statik int si; // { dg-error "1: 'statik' does not name a type; did >>>>>>>>> you mean 'static'?" } >>>>>>>>> +/* { dg-begin-multiline-output "" } >>>>>>>>> + statik int si; >>>>>>>>> + ^~~~~~ >>>>>>>>> + static >>>>>>>>> + { dg-end-multiline-output "" } */ >>>>>>>>> ============================================ >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Regards, >>>>>>>>> Michel >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>> >>