| Message ID | 1479246750-22427-1-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
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®rtested 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
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", &candidates)); ASSERT_STREQ ("cherry", find_closest_string ("berry", &candidates)); ASSERT_EQ (NULL, find_closest_string ("not like the others", &candidates)); + + /* 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", &candidates)); } /* 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 m_best_candidate; } diff --git a/gcc/testsuite/g++.dg/spellcheck-c++-11-keyword.C b/gcc/testsuite/g++.dg/spellcheck-c++-11-keyword.C new file mode 100644 index 0000000..0984af9 --- /dev/null +++ b/gcc/testsuite/g++.dg/spellcheck-c++-11-keyword.C @@ -0,0 +1,15 @@ +/* c++/77922: "constexpr" is only available from C++11 onwards. + We shouldn't offer it as a spellcheck suggestion in C++98. */ +// { dg-options "-std=c++98" } + +constexpr int a = 1; // { dg-bogus "did you mean" } +// { dg-error ".constexpr. does not name a type" "" { target *-*-* } .-1 } +// { dg-message "C\\+\\+11 .constexpr. only available with -std=c\\+\\+11 or -std=gnu\\+\\+11" "" { target *-*-* } .-2 } + +/* If the user typos "constexpr" (here as "consexpr"), don't offer it as a + spelling suggestion in C++98 mode. */ +consexpr int a = 1; // { dg-bogus "did you mean" } +// { dg-error ".consexpr. does not name a type" "" { target *-*-* } .-1 } + +decltype i = 0; // { dg-bogus "did you mean" } +// { dg-error ".decltype. does not name a type" "" { target *-*-* } .-1 } diff --git a/gcc/testsuite/g++.dg/spellcheck-macro-ordering.C b/gcc/testsuite/g++.dg/spellcheck-macro-ordering.C new file mode 100644 index 0000000..3b888c6 --- /dev/null +++ b/gcc/testsuite/g++.dg/spellcheck-macro-ordering.C @@ -0,0 +1,15 @@ +// PR c++/72786 + +/* Example of a macro-ordering issue, where the use is before the defn. */ + +class DocTargetDriver { + virtual void clone() const OVERRIDE { } + /* Offering "OVERRIDE" as a spelling suggestion for "OVERRIDE" would be + nonsensical. */ + // { dg-bogus "did you mean" "" { target *-*-* } .-3 } + // { dg-error "expected .;. at end of member declaration" "" { target *-*-* } .-4 } + // { dg-error ".OVERRIDE. does not name a type" "" { target *-*-* } .-5 } +}; + +#define OVERRIDE override + diff --git a/gcc/testsuite/g++.dg/spellcheck-pr78313.C b/gcc/testsuite/g++.dg/spellcheck-pr78313.C new file mode 100644 index 0000000..e34176d --- /dev/null +++ b/gcc/testsuite/g++.dg/spellcheck-pr78313.C @@ -0,0 +1,11 @@ +// PR c++/78313 (see also PR c++/72774) +// { dg-do compile } + +void baz (); +namespace A { void foo (); } +void bar () +{ + using A::foo; + 0 ? static_cast<foo> (0) : baz; // { dg-bogus "did you mean" } + // { dg-error "does not name a type" "" { target *-*-* } .-1 } +}