On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill <[email protected]> wrote:
>
> On 9/17/21 13:31, Michel Morin wrote:
> > On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill <[email protected]> wrote:
> >>
> >> On 9/16/21 11:50, Michel Morin wrote:
> >>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill <[email protected]> wrote:
> >>>>
> >>>> On 9/14/21 04:29, Michel Morin via Gcc-patches wrote:
> >>>>> On Tue, Sep 14, 2021 at 7:14 AM David Malcolm <[email protected]>
> >>>>> 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
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>
>
From 1edc3b6afe58b7ac5f88c586490b79bad793aafa Mon Sep 17 00:00:00 2001
From: morinmorin <[email protected]>
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
From ad640e12ab78c582f3e83ae2f38b830308542030 Mon Sep 17 00:00:00 2001
From: morinmorin <[email protected]>
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
