On Thu, Sep 23, 2021 at 5:09 AM Jason Merrill <ja...@redhat.com> wrote: > > On 9/21/21 20:53, Michel Morin wrote: > > On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill <ja...@redhat.com> wrote: > >> > >> On 9/17/21 13:31, Michel Morin wrote: > >>> On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <ja...@redhat.com> wrote: > >>>> > >>>> On 9/16/21 11:50, Michel Morin wrote: > >>>>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <ja...@redhat.com> wrote: > >>>>>> > >>>>>> On 9/14/21 04:29, Michel Morin via Gcc-patches wrote: > >>>>>>> On Tue, Sep 14, 2021 at 7:14 AM David Malcolm <dmalc...@redhat.com> > >>>>>>> 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. I assume these patches are small enough that copyright assignment or DCO certification are not needed. (If needed, I'll prepare one.) > >>> 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 > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>> > >> >