* [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
@ 2017-11-22 16:35 David Malcolm
2017-12-11 16:10 ` PING: " David Malcolm
2017-12-11 22:25 ` Jason Merrill
0 siblings, 2 replies; 9+ messages in thread
From: David Malcolm @ 2017-11-22 16:35 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
lookup_name_fuzzy can offer some reserved words as suggestions for
misspelled words, helping with "singed"/"signed" typos.
PR c++/81610 and PR c++/80567 report problems where the C++ frontend
suggested "if", "for" and "else" as corrections for misspelled variable
names.
The root cause is that in r247233
("Fix spelling suggestions for reserved words (PR c++/80177)")
I loosened the conditions on these reserved words, adding this condition:
if (kind == FUZZY_LOOKUP_TYPENAME)
to the logic for rejecting words that don't start decl-specifiers, to
allow for "static_assert" to be offered.
This is too loose a condition: we don't want to suggest *any* reserved word
when we're in a context where we don't know we expect a typename.
For the kinds of error-recover situations where we're suggesting
spelling corrections we don't have much contextual information, so it
seems prudent to be stricter about which reserved words we offer
as spelling suggestions; I don't think it makes sense for us to
suggest e.g. "for".
This patch implements that by effectively reinstating the old logic,
but special-casing RID_STATIC_ASSERT, moving the logic to a new
subroutine (in case we want to allow for other special-cases).
I attempted to add suggestions for the various RID_*CAST, to cope
with e.g. "reinterptet_cast" (I can never type that correctly on the
first try), but the following '<' token confuses the error-recovery
enough that the suggestion code isn't triggered.
Hence this more minimal fix.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/81610
PR c++/80567
* name-lookup.c (suggest_rid_p): New function.
(lookup_name_fuzzy): Replace enum-rid-filtering logic with call to
suggest_rid_p.
gcc/testsuite/ChangeLog:
PR c++/81610
PR c++/80567
* g++.dg/spellcheck-reswords.C: New test case.
* g++.dg/spellcheck-stdlib.C: Remove xfail from dg-bogus
suggestion of "if".
---
gcc/cp/name-lookup.c | 31 +++++++++++++++++++++++++++---
gcc/testsuite/g++.dg/spellcheck-reswords.C | 11 +++++++++++
gcc/testsuite/g++.dg/spellcheck-stdlib.C | 2 +-
3 files changed, 40 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/spellcheck-reswords.C
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 7c363b0..a96be46 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5671,6 +5671,32 @@ class macro_use_before_def : public deferred_diagnostic
cpp_hashnode *m_macro;
};
+/* Determine if it can ever make sense to offer RID as a suggestion for
+ a misspelling.
+
+ Subroutine of lookup_name_fuzzy. */
+
+static bool
+suggest_rid_p (enum rid rid)
+{
+ switch (rid)
+ {
+ /* Support suggesting function-like keywords. */
+ case RID_STATIC_ASSERT:
+ return true;
+
+ default:
+ /* Support suggesting the various decl-specifier words, to handle
+ e.g. "singed" vs "signed" typos. */
+ if (cp_keyword_starts_decl_specifier_p (rid))
+ return true;
+
+ /* Otherwise, don't offer it. This avoids suggesting e.g. "if"
+ and "do" for short misspellings, which are likely to lead to
+ nonsensical results. */
+ return false;
+ }
+}
/* Search for near-matches for NAME within the current bindings, and within
macro names, returning the best match as a const char *, or NULL if
@@ -5735,9 +5761,8 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
{
const c_common_resword *resword = &c_common_reswords[i];
- if (kind == FUZZY_LOOKUP_TYPENAME)
- if (!cp_keyword_starts_decl_specifier_p (resword->rid))
- continue;
+ if (!suggest_rid_p (resword->rid))
+ continue;
tree resword_identifier = ridpointers [resword->rid];
if (!resword_identifier)
diff --git a/gcc/testsuite/g++.dg/spellcheck-reswords.C b/gcc/testsuite/g++.dg/spellcheck-reswords.C
new file mode 100644
index 0000000..db6104b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-reswords.C
@@ -0,0 +1,11 @@
+void pr81610 (void *p)
+{
+ forget (p); // { dg-error "not declared" }
+ // { dg-bogus "'for'" "" { target *-*-*} .-1 }
+}
+
+void pr80567 (void *p)
+{
+ memset (p, 0, 4); // { dg-error "not declared" }
+ // { dg-bogus "'else'" "" { target *-*-*} .-1 }
+}
diff --git a/gcc/testsuite/g++.dg/spellcheck-stdlib.C b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
index 6e6ab1d..c7a6626 100644
--- a/gcc/testsuite/g++.dg/spellcheck-stdlib.C
+++ b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
@@ -16,7 +16,7 @@ void test_cstdio (void)
FILE *f; // { dg-error "'FILE' was not declared in this scope" }
// { dg-message "'FILE' is defined in header '<cstdio>'; did you forget to '#include <cstdio>'?" "" { target *-*-* } .-1 }
// { dg-error "'f' was not declared in this scope" "" { target *-*-* } .-2 }
- // { dg-bogus "suggested alternative: 'if'" "PR c++/80567" { xfail *-*-* } .-3 }
+ // { dg-bogus "suggested alternative: 'if'" "PR c++/80567" { target *-*-* } .-3 }
char buf[BUFSIZ]; // { dg-error "'BUFSIZ' was not declared" }
// { dg-message "'BUFSIZ' is defined in header '<cstdio>'; did you forget to '#include <cstdio>'?" "" { target *-*-* } .-1 }
--
1.8.5.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* PING: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2017-11-22 16:35 [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567) David Malcolm
@ 2017-12-11 16:10 ` David Malcolm
2017-12-11 22:25 ` Jason Merrill
1 sibling, 0 replies; 9+ messages in thread
From: David Malcolm @ 2017-12-11 16:10 UTC (permalink / raw)
To: gcc-patches
Ping: https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
On Wed, 2017-11-22 at 10:36 -0500, David Malcolm wrote:
> lookup_name_fuzzy can offer some reserved words as suggestions for
> misspelled words, helping with "singed"/"signed" typos.
>
> PR c++/81610 and PR c++/80567 report problems where the C++ frontend
> suggested "if", "for" and "else" as corrections for misspelled
> variable
> names.
>
> The root cause is that in r247233
> ("Fix spelling suggestions for reserved words (PR c++/80177)")
> I loosened the conditions on these reserved words, adding this
> condition:
> if (kind == FUZZY_LOOKUP_TYPENAME)
> to the logic for rejecting words that don't start decl-specifiers, to
> allow for "static_assert" to be offered.
>
> This is too loose a condition: we don't want to suggest *any*
> reserved word
> when we're in a context where we don't know we expect a typename.
>
> For the kinds of error-recover situations where we're suggesting
> spelling corrections we don't have much contextual information, so it
> seems prudent to be stricter about which reserved words we offer
> as spelling suggestions; I don't think it makes sense for us to
> suggest e.g. "for".
>
> This patch implements that by effectively reinstating the old logic,
> but special-casing RID_STATIC_ASSERT, moving the logic to a new
> subroutine (in case we want to allow for other special-cases).
>
> I attempted to add suggestions for the various RID_*CAST, to cope
> with e.g. "reinterptet_cast" (I can never type that correctly on the
> first try), but the following '<' token confuses the error-recovery
> enough that the suggestion code isn't triggered.
>
> Hence this more minimal fix.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/81610
> PR c++/80567
> * name-lookup.c (suggest_rid_p): New function.
> (lookup_name_fuzzy): Replace enum-rid-filtering logic with call
> to
> suggest_rid_p.
>
> gcc/testsuite/ChangeLog:
> PR c++/81610
> PR c++/80567
> * g++.dg/spellcheck-reswords.C: New test case.
> * g++.dg/spellcheck-stdlib.C: Remove xfail from dg-bogus
> suggestion of "if".
> ---
> gcc/cp/name-lookup.c | 31
> +++++++++++++++++++++++++++---
> gcc/testsuite/g++.dg/spellcheck-reswords.C | 11 +++++++++++
> gcc/testsuite/g++.dg/spellcheck-stdlib.C | 2 +-
> 3 files changed, 40 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/spellcheck-reswords.C
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 7c363b0..a96be46 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -5671,6 +5671,32 @@ class macro_use_before_def : public
> deferred_diagnostic
> cpp_hashnode *m_macro;
> };
>
> +/* Determine if it can ever make sense to offer RID as a suggestion
> for
> + a misspelling.
> +
> + Subroutine of lookup_name_fuzzy. */
> +
> +static bool
> +suggest_rid_p (enum rid rid)
> +{
> + switch (rid)
> + {
> + /* Support suggesting function-like keywords. */
> + case RID_STATIC_ASSERT:
> + return true;
> +
> + default:
> + /* Support suggesting the various decl-specifier words, to
> handle
> + e.g. "singed" vs "signed" typos. */
> + if (cp_keyword_starts_decl_specifier_p (rid))
> + return true;
> +
> + /* Otherwise, don't offer it. This avoids suggesting e.g.
> "if"
> + and "do" for short misspellings, which are likely to lead
> to
> + nonsensical results. */
> + return false;
> + }
> +}
>
> /* Search for near-matches for NAME within the current bindings, and
> within
> macro names, returning the best match as a const char *, or NULL
> if
> @@ -5735,9 +5761,8 @@ lookup_name_fuzzy (tree name, enum
> lookup_name_fuzzy_kind kind, location_t loc)
> {
> const c_common_resword *resword = &c_common_reswords[i];
>
> - if (kind == FUZZY_LOOKUP_TYPENAME)
> - if (!cp_keyword_starts_decl_specifier_p (resword->rid))
> - continue;
> + if (!suggest_rid_p (resword->rid))
> + continue;
>
> tree resword_identifier = ridpointers [resword->rid];
> if (!resword_identifier)
> diff --git a/gcc/testsuite/g++.dg/spellcheck-reswords.C
> b/gcc/testsuite/g++.dg/spellcheck-reswords.C
> new file mode 100644
> index 0000000..db6104b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/spellcheck-reswords.C
> @@ -0,0 +1,11 @@
> +void pr81610 (void *p)
> +{
> + forget (p); // { dg-error "not declared" }
> + // { dg-bogus "'for'" "" { target *-*-*} .-1 }
> +}
> +
> +void pr80567 (void *p)
> +{
> + memset (p, 0, 4); // { dg-error "not declared" }
> + // { dg-bogus "'else'" "" { target *-*-*} .-1 }
> +}
> diff --git a/gcc/testsuite/g++.dg/spellcheck-stdlib.C
> b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
> index 6e6ab1d..c7a6626 100644
> --- a/gcc/testsuite/g++.dg/spellcheck-stdlib.C
> +++ b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
> @@ -16,7 +16,7 @@ void test_cstdio (void)
> FILE *f; // { dg-error "'FILE' was not declared in this scope" }
> // { dg-message "'FILE' is defined in header '<cstdio>'; did you
> forget to '#include <cstdio>'?" "" { target *-*-* } .-1 }
> // { dg-error "'f' was not declared in this scope" "" { target *-
> *-* } .-2 }
> - // { dg-bogus "suggested alternative: 'if'" "PR c++/80567" { xfail
> *-*-* } .-3 }
> + // { dg-bogus "suggested alternative: 'if'" "PR c++/80567" {
> target *-*-* } .-3 }
>
> char buf[BUFSIZ]; // { dg-error "'BUFSIZ' was not declared" }
> // { dg-message "'BUFSIZ' is defined in header '<cstdio>'; did you
> forget to '#include <cstdio>'?" "" { target *-*-* } .-1 }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2017-11-22 16:35 [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567) David Malcolm
2017-12-11 16:10 ` PING: " David Malcolm
@ 2017-12-11 22:25 ` Jason Merrill
2018-01-26 20:15 ` David Malcolm
1 sibling, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2017-12-11 22:25 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches List
On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> PR c++/81610 and PR c++/80567 report problems where the C++ frontend
> suggested "if", "for" and "else" as corrections for misspelled variable
> names.
Hmm, what about cases where people are actually misspelling keywords?
Don't we want to handle that?
fi (true) { }
retrun 42;
In the PRs you mention, the actual identifiers are 1) missing
includes, which we should check first, and 2) pretty far from the
suggested keywords.
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2017-12-11 22:25 ` Jason Merrill
@ 2018-01-26 20:15 ` David Malcolm
2018-02-02 21:18 ` [PING] " David Malcolm
2018-02-07 17:21 ` Jason Merrill
0 siblings, 2 replies; 9+ messages in thread
From: David Malcolm @ 2018-01-26 20:15 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Mon, 2017-12-11 at 17:24 -0500, Jason Merrill wrote:
> On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@redhat.com>
> wrote:
Original post:
https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
> > PR c++/81610 and PR c++/80567 report problems where the C++
> > frontend
> > suggested "if", "for" and "else" as corrections for misspelled
> > variable
> > names.
I've now marked these PRs as regressions: the nonsensical suggestions
are only offered by trunk, not by gcc 7 and earlier.
> Hmm, what about cases where people are actually misspelling keywords?
> Don't we want to handle that?
>
> fi (true) { }
> retrun 42;
I'd prefer not to.
gcc 7 and earlier don't attempt to correct the spelling of the "fi" and
"retrun" above.
trunk currently does offer "return" as a suggestion, but it was by
accident, and I'm wary of attempting to support these corrections: is
"fi" meant to be an "if", or a function call that's missing its decl,
or a name lookup issue? ...etc
> In the PRs you mention, the actual identifiers are 1) missing
> includes, which we should check first, and 2) pretty far from the
> suggested keywords.
The C++ FE is missing a suggestion about which #include to use for
"memset", but I'd prefer to treat that as a follow-up patch (and
probably for next stage 1).
In the meantime, is this patch OK for trunk? (as a regression fix)
Thanks
Dave
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PING] Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2018-01-26 20:15 ` David Malcolm
@ 2018-02-02 21:18 ` David Malcolm
2018-02-07 17:21 ` Jason Merrill
1 sibling, 0 replies; 9+ messages in thread
From: David Malcolm @ 2018-02-02 21:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
Ping
On Fri, 2018-01-26 at 13:12 -0500, David Malcolm wrote:
> On Mon, 2017-12-11 at 17:24 -0500, Jason Merrill wrote:
> > On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@redhat.co
> > m>
> > wrote:
>
> Original post:
> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
>
> > > PR c++/81610 and PR c++/80567 report problems where the C++
> > > frontend
> > > suggested "if", "for" and "else" as corrections for misspelled
> > > variable
> > > names.
>
> I've now marked these PRs as regressions: the nonsensical suggestions
> are only offered by trunk, not by gcc 7 and earlier.
>
> > Hmm, what about cases where people are actually misspelling
> > keywords?
> > Don't we want to handle that?
> >
> > fi (true) { }
> > retrun 42;
>
> I'd prefer not to.
>
> gcc 7 and earlier don't attempt to correct the spelling of the "fi"
> and
> "retrun" above.
>
> trunk currently does offer "return" as a suggestion, but it was by
> accident, and I'm wary of attempting to support these corrections: is
> "fi" meant to be an "if", or a function call that's missing its decl,
> or a name lookup issue? ...etc
>
> > In the PRs you mention, the actual identifiers are 1) missing
> > includes, which we should check first, and 2) pretty far from the
> > suggested keywords.
>
> The C++ FE is missing a suggestion about which #include to use for
> "memset", but I'd prefer to treat that as a follow-up patch (and
> probably for next stage 1).
>
> In the meantime, is this patch OK for trunk? (as a regression fix)
>
> Thanks
> Dave
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2018-01-26 20:15 ` David Malcolm
2018-02-02 21:18 ` [PING] " David Malcolm
@ 2018-02-07 17:21 ` Jason Merrill
2018-02-07 18:12 ` David Malcolm
1 sibling, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2018-02-07 17:21 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches List
On Fri, Jan 26, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Mon, 2017-12-11 at 17:24 -0500, Jason Merrill wrote:
>> On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>
> Original post:
> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
>
>> > PR c++/81610 and PR c++/80567 report problems where the C++
>> > frontend
>> > suggested "if", "for" and "else" as corrections for misspelled
>> > variable
>> > names.
>
> I've now marked these PRs as regressions: the nonsensical suggestions
> are only offered by trunk, not by gcc 7 and earlier.
>
>> Hmm, what about cases where people are actually misspelling keywords?
>> Don't we want to handle that?
>>
>> fi (true) { }
>> retrun 42;
>
> I'd prefer not to.
>
> gcc 7 and earlier don't attempt to correct the spelling of the "fi" and
> "retrun" above.
>
> trunk currently does offer "return" as a suggestion, but it was by
> accident, and I'm wary of attempting to support these corrections: is
> "fi" meant to be an "if", or a function call that's missing its decl,
> or a name lookup issue? ...etc
>
>> In the PRs you mention, the actual identifiers are 1) missing
>> includes, which we should check first, and 2) pretty far from the
>> suggested keywords.
>
> The C++ FE is missing a suggestion about which #include to use for
> "memset", but I'd prefer to treat that as a follow-up patch (and
> probably for next stage 1).
>
> In the meantime, is this patch OK for trunk? (as a regression fix)
Yes.
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2018-02-07 17:21 ` Jason Merrill
@ 2018-02-07 18:12 ` David Malcolm
2018-02-07 18:22 ` Jason Merrill
0 siblings, 1 reply; 9+ messages in thread
From: David Malcolm @ 2018-02-07 18:12 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Wed, 2018-02-07 at 12:21 -0500, Jason Merrill wrote:
> On Fri, Jan 26, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > On Mon, 2017-12-11 at 17:24 -0500, Jason Merrill wrote:
> > > On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@redhat.
> > > com>
> > > wrote:
> >
> > Original post:
> > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
> >
> > > > PR c++/81610 and PR c++/80567 report problems where the C++
> > > > frontend
> > > > suggested "if", "for" and "else" as corrections for misspelled
> > > > variable
> > > > names.
> >
> > I've now marked these PRs as regressions: the nonsensical
> > suggestions
> > are only offered by trunk, not by gcc 7 and earlier.
> >
> > > Hmm, what about cases where people are actually misspelling
> > > keywords?
> > > Don't we want to handle that?
> > >
> > > fi (true) { }
> > > retrun 42;
> >
> > I'd prefer not to.
> >
> > gcc 7 and earlier don't attempt to correct the spelling of the "fi"
> > and
> > "retrun" above.
> >
> > trunk currently does offer "return" as a suggestion, but it was by
> > accident, and I'm wary of attempting to support these corrections:
> > is
> > "fi" meant to be an "if", or a function call that's missing its
> > decl,
> > or a name lookup issue? ...etc
> >
> > > In the PRs you mention, the actual identifiers are 1) missing
> > > includes, which we should check first, and 2) pretty far from the
> > > suggested keywords.
> >
> > The C++ FE is missing a suggestion about which #include to use for
> > "memset", but I'd prefer to treat that as a follow-up patch (and
> > probably for next stage 1).
> >
> > In the meantime, is this patch OK for trunk? (as a regression fix)
>
> Yes.
Thanks; committed (r257456).
FWIW, I've filed PR c++/84269 so I remember to fix the missing
suggestion for "memset" (in gcc 9 stage1).
Dave
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2018-02-07 18:12 ` David Malcolm
@ 2018-02-07 18:22 ` Jason Merrill
2018-02-07 18:28 ` David Malcolm
0 siblings, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2018-02-07 18:22 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches List
On Wed, Feb 7, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Wed, 2018-02-07 at 12:21 -0500, Jason Merrill wrote:
>> On Fri, Jan 26, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>> > On Mon, 2017-12-11 at 17:24 -0500, Jason Merrill wrote:
>> > > On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@redhat.
>> > > com>
>> > > wrote:
>> >
>> > Original post:
>> > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
>> >
>> > > > PR c++/81610 and PR c++/80567 report problems where the C++
>> > > > frontend
>> > > > suggested "if", "for" and "else" as corrections for misspelled
>> > > > variable
>> > > > names.
>> >
>> > I've now marked these PRs as regressions: the nonsensical
>> > suggestions
>> > are only offered by trunk, not by gcc 7 and earlier.
>> >
>> > > Hmm, what about cases where people are actually misspelling
>> > > keywords?
>> > > Don't we want to handle that?
>> > >
>> > > fi (true) { }
>> > > retrun 42;
>> >
>> > I'd prefer not to.
>> >
>> > gcc 7 and earlier don't attempt to correct the spelling of the "fi"
>> > and
>> > "retrun" above.
>> >
>> > trunk currently does offer "return" as a suggestion, but it was by
>> > accident, and I'm wary of attempting to support these corrections:
>> > is
>> > "fi" meant to be an "if", or a function call that's missing its
>> > decl,
>> > or a name lookup issue? ...etc
>> >
>> > > In the PRs you mention, the actual identifiers are 1) missing
>> > > includes, which we should check first, and 2) pretty far from the
>> > > suggested keywords.
>> >
>> > The C++ FE is missing a suggestion about which #include to use for
>> > "memset", but I'd prefer to treat that as a follow-up patch (and
>> > probably for next stage 1).
>> >
>> > In the meantime, is this patch OK for trunk? (as a regression fix)
>>
>> Yes.
>
> Thanks; committed (r257456).
>
> FWIW, I've filed PR c++/84269 so I remember to fix the missing
> suggestion for "memset" (in gcc 9 stage1).
Did you have a reaction to my comment about the suggested keyword
being pretty far from the actual identifier? Do we want to lower the
cutoff for suggestions at all?
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
2018-02-07 18:22 ` Jason Merrill
@ 2018-02-07 18:28 ` David Malcolm
0 siblings, 0 replies; 9+ messages in thread
From: David Malcolm @ 2018-02-07 18:28 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Wed, 2018-02-07 at 13:22 -0500, Jason Merrill wrote:
> On Wed, Feb 7, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > On Wed, 2018-02-07 at 12:21 -0500, Jason Merrill wrote:
> > > On Fri, Jan 26, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.c
> > > om>
> > > wrote:
> > > > On Mon, 2017-12-11 at 17:24 -0500, Jason Merrill wrote:
> > > > > On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@red
> > > > > hat.
> > > > > com>
> > > > > wrote:
> > > >
> > > > Original post:
> > > > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
> > > >
> > > > > > PR c++/81610 and PR c++/80567 report problems where the C++
> > > > > > frontend
> > > > > > suggested "if", "for" and "else" as corrections for
> > > > > > misspelled
> > > > > > variable
> > > > > > names.
> > > >
> > > > I've now marked these PRs as regressions: the nonsensical
> > > > suggestions
> > > > are only offered by trunk, not by gcc 7 and earlier.
> > > >
> > > > > Hmm, what about cases where people are actually misspelling
> > > > > keywords?
> > > > > Don't we want to handle that?
> > > > >
> > > > > fi (true) { }
> > > > > retrun 42;
> > > >
> > > > I'd prefer not to.
> > > >
> > > > gcc 7 and earlier don't attempt to correct the spelling of the
> > > > "fi"
> > > > and
> > > > "retrun" above.
> > > >
> > > > trunk currently does offer "return" as a suggestion, but it was
> > > > by
> > > > accident, and I'm wary of attempting to support these
> > > > corrections:
> > > > is
> > > > "fi" meant to be an "if", or a function call that's missing its
> > > > decl,
> > > > or a name lookup issue? ...etc
> > > >
> > > > > In the PRs you mention, the actual identifiers are 1) missing
> > > > > includes, which we should check first, and 2) pretty far from
> > > > > the
> > > > > suggested keywords.
> > > >
> > > > The C++ FE is missing a suggestion about which #include to use
> > > > for
> > > > "memset", but I'd prefer to treat that as a follow-up patch
> > > > (and
> > > > probably for next stage 1).
> > > >
> > > > In the meantime, is this patch OK for trunk? (as a regression
> > > > fix)
> > >
> > > Yes.
> >
> > Thanks; committed (r257456).
> >
> > FWIW, I've filed PR c++/84269 so I remember to fix the missing
> > suggestion for "memset" (in gcc 9 stage1).
>
> Did you have a reaction to my comment about the suggested keyword
> being pretty far from the actual identifier? Do we want to lower the
> cutoff for suggestions at all?
I've played around with tweaking how the cutoff works, [1] in response
to e.g. PR c/82967 ('"did you mean" suggestions are way too
suggestive'), but I've not yet come up with a version I prefer to the
current implementation.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-07 18:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-22 16:35 [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567) David Malcolm
2017-12-11 16:10 ` PING: " David Malcolm
2017-12-11 22:25 ` Jason Merrill
2018-01-26 20:15 ` David Malcolm
2018-02-02 21:18 ` [PING] " David Malcolm
2018-02-07 17:21 ` Jason Merrill
2018-02-07 18:12 ` David Malcolm
2018-02-07 18:22 ` Jason Merrill
2018-02-07 18:28 ` David Malcolm
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).