From patchwork Tue Nov 15 21:52:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 695275 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tJL0124Gtz9t0p for ; Wed, 16 Nov 2016 08:20:32 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="b+OYVuBX"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id; q=dns; s=default; b=s9Aeq5tXSevl fVHLj+Cmjk2Dl+OriuZyqaG1kLQof5pz+MGEahKlf5oY4WBhcdEiT2ogIJ2fGyLC 6ltRc08kRB+eR2a2/zbrdau6LOekB/+nzvKK8ib08/PShJ/9OuHRR7tDG2QwLCHs 67RSL90MPD7/wbAMCWXOEN1xgbojtPo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id; s=default; bh=P7VFhpitF3UUMpNJo/ e4nnf7DVc=; b=b+OYVuBXcUCCdylig7w2hsDDhQ0lFeBOtKpaKY/b37OIA00Ld2 bpfUbPCOdrU3Hhpv71+WdKa5+wF1sn3rZdR0qdYsT/0vWC16JEMMq0Eo/8uH2OXp ekdq/xqS9OOO+G6vTnijmTbhZftVVJ7gX/ICz30Q2nDUno4bcCPLdJFFA= Received: (qmail 96472 invoked by alias); 15 Nov 2016 21:20:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 96463 invoked by uid 89); 15 Nov 2016 21:20:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=1853, 1.8.5.3, berry, survived X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Nov 2016 21:20:20 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6F31267BAB for ; Tue, 15 Nov 2016 21:20:19 +0000 (UTC) Received: from c64.redhat.com (vpn-226-193.phx2.redhat.com [10.3.226.193]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAFLKInn026843; Tue, 15 Nov 2016 16:20:18 -0500 From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [PATCH] spellcheck bugfixes: don't offer the goal string as a suggestion Date: Tue, 15 Nov 2016 16:52:30 -0500 Message-Id: <1479246750-22427-1-git-send-email-dmalcolm@redhat.com> X-IsSubscribed: yes 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. --- 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", &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 (0) : baz; // { dg-bogus "did you mean" } + // { dg-error "does not name a type" "" { target *-*-* } .-1 } +}