* [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565]
@ 2021-09-13 18:35 Michel Morin
2021-09-13 22:14 ` David Malcolm
0 siblings, 1 reply; 13+ messages in thread
From: Michel Morin @ 2021-09-13 18:35 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2635 bytes --]
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):
============================================
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
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1960 bytes --]
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index f9c2c8ac3a7..5295911eb82 100644
--- 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:
diff --git a/gcc/testsuite/g++.dg/spellcheck-typenames.C b/gcc/testsuite/g++.dg/spellcheck-typenames.C
index ff53ecc6303..75f80480e16 100644
--- 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 "" } */
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-13 18:35 [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] Michel Morin @ 2021-09-13 22:14 ` David Malcolm 2021-09-14 8:29 ` Michel Morin 0 siblings, 1 reply; 13+ messages in thread From: David Malcolm @ 2021-09-13 22:14 UTC (permalink / raw) To: Michel Morin, gcc-patches 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. 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-13 22:14 ` David Malcolm @ 2021-09-14 8:29 ` Michel Morin 2021-09-15 20:43 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Michel Morin @ 2021-09-14 8:29 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches On Tue, Sep 14, 2021 at 7:14 AM David Malcolm <dmalcolm@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 ;) 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-14 8:29 ` Michel Morin @ 2021-09-15 20:43 ` Jason Merrill 2021-09-16 15:50 ` Michel Morin 0 siblings, 1 reply; 13+ messages in thread From: Jason Merrill @ 2021-09-15 20:43 UTC (permalink / raw) To: Michel Morin, David Malcolm; +Cc: gcc-patches On 9/14/21 04:29, Michel Morin via Gcc-patches wrote: > On Tue, Sep 14, 2021 at 7:14 AM David Malcolm <dmalcolm@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. >> 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 >> >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-15 20:43 ` Jason Merrill @ 2021-09-16 15:50 ` Michel Morin 2021-09-16 18:23 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Michel Morin @ 2021-09-16 15:50 UTC (permalink / raw) To: Jason Merrill; +Cc: David Malcolm, gcc-patches [-- Attachment #1: Type: text/plain, Size: 5266 bytes --] On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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? 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 > >> > >> > > > [-- Attachment #2: add_and_remove.git_diff.txt --] [-- Type: text/plain, Size: 2590 bytes --] From ebf35375e55b1335d24dbbf8cee803f51813adab Mon Sep 17 00:00:00 2001 From: morinmorin <mimomorin@gmail.com> Date: Thu, 16 Sep 2021 23:26:51 +0900 Subject: [PATCH] c++: add spellcheck suggestions for typedef etc. [PR77565] cp_keyword_starts_decl_specifier_p misses many keywords that can start decl-specifiers. This patch adds support for those keywords. It also removes support for attributes, which cannot start decl-spec. PR c++/77565 gcc/cp/ChangeLog: * parser.c (cp_keyword_starts_decl_specifier_p): Handle more decl-specifiers (typedef/inline/cv/explicit/virtual/friend). Do not handle attributes. gcc/testsuite/ChangeLog: * g++.dg/spellcheck-pr77565.C: New test. --- gcc/cp/parser.c | 11 ++++++++++- gcc/testsuite/g++.dg/spellcheck-pr77565.C | 12 ++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr77565.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 8d60f40706b..2c22b47bb68 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -1051,8 +1051,17 @@ 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: + /* Function specifiers. */ + case RID_EXPLICIT: + case RID_VIRTUAL: + /* friend/typdef/inline specifiers. */ + case RID_FRIEND: + case RID_TYPEDEF: + case RID_INLINE: /* GNU extensions. */ - case RID_ATTRIBUTE: case RID_TYPEOF: /* C++11 extensions. */ case RID_DECLTYPE: diff --git a/gcc/testsuite/g++.dg/spellcheck-pr77565.C b/gcc/testsuite/g++.dg/spellcheck-pr77565.C new file mode 100644 index 00000000000..2257f7b699d --- /dev/null +++ b/gcc/testsuite/g++.dg/spellcheck-pr77565.C @@ -0,0 +1,12 @@ +/* { dg-do compile } */ + +typdef int my_int; // { dg-error "1: 'typdef' does not name a type; did you mean 'typedef'\\?" } +inlien int i_fn(); // { dg-error "1: 'inlien' does not name a type; did you mean 'inline'\\?" } +coonst int ci = 1; // { dg-error "1: 'coonst' does not name a type; did you mean 'const'\\?" } +voltil int vi = 2; // { dg-error "1: 'voltil' does not name a type; did you mean 'volatile'\\?" } + +class my_class { + explict my_class(int); // { dg-error "3: 'explict' does not name a type; did you mean 'explicit'\\?" } + friends double f_fn(); // { dg-error "3: 'friends' does not name a type; did you mean 'friend'\\?" } + virtial double v_fn(); // { dg-error "3: 'virtial' does not name a type; did you mean 'virtual'\\?" } +}; -- 2.31.1 [-- Attachment #3: add_only.git_diff.txt --] [-- Type: text/plain, Size: 2421 bytes --] From e49d0fcfc616dcd90b7478bdf89466503a033b12 Mon Sep 17 00:00:00 2001 From: morinmorin <mimomorin@gmail.com> Date: Thu, 16 Sep 2021 23:29:54 +0900 Subject: [PATCH] c++: add spellcheck suggestions for typedef etc. [PR77565] cp_keyword_starts_decl_specifier_p misses many keywords that can start decl-specifiers. This patch adds support for those keywords. PR c++/77565 gcc/cp/ChangeLog: * parser.c (cp_keyword_starts_decl_specifier_p): Handle more decl-specifiers (typedef/inline/cv/explicit/virtual/friend). gcc/testsuite/ChangeLog: * g++.dg/spellcheck-pr77565.C: New test. --- gcc/cp/parser.c | 10 ++++++++++ gcc/testsuite/g++.dg/spellcheck-pr77565.C | 12 ++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr77565.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 8d60f40706b..40308d0d33f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -1051,6 +1051,16 @@ 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: + /* Function specifiers. */ + case RID_EXPLICIT: + case RID_VIRTUAL: + /* friend/typdef/inline specifiers. */ + case RID_FRIEND: + case RID_TYPEDEF: + case RID_INLINE: /* GNU extensions. */ case RID_ATTRIBUTE: case RID_TYPEOF: diff --git a/gcc/testsuite/g++.dg/spellcheck-pr77565.C b/gcc/testsuite/g++.dg/spellcheck-pr77565.C new file mode 100644 index 00000000000..2257f7b699d --- /dev/null +++ b/gcc/testsuite/g++.dg/spellcheck-pr77565.C @@ -0,0 +1,12 @@ +/* { dg-do compile } */ + +typdef int my_int; // { dg-error "1: 'typdef' does not name a type; did you mean 'typedef'\\?" } +inlien int i_fn(); // { dg-error "1: 'inlien' does not name a type; did you mean 'inline'\\?" } +coonst int ci = 1; // { dg-error "1: 'coonst' does not name a type; did you mean 'const'\\?" } +voltil int vi = 2; // { dg-error "1: 'voltil' does not name a type; did you mean 'volatile'\\?" } + +class my_class { + explict my_class(int); // { dg-error "3: 'explict' does not name a type; did you mean 'explicit'\\?" } + friends double f_fn(); // { dg-error "3: 'friends' does not name a type; did you mean 'friend'\\?" } + virtial double v_fn(); // { dg-error "3: 'virtial' does not name a type; did you mean 'virtual'\\?" } +}; -- 2.31.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-16 15:50 ` Michel Morin @ 2021-09-16 18:23 ` Jason Merrill 2021-09-17 17:31 ` Michel Morin 0 siblings, 1 reply; 13+ messages in thread From: Jason Merrill @ 2021-09-16 18:23 UTC (permalink / raw) To: Michel Morin; +Cc: David Malcolm, gcc-patches On 9/16/21 11:50, Michel Morin wrote: > On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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. > 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 >>>> >>>> >>> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-16 18:23 ` Jason Merrill @ 2021-09-17 17:31 ` Michel Morin 2021-09-20 20:24 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Michel Morin @ 2021-09-17 17:31 UTC (permalink / raw) To: Jason Merrill; +Cc: David Malcolm, gcc-patches [-- Attachment #1: Type: text/plain, Size: 9786 bytes --] On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <jason@redhat.com> wrote: > > On 9/16/21 11:50, Michel Morin wrote: > > On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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... 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 > >>>> > >>>> > >>> > >> > [-- Attachment #2: add_only.git_diff.txt --] [-- Type: text/plain, Size: 2421 bytes --] From e49d0fcfc616dcd90b7478bdf89466503a033b12 Mon Sep 17 00:00:00 2001 From: morinmorin <mimomorin@gmail.com> Date: Thu, 16 Sep 2021 23:29:54 +0900 Subject: [PATCH] c++: add spellcheck suggestions for typedef etc. [PR77565] cp_keyword_starts_decl_specifier_p misses many keywords that can start decl-specifiers. This patch adds support for those keywords. PR c++/77565 gcc/cp/ChangeLog: * parser.c (cp_keyword_starts_decl_specifier_p): Handle more decl-specifiers (typedef/inline/cv/explicit/virtual/friend). gcc/testsuite/ChangeLog: * g++.dg/spellcheck-pr77565.C: New test. --- gcc/cp/parser.c | 10 ++++++++++ gcc/testsuite/g++.dg/spellcheck-pr77565.C | 12 ++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr77565.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 8d60f40706b..40308d0d33f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -1051,6 +1051,16 @@ 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: + /* Function specifiers. */ + case RID_EXPLICIT: + case RID_VIRTUAL: + /* friend/typdef/inline specifiers. */ + case RID_FRIEND: + case RID_TYPEDEF: + case RID_INLINE: /* GNU extensions. */ case RID_ATTRIBUTE: case RID_TYPEOF: diff --git a/gcc/testsuite/g++.dg/spellcheck-pr77565.C b/gcc/testsuite/g++.dg/spellcheck-pr77565.C new file mode 100644 index 00000000000..2257f7b699d --- /dev/null +++ b/gcc/testsuite/g++.dg/spellcheck-pr77565.C @@ -0,0 +1,12 @@ +/* { dg-do compile } */ + +typdef int my_int; // { dg-error "1: 'typdef' does not name a type; did you mean 'typedef'\\?" } +inlien int i_fn(); // { dg-error "1: 'inlien' does not name a type; did you mean 'inline'\\?" } +coonst int ci = 1; // { dg-error "1: 'coonst' does not name a type; did you mean 'const'\\?" } +voltil int vi = 2; // { dg-error "1: 'voltil' does not name a type; did you mean 'volatile'\\?" } + +class my_class { + explict my_class(int); // { dg-error "3: 'explict' does not name a type; did you mean 'explicit'\\?" } + friends double f_fn(); // { dg-error "3: 'friends' does not name a type; did you mean 'friend'\\?" } + virtial double v_fn(); // { dg-error "3: 'virtial' does not name a type; did you mean 'virtual'\\?" } +}; -- 2.31.1 [-- Attachment #3: remove_attr.git_diff.txt --] [-- Type: text/plain, Size: 2405 bytes --] From d0721e86ad7ae80c80cd4bec024343b834723723 Mon Sep 17 00:00:00 2001 From: morinmorin <mimomorin@gmail.com> Date: Sat, 18 Sep 2021 01:45:12 +0900 Subject: [PATCH] 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) -- 2.31.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-17 17:31 ` Michel Morin @ 2021-09-20 20:24 ` Jason Merrill 2021-09-22 0:53 ` Michel Morin 0 siblings, 1 reply; 13+ messages in thread From: Jason Merrill @ 2021-09-20 20:24 UTC (permalink / raw) To: Michel Morin; +Cc: David Malcolm, gcc-patches On 9/17/21 13:31, Michel Morin wrote: > On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <jason@redhat.com> wrote: >> >> On 9/16/21 11:50, Michel Morin wrote: >>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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. 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. > 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 >>>>>> >>>>>> >>>>> >>>> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-20 20:24 ` Jason Merrill @ 2021-09-22 0:53 ` Michel Morin 2021-09-22 20:08 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Michel Morin @ 2021-09-22 0:53 UTC (permalink / raw) To: Jason Merrill; +Cc: David Malcolm, gcc-patches [-- Attachment #1: Type: text/plain, Size: 11205 bytes --] On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill <jason@redhat.com> wrote: > > On 9/17/21 13:31, Michel Morin wrote: > > On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <jason@redhat.com> wrote: > >> > >> On 9/16/21 11:50, Michel Morin wrote: > >>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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! > > > 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 > >>>>>> > >>>>>> > >>>>> > >>>> > >> > [-- Attachment #2: 0001-add-typedef-etc.git-patch.txt --] [-- Type: text/plain, Size: 2426 bytes --] From 1edc3b6afe58b7ac5f88c586490b79bad793aafa Mon Sep 17 00:00:00 2001 From: morinmorin <mimomorin@gmail.com> Date: Thu, 16 Sep 2021 23:29:54 +0900 Subject: [PATCH 1/2] c++: add spellcheck suggestions for typedef etc. [PR77565] cp_keyword_starts_decl_specifier_p misses many keywords that can start decl-specifiers. This patch adds support for those keywords. PR c++/77565 gcc/cp/ChangeLog: * parser.c (cp_keyword_starts_decl_specifier_p): Handle more decl-specifiers (typedef/inline/cv/explicit/virtual/friend). gcc/testsuite/ChangeLog: * g++.dg/spellcheck-pr77565.C: New test. --- gcc/cp/parser.c | 10 ++++++++++ gcc/testsuite/g++.dg/spellcheck-pr77565.C | 12 ++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr77565.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 62908daa5b7..40569fc410a 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -1051,6 +1051,16 @@ 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: + /* Function specifiers. */ + case RID_EXPLICIT: + case RID_VIRTUAL: + /* friend/typdef/inline specifiers. */ + case RID_FRIEND: + case RID_TYPEDEF: + case RID_INLINE: /* GNU extensions. */ case RID_ATTRIBUTE: case RID_TYPEOF: diff --git a/gcc/testsuite/g++.dg/spellcheck-pr77565.C b/gcc/testsuite/g++.dg/spellcheck-pr77565.C new file mode 100644 index 00000000000..2257f7b699d --- /dev/null +++ b/gcc/testsuite/g++.dg/spellcheck-pr77565.C @@ -0,0 +1,12 @@ +/* { dg-do compile } */ + +typdef int my_int; // { dg-error "1: 'typdef' does not name a type; did you mean 'typedef'\\?" } +inlien int i_fn(); // { dg-error "1: 'inlien' does not name a type; did you mean 'inline'\\?" } +coonst int ci = 1; // { dg-error "1: 'coonst' does not name a type; did you mean 'const'\\?" } +voltil int vi = 2; // { dg-error "1: 'voltil' does not name a type; did you mean 'volatile'\\?" } + +class my_class { + explict my_class(int); // { dg-error "3: 'explict' does not name a type; did you mean 'explicit'\\?" } + friends double f_fn(); // { dg-error "3: 'friends' does not name a type; did you mean 'friend'\\?" } + virtial double v_fn(); // { dg-error "3: 'virtial' does not name a type; did you mean 'virtual'\\?" } +}; -- 2.31.1 [-- Attachment #3: 0002-remove-gnu-attr.git-patch.txt --] [-- Type: text/plain, Size: 3148 bytes --] From ad640e12ab78c582f3e83ae2f38b830308542030 Mon Sep 17 00:00:00 2001 From: morinmorin <mimomorin@gmail.com> Date: Wed, 22 Sep 2021 08:04:31 +0900 Subject: [PATCH 2/2] 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. (cp_parser_lambda_declarator_opt): Likewise. --- gcc/cp/parser.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 40569fc410a..f5ec7688213 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: @@ -11466,8 +11465,7 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) /* In the decl-specifier-seq of the lambda-declarator, each decl-specifier shall either be mutable or constexpr. */ int declares_class_or_enum; - if (cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer) - && !cp_next_tokens_can_be_gnu_attribute_p (parser)) + if (cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)) cp_parser_decl_specifier_seq (parser, CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR, &lambda_specs, &declares_class_or_enum); @@ -30835,23 +30833,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) -- 2.31.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-22 0:53 ` Michel Morin @ 2021-09-22 20:08 ` Jason Merrill 2021-09-22 23:05 ` Michel Morin 0 siblings, 1 reply; 13+ messages in thread From: Jason Merrill @ 2021-09-22 20:08 UTC (permalink / raw) To: Michel Morin; +Cc: David Malcolm, gcc-patches On 9/21/21 20:53, Michel Morin wrote: > On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill <jason@redhat.com> wrote: >> >> On 9/17/21 13:31, Michel Morin wrote: >>> On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <jason@redhat.com> wrote: >>>> >>>> On 9/16/21 11:50, Michel Morin wrote: >>>>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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? >>> 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 >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-22 20:08 ` Jason Merrill @ 2021-09-22 23:05 ` Michel Morin 2021-09-23 20:29 ` Jason Merrill 0 siblings, 1 reply; 13+ messages in thread From: Michel Morin @ 2021-09-22 23:05 UTC (permalink / raw) To: Jason Merrill; +Cc: David Malcolm, gcc-patches On Thu, Sep 23, 2021 at 5:09 AM Jason Merrill <jason@redhat.com> wrote: > > On 9/21/21 20:53, Michel Morin wrote: > > On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill <jason@redhat.com> wrote: > >> > >> On 9/17/21 13:31, Michel Morin wrote: > >>> On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <jason@redhat.com> wrote: > >>>> > >>>> On 9/16/21 11:50, Michel Morin wrote: > >>>>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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 > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>> > >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-22 23:05 ` Michel Morin @ 2021-09-23 20:29 ` Jason Merrill 2021-09-24 0:49 ` Michel Morin 0 siblings, 1 reply; 13+ messages in thread From: Jason Merrill @ 2021-09-23 20:29 UTC (permalink / raw) To: Michel Morin; +Cc: David Malcolm, gcc-patches List On Wed, Sep 22, 2021 at 7:05 PM Michel Morin <mimomorin@gmail.com> wrote: On Thu, Sep 23, 2021 at 5:09 AM Jason Merrill <jason@redhat.com> wrote: > > On 9/21/21 20:53, Michel Morin wrote: > > On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill <jason@redhat.com> wrote: > >> > >> On 9/17/21 13:31, Michel Morin wrote: > >>> On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <jason@redhat.com> wrote: > >>>> > >>>> On 9/16/21 11:50, Michel Morin wrote: > >>>>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <jason@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 <dmalcolm@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. Ah! Sorry, I was confusing you with Mikael Morin <mikael.morin@sfr.fr>, 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] 2021-09-23 20:29 ` Jason Merrill @ 2021-09-24 0:49 ` Michel Morin 0 siblings, 0 replies; 13+ messages in thread From: Michel Morin @ 2021-09-24 0:49 UTC (permalink / raw) To: Jason Merrill; +Cc: David Malcolm, gcc-patches List On Fri, Sep 24, 2021 at 5:29 AM Jason Merrill <jason@redhat.com> wrote: > > On Wed, Sep 22, 2021 at 7:05 PM Michel Morin <mimomorin@gmail.com> wrote: > > On Thu, Sep 23, 2021 at 5:09 AM Jason Merrill <jason@redhat.com> wrote: > > > > On 9/21/21 20:53, Michel Morin wrote: > > > On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill > <jason@redhat.com> wrote: > > >> > > >> On 9/17/21 13:31, Michel Morin wrote: > > >>> On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill > <jason@redhat.com> wrote: > > >>>> > > >>>> On 9/16/21 11:50, Michel Morin wrote: > > >>>>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill > <jason@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 > <dmalcolm@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. > > > Ah! Sorry, I was confusing you with Mikael Morin <mikael.morin@sfr.fr>, > 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-09-24 0:49 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-13 18:35 [PATCH] c++: fix wrong fixit hints for misspelled typedef [PR77565] Michel Morin 2021-09-13 22:14 ` David Malcolm 2021-09-14 8:29 ` Michel Morin 2021-09-15 20:43 ` Jason Merrill 2021-09-16 15:50 ` Michel Morin 2021-09-16 18:23 ` Jason Merrill 2021-09-17 17:31 ` Michel Morin 2021-09-20 20:24 ` Jason Merrill 2021-09-22 0:53 ` Michel Morin 2021-09-22 20:08 ` Jason Merrill 2021-09-22 23:05 ` Michel Morin 2021-09-23 20:29 ` Jason Merrill 2021-09-24 0:49 ` Michel Morin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).