* [PATCH] Fix spelling suggestions for reserved words (PR c++/80177)
@ 2017-03-31 16:28 David Malcolm
2017-04-24 19:27 ` [PING for gcc 8] " David Malcolm
0 siblings, 1 reply; 3+ messages in thread
From: David Malcolm @ 2017-03-31 16:28 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 7521 bytes --]
As noted in the PR, the C++ frontend currently offers a poor
suggestion for this misspelling:
a.C: In function âvoid f()â:
a.C:3:3: error: âstatic_assertionâ was not declared in this scope
static_assertion (1 == 0, "1 == 0");
^~~~~~~~~~~~~~~~
a.C:3:3: note: suggested alternative: â__cpp_static_assertâ
static_assertion (1 == 0, "1 == 0");
^~~~~~~~~~~~~~~~
__cpp_static_assert
when it ought to offer "static_assert" as a suggestion instead.
The root causes are two issues within lookup_name_fuzzy
(called here with FUZZY_LOOKUP_NAME):
(a) If it finds a good enough match in the preprocessor it will
return the best match *before* considering reserved words,
rather than picking the closest match overall.
The fix is to have merge all the results into one best_match
instance, and pick the overall winner. However, given that
some candidates are identifiers (trees), and others are cpp
macros, the best_match instance's candidate type needs to
be converted from tree to const char *. This has some minor
knock-on effects within name-lookup.c. Sadly it means some
extra calls to strlen (one per candidate), but this will be
purely when error-handling.
(b) It rejects "static_assert" here:
4998 if (!cp_keyword_starts_decl_specifier_p (resword->rid))
4999 continue;
as "static_assert" doesn't start a decl specifier.
The fix is to only apply this rejection criterion if we're looking
for typenames, rather than for names in general.
This patch addresses both issues and adds test coverage.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
Adds 7 PASS and 1 UNSUPPORTED (for -std=c++98) to g++.sum
OK for next stage 1?
gcc/cp/ChangeLog:
PR c++/80177
* name-lookup.c (suggest_alternative_in_explicit_scope): Convert
candidate type of bm from tree to const char *.
(consider_binding_level): Likewise.
(lookup_name_fuzzy): Likewise, using this to merge the best
result from the preprocessor into bm, rather than immediately
returning, so that better matches from reserved words can "win".
Guard the rejection of keywords that don't start decl-specifiers
so it only happens for FUZZY_LOOKUP_TYPENAME.
gcc/testsuite/ChangeLog:
PR c++/80177
* g++.dg/spellcheck-pr80177.C: New test case.
---
gcc/cp/name-lookup.c | 37 +++++++++++++------------------
gcc/testsuite/g++.dg/spellcheck-pr80177.C | 7 ++++++
2 files changed, 23 insertions(+), 21 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr80177.C
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 994f7f0..16ec0a1 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -48,7 +48,8 @@ static bool lookup_using_namespace (tree, struct scope_binding *, tree,
tree, int);
static bool qualified_lookup_using_namespace (tree, tree,
struct scope_binding *, int);
-static void consider_binding_level (tree name, best_match <tree, tree> &bm,
+static void consider_binding_level (tree name,
+ best_match <tree, const char *> &bm,
cp_binding_level *lvl,
bool look_within_fields,
enum lookup_name_fuzzy_kind kind);
@@ -4550,14 +4551,13 @@ suggest_alternative_in_explicit_scope (location_t location, tree name,
cp_binding_level *level = NAMESPACE_LEVEL (scope);
- best_match <tree, tree> bm (name);
+ best_match <tree, const char *> bm (name);
consider_binding_level (name, bm, level, false, FUZZY_LOOKUP_NAME);
/* See if we have a good suggesion for the user. */
- tree best_id = bm.get_best_meaningful_candidate ();
- if (best_id)
+ const char *fuzzy_name = bm.get_best_meaningful_candidate ();
+ if (fuzzy_name)
{
- const char *fuzzy_name = IDENTIFIER_POINTER (best_id);
gcc_rich_location richloc (location);
richloc.add_fixit_replace (fuzzy_name);
inform_at_rich_loc (&richloc, "suggested alternative: %qs",
@@ -4797,7 +4797,7 @@ qualified_lookup_using_namespace (tree name, tree scope,
Traverse binding level LVL, looking for good name matches for NAME
(and BM). */
static void
-consider_binding_level (tree name, best_match <tree, tree> &bm,
+consider_binding_level (tree name, best_match <tree, const char *> &bm,
cp_binding_level *lvl, bool look_within_fields,
enum lookup_name_fuzzy_kind kind)
{
@@ -4809,7 +4809,7 @@ consider_binding_level (tree name, best_match <tree, tree> &bm,
tree best_matching_field
= lookup_member_fuzzy (type, name, want_type_p);
if (best_matching_field)
- bm.consider (best_matching_field);
+ bm.consider (IDENTIFIER_POINTER (best_matching_field));
}
for (tree t = lvl->names; t; t = TREE_CHAIN (t))
@@ -4838,7 +4838,7 @@ consider_binding_level (tree name, best_match <tree, tree> &bm,
if (tree name = DECL_NAME (d))
/* Ignore internal names with spaces in them. */
if (!strchr (IDENTIFIER_POINTER (name), ' '))
- bm.consider (DECL_NAME (d));
+ bm.consider (IDENTIFIER_POINTER (name));
}
}
@@ -4851,7 +4851,7 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
{
gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
- best_match <tree, tree> bm (name);
+ best_match <tree, const char *> bm (name);
cp_binding_level *lvl;
for (lvl = scope_chain->class_bindings; lvl; lvl = lvl->level_chain)
@@ -4874,9 +4874,9 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
the identifiers already checked. */
best_macro_match bmm (name, bm.get_best_distance (), parse_in);
cpp_hashnode *best_macro = bmm.get_best_meaningful_candidate ();
- /* If a macro is the closest so far to NAME, suggest it. */
+ /* If a macro is the closest so far to NAME, consider it. */
if (best_macro)
- return (const char *)best_macro->ident.str;
+ bm.consider ((const char *)best_macro->ident.str);
/* Try the "starts_decl_specifier_p" keywords to detect
"singed" vs "signed" typos. */
@@ -4884,8 +4884,9 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
{
const c_common_resword *resword = &c_common_reswords[i];
- if (!cp_keyword_starts_decl_specifier_p (resword->rid))
- continue;
+ if (kind == FUZZY_LOOKUP_TYPENAME)
+ if (!cp_keyword_starts_decl_specifier_p (resword->rid))
+ continue;
tree resword_identifier = ridpointers [resword->rid];
if (!resword_identifier)
@@ -4897,16 +4898,10 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
if (!C_IS_RESERVED_WORD (resword_identifier))
continue;
- bm.consider (resword_identifier);
+ bm.consider (IDENTIFIER_POINTER (resword_identifier));
}
- /* See if we have a good suggesion for the user. */
- tree best_id = bm.get_best_meaningful_candidate ();
- if (best_id)
- return IDENTIFIER_POINTER (best_id);
-
- /* No meaningful suggestion available. */
- return NULL;
+ return bm.get_best_meaningful_candidate ();
}
/* Subroutine of outer_binding.
diff --git a/gcc/testsuite/g++.dg/spellcheck-pr80177.C b/gcc/testsuite/g++.dg/spellcheck-pr80177.C
new file mode 100644
index 0000000..2ff24e8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-pr80177.C
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+void pr80177 ()
+{
+ static_assertion (1 == 0, "1 == 0"); // { dg-error "3: 'static_assertion' was not declared in this scope" }
+ // { dg-message "3: suggested alternative: 'static_assert'" "" { target *-*-* } .-1 }
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PING for gcc 8] Re: [PATCH] Fix spelling suggestions for reserved words (PR c++/80177)
2017-03-31 16:28 [PATCH] Fix spelling suggestions for reserved words (PR c++/80177) David Malcolm
@ 2017-04-24 19:27 ` David Malcolm
2017-04-25 11:34 ` Nathan Sidwell
0 siblings, 1 reply; 3+ messages in thread
From: David Malcolm @ 2017-04-24 19:27 UTC (permalink / raw)
To: gcc-patches
Ping for gcc 8.
On Fri, 2017-03-31 at 12:41 -0400, David Malcolm wrote:
> As noted in the PR, the C++ frontend currently offers a poor
> suggestion for this misspelling:
>
> a.C: In function âvoid f()â:
> a.C:3:3: error: âstatic_assertionâ was not declared in this scope
> static_assertion (1 == 0, "1 == 0");
> ^~~~~~~~~~~~~~~~
> a.C:3:3: note: suggested alternative: â__cpp_static_assertâ
> static_assertion (1 == 0, "1 == 0");
> ^~~~~~~~~~~~~~~~
> __cpp_static_assert
>
> when it ought to offer "static_assert" as a suggestion instead.
>
> The root causes are two issues within lookup_name_fuzzy
> (called here with FUZZY_LOOKUP_NAME):
>
> (a) If it finds a good enough match in the preprocessor it will
> return the best match *before* considering reserved words,
> rather than picking the closest match overall.
>
> The fix is to have merge all the results into one best_match
> instance, and pick the overall winner. However, given that
> some candidates are identifiers (trees), and others are cpp
> macros, the best_match instance's candidate type needs to
> be converted from tree to const char *. This has some minor
> knock-on effects within name-lookup.c. Sadly it means some
> extra calls to strlen (one per candidate), but this will be
> purely when error-handling.
>
> (b) It rejects "static_assert" here:
>
> 4998 if (!cp_keyword_starts_decl_specifier_p (resword->rid))
> 4999 continue;
>
> as "static_assert" doesn't start a decl specifier.
>
> The fix is to only apply this rejection criterion if we're
> looking
> for typenames, rather than for names in general.
>
> This patch addresses both issues and adds test coverage.
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
> Adds 7 PASS and 1 UNSUPPORTED (for -std=c++98) to g++.sum
>
> OK for next stage 1?
>
> gcc/cp/ChangeLog:
> PR c++/80177
> * name-lookup.c (suggest_alternative_in_explicit_scope):
> Convert
> candidate type of bm from tree to const char *.
> (consider_binding_level): Likewise.
> (lookup_name_fuzzy): Likewise, using this to merge the best
> result from the preprocessor into bm, rather than immediately
> returning, so that better matches from reserved words can
> "win".
> Guard the rejection of keywords that don't start decl
> -specifiers
> so it only happens for FUZZY_LOOKUP_TYPENAME.
>
> gcc/testsuite/ChangeLog:
> PR c++/80177
> * g++.dg/spellcheck-pr80177.C: New test case.
> ---
> gcc/cp/name-lookup.c | 37 +++++++++++++--------
> ----------
> gcc/testsuite/g++.dg/spellcheck-pr80177.C | 7 ++++++
> 2 files changed, 23 insertions(+), 21 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr80177.C
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 994f7f0..16ec0a1 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -48,7 +48,8 @@ static bool lookup_using_namespace (tree, struct
> scope_binding *, tree,
> tree, int);
> static bool qualified_lookup_using_namespace (tree, tree,
> struct scope_binding
> *, int);
> -static void consider_binding_level (tree name, best_match <tree,
> tree> &bm,
> +static void consider_binding_level (tree name,
> + best_match <tree, const char *>
> &bm,
> cp_binding_level *lvl,
> bool look_within_fields,
> enum lookup_name_fuzzy_kind
> kind);
> @@ -4550,14 +4551,13 @@ suggest_alternative_in_explicit_scope
> (location_t location, tree name,
>
> cp_binding_level *level = NAMESPACE_LEVEL (scope);
>
> - best_match <tree, tree> bm (name);
> + best_match <tree, const char *> bm (name);
> consider_binding_level (name, bm, level, false,
> FUZZY_LOOKUP_NAME);
>
> /* See if we have a good suggesion for the user. */
> - tree best_id = bm.get_best_meaningful_candidate ();
> - if (best_id)
> + const char *fuzzy_name = bm.get_best_meaningful_candidate ();
> + if (fuzzy_name)
> {
> - const char *fuzzy_name = IDENTIFIER_POINTER (best_id);
> gcc_rich_location richloc (location);
> richloc.add_fixit_replace (fuzzy_name);
> inform_at_rich_loc (&richloc, "suggested alternative: %qs",
> @@ -4797,7 +4797,7 @@ qualified_lookup_using_namespace (tree name,
> tree scope,
> Traverse binding level LVL, looking for good name matches for
> NAME
> (and BM). */
> static void
> -consider_binding_level (tree name, best_match <tree, tree> &bm,
> +consider_binding_level (tree name, best_match <tree, const char *>
> &bm,
> cp_binding_level *lvl, bool
> look_within_fields,
> enum lookup_name_fuzzy_kind kind)
> {
> @@ -4809,7 +4809,7 @@ consider_binding_level (tree name, best_match
> <tree, tree> &bm,
> tree best_matching_field
> = lookup_member_fuzzy (type, name, want_type_p);
> if (best_matching_field)
> - bm.consider (best_matching_field);
> + bm.consider (IDENTIFIER_POINTER (best_matching_field));
> }
>
> for (tree t = lvl->names; t; t = TREE_CHAIN (t))
> @@ -4838,7 +4838,7 @@ consider_binding_level (tree name, best_match
> <tree, tree> &bm,
> if (tree name = DECL_NAME (d))
> /* Ignore internal names with spaces in them. */
> if (!strchr (IDENTIFIER_POINTER (name), ' '))
> - bm.consider (DECL_NAME (d));
> + bm.consider (IDENTIFIER_POINTER (name));
> }
> }
>
> @@ -4851,7 +4851,7 @@ lookup_name_fuzzy (tree name, enum
> lookup_name_fuzzy_kind kind)
> {
> gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
>
> - best_match <tree, tree> bm (name);
> + best_match <tree, const char *> bm (name);
>
> cp_binding_level *lvl;
> for (lvl = scope_chain->class_bindings; lvl; lvl = lvl
> ->level_chain)
> @@ -4874,9 +4874,9 @@ lookup_name_fuzzy (tree name, enum
> lookup_name_fuzzy_kind kind)
> the identifiers already checked. */
> best_macro_match bmm (name, bm.get_best_distance (), parse_in);
> cpp_hashnode *best_macro = bmm.get_best_meaningful_candidate ();
> - /* If a macro is the closest so far to NAME, suggest it. */
> + /* If a macro is the closest so far to NAME, consider it. */
> if (best_macro)
> - return (const char *)best_macro->ident.str;
> + bm.consider ((const char *)best_macro->ident.str);
>
> /* Try the "starts_decl_specifier_p" keywords to detect
> "singed" vs "signed" typos. */
> @@ -4884,8 +4884,9 @@ lookup_name_fuzzy (tree name, enum
> lookup_name_fuzzy_kind kind)
> {
> const c_common_resword *resword = &c_common_reswords[i];
>
> - if (!cp_keyword_starts_decl_specifier_p (resword->rid))
> - continue;
> + if (kind == FUZZY_LOOKUP_TYPENAME)
> + if (!cp_keyword_starts_decl_specifier_p (resword->rid))
> + continue;
>
> tree resword_identifier = ridpointers [resword->rid];
> if (!resword_identifier)
> @@ -4897,16 +4898,10 @@ lookup_name_fuzzy (tree name, enum
> lookup_name_fuzzy_kind kind)
> if (!C_IS_RESERVED_WORD (resword_identifier))
> continue;
>
> - bm.consider (resword_identifier);
> + bm.consider (IDENTIFIER_POINTER (resword_identifier));
> }
>
> - /* See if we have a good suggesion for the user. */
> - tree best_id = bm.get_best_meaningful_candidate ();
> - if (best_id)
> - return IDENTIFIER_POINTER (best_id);
> -
> - /* No meaningful suggestion available. */
> - return NULL;
> + return bm.get_best_meaningful_candidate ();
> }
>
> /* Subroutine of outer_binding.
> diff --git a/gcc/testsuite/g++.dg/spellcheck-pr80177.C
> b/gcc/testsuite/g++.dg/spellcheck-pr80177.C
> new file mode 100644
> index 0000000..2ff24e8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/spellcheck-pr80177.C
> @@ -0,0 +1,7 @@
> +// { dg-do compile { target c++11 } }
> +
> +void pr80177 ()
> +{
> + static_assertion (1 == 0, "1 == 0"); // { dg-error "3:
> 'static_assertion' was not declared in this scope" }
> + // { dg-message "3: suggested alternative: 'static_assert'" "" {
> target *-*-* } .-1 }
> +}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PING for gcc 8] Re: [PATCH] Fix spelling suggestions for reserved words (PR c++/80177)
2017-04-24 19:27 ` [PING for gcc 8] " David Malcolm
@ 2017-04-25 11:34 ` Nathan Sidwell
0 siblings, 0 replies; 3+ messages in thread
From: Nathan Sidwell @ 2017-04-25 11:34 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 04/24/2017 02:58 PM, David Malcolm wrote:
> Ping for gcc 8.
>
> On Fri, 2017-03-31 at 12:41 -0400, David Malcolm wrote:
>> As noted in the PR, the C++ frontend currently offers a poor
>> suggestion for this misspelling:
>> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>> Adds 7 PASS and 1 UNSUPPORTED (for -std=c++98) to g++.sum
>>
>> OK for next stage 1?
Ok.
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-25 11:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-31 16:28 [PATCH] Fix spelling suggestions for reserved words (PR c++/80177) David Malcolm
2017-04-24 19:27 ` [PING for gcc 8] " David Malcolm
2017-04-25 11:34 ` Nathan Sidwell
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).