Re: [PATCH] spellcheck bugfixes: don't offer the goal string as a suggestion

2016-11-28 Thread Jeff Law

On 11/15/2016 02:52 PM, David Malcolm wrote:

This patch addresses various bugs in the spellcheck code in which
the goal string somehow makes it into the candidate list.
The goal string will always have an edit distance of 0 to itself, and
thus is the "closest" string to the goal, but offering it as a
suggestion will always be nonsensical e.g.
  'constexpr' does not name a type; did you mean 'constexpr'?

Ultimately such suggestions are due to bugs in constructing the
candidate list.

As a band-aid, the patch updates
best_match::get_best_meaningful_candidate so that we no longer
offer suggestions for the case where the edit distance == 0
(where candidate == goal).

Doing so fixes PR c++/72786, PR c++/77922, and PR c++/78313.

I looked at fixing the candidate list in each of these bugs.

PR c++/72786 (macro defined after use): this occurs because we're
using the set of macro names at the end of parsing, rather than
at the point of parsing the site of the would-be macro usage.
A better fix would be to indicate this, but would be somewhat
invasive, needing a new internal API (perhaps too much for
stage 3?) so hopefully the band-aid is good enough for GCC 7.

PR c++/77922: the patch updates C++'s lookup_name_fuzzy to only
suggest reserved words that are matched by "-std=" etc, which
thus eliminating bogus words from the candidate list.

PR c++/78313: I attempted to prune the candidate list here, but it
led to a worse message (see the comment in that bug), hence I'd
prefer to rely on the best_match::get_best_meaningful_candidate
fix for this one.

Successfully bootstrapped on x86_64-pc-linux-gnu; adds
26 PASS results to g++.sum.

OK for trunk?

gcc/cp/ChangeLog:
PR c++/77922
* name-lookup.c (lookup_name_fuzzy): Filter out reserved words
that were filtered out by init_reswords.

gcc/ChangeLog:
PR c++/72774
PR c++/72786
PR c++/77922
PR c++/78313
* spellcheck.c (selftest::test_find_closest_string): Verify that
we don't offer the goal string as a suggestion.
* spellcheck.h (best_match::get_best_meaningful_candidate): Don't
offer the goal string as a suggestion.

gcc/testsuite/ChangeLog:
PR c++/72774
PR c++/72786
PR c++/77922
PR c++/78313
* g++.dg/spellcheck-c++-11-keyword.C: New test case.
* g++.dg/spellcheck-macro-ordering.C: New test case.
* g++.dg/spellcheck-pr78313.C: New test case.

OK.  Sorry for the delays.

jeff



[PATCH] spellcheck bugfixes: don't offer the goal string as a suggestion

2016-11-15 Thread David Malcolm
This patch addresses various bugs in the spellcheck code in which
the goal string somehow makes it into the candidate list.
The goal string will always have an edit distance of 0 to itself, and
thus is the "closest" string to the goal, but offering it as a
suggestion will always be nonsensical e.g.
  'constexpr' does not name a type; did you mean 'constexpr'?

Ultimately such suggestions are due to bugs in constructing the
candidate list.

As a band-aid, the patch updates
best_match::get_best_meaningful_candidate so that we no longer
offer suggestions for the case where the edit distance == 0
(where candidate == goal).

Doing so fixes PR c++/72786, PR c++/77922, and PR c++/78313.

I looked at fixing the candidate list in each of these bugs.

PR c++/72786 (macro defined after use): this occurs because we're
using the set of macro names at the end of parsing, rather than
at the point of parsing the site of the would-be macro usage.
A better fix would be to indicate this, but would be somewhat
invasive, needing a new internal API (perhaps too much for
stage 3?) so hopefully the band-aid is good enough for GCC 7.

PR c++/77922: the patch updates C++'s lookup_name_fuzzy to only
suggest reserved words that are matched by "-std=" etc, which
thus eliminating bogus words from the candidate list.

PR c++/78313: I attempted to prune the candidate list here, but it
led to a worse message (see the comment in that bug), hence I'd
prefer to rely on the best_match::get_best_meaningful_candidate
fix for this one.

Successfully bootstrapped on x86_64-pc-linux-gnu; adds
26 PASS results to g++.sum.

OK for trunk?

gcc/cp/ChangeLog:
PR c++/77922
* name-lookup.c (lookup_name_fuzzy): Filter out reserved words
that were filtered out by init_reswords.

gcc/ChangeLog:
PR c++/72774
PR c++/72786
PR c++/77922
PR c++/78313
* spellcheck.c (selftest::test_find_closest_string): Verify that
we don't offer the goal string as a suggestion.
* spellcheck.h (best_match::get_best_meaningful_candidate): Don't
offer the goal string as a suggestion.

gcc/testsuite/ChangeLog:
PR c++/72774
PR c++/72786
PR c++/77922
PR c++/78313
* g++.dg/spellcheck-c++-11-keyword.C: New test case.
* g++.dg/spellcheck-macro-ordering.C: New test case.
* g++.dg/spellcheck-pr78313.C: New test case.
---
 gcc/cp/name-lookup.c |  6 ++
 gcc/spellcheck.c |  5 +
 gcc/spellcheck.h | 10 ++
 gcc/testsuite/g++.dg/spellcheck-c++-11-keyword.C | 15 +++
 gcc/testsuite/g++.dg/spellcheck-macro-ordering.C | 15 +++
 gcc/testsuite/g++.dg/spellcheck-pr78313.C| 11 +++
 6 files changed, 62 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-c++-11-keyword.C
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-macro-ordering.C
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr78313.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 7ad65b8..84e064d 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4811,6 +4811,12 @@ lookup_name_fuzzy (tree name, enum 
lookup_name_fuzzy_kind kind)
   if (!resword_identifier)
continue;
   gcc_assert (TREE_CODE (resword_identifier) == IDENTIFIER_NODE);
+
+  /* Only consider reserved words that survived the
+filtering in init_reswords (e.g. for -std).  */
+  if (!C_IS_RESERVED_WORD (resword_identifier))
+   continue;
+
   bm.consider (resword_identifier);
 }
 
diff --git a/gcc/spellcheck.c b/gcc/spellcheck.c
index b37b1e4..86cdee1 100644
--- a/gcc/spellcheck.c
+++ b/gcc/spellcheck.c
@@ -210,6 +210,11 @@ test_find_closest_string ()
   ASSERT_STREQ ("banana", find_closest_string ("banyan", ));
   ASSERT_STREQ ("cherry", find_closest_string ("berry", ));
   ASSERT_EQ (NULL, find_closest_string ("not like the others", ));
+
+  /* If the goal string somehow makes it into the candidate list, offering
+ it as a suggestion will be nonsensical.  Verify that we don't offer such
+ suggestions.  */
+  ASSERT_EQ (NULL, find_closest_string ("banana", ));
 }
 
 /* Test data for test_metric_conditions.  */
diff --git a/gcc/spellcheck.h b/gcc/spellcheck.h
index b48cfbc..41c9308 100644
--- a/gcc/spellcheck.h
+++ b/gcc/spellcheck.h
@@ -165,6 +165,16 @@ class best_match
if (m_best_distance > cutoff)
  return NULL;
 }
+
+/* If the goal string somehow makes it into the candidate list, offering
+   it as a suggestion will be nonsensical e.g.
+ 'constexpr' does not name a type; did you mean 'constexpr'?
+   Ultimately such suggestions are due to bugs in constructing the
+   candidate list, but as a band-aid, do not offer suggestions for
+   distance == 0 (where candidate == goal).  */
+if (m_best_distance == 0)
+  return NULL;
+
 return