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... 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 > >>>> > >>>> > >>> > >> >