From patchwork Thu Dec 2 19:10:44 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Froyd X-Patchwork-Id: 74012 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]) by ozlabs.org (Postfix) with SMTP id E00DD1007D2 for ; Fri, 3 Dec 2010 06:11:04 +1100 (EST) Received: (qmail 10197 invoked by alias); 2 Dec 2010 19:11:00 -0000 Received: (qmail 10185 invoked by uid 22791); 2 Dec 2010 19:10:56 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL, BAYES_00, TW_CX, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 02 Dec 2010 19:10:49 +0000 Received: (qmail 21649 invoked from network); 2 Dec 2010 19:10:46 -0000 Received: from unknown (HELO codesourcery.com) (froydnj@127.0.0.2) by mail.codesourcery.com with ESMTPA; 2 Dec 2010 19:10:46 -0000 Date: Thu, 2 Dec 2010 14:10:44 -0500 From: Nathan Froyd To: Mark Mitchell Cc: gcc-patches@gcc.gnu.org, jason@redhat.com Subject: Re: [PATCH, c++] fix PR 45330, suggest alternatives for failed name lookups Message-ID: <20101202191043.GC18963@nightcrawler> References: <20101110181356.GM7991@nightcrawler> <4CE33E77.3090604@codesourcery.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4CE33E77.3090604@codesourcery.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes 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 On Tue, Nov 16, 2010 at 06:31:19PM -0800, Mark Mitchell wrote: > On 11/10/2010 10:13 AM, Nathan Froyd wrote: > I thought about this for a while from two angles: > > (a) Will that be incredibly slow? > (b) Is there any reasonable way to limit the set of namespaces searched? > > I decided (a) doesn't really matter; we're issuing an error, worst-case > we probably have a few thousand namespaces, should be OK. Yeah, I was worried about this too, but decided it didn't matter. > And, of course, we could refuse to search more than 100 namespaces, if > we wanted. I think you should probably add a --param > (max-namespace-for-diagnostic-help) for that, and start with 1000 as > the default, just to ensure sanity? New param added, cxx-max-namespaces-for-diagnostic-help. Making sure to consult this meant that it was somewhat easier to turn the suggestion code into an iterative process rather than a recursive process, so I did that. > > I didn't include a testcase, as the testcase tweaks indicated there are > > a number of testcases testing similar things. > > OK, but can you make at least one of the testcases check for the proper > alternative name? (Right now, it looks like you're just checking the > line numbers.) I went ahead and updated all the testcases to check for the names (particularly good for those tests where we search multiple namespaces). I also added a new testcase to make sure we handle the new param correctly. Tested on x86_64-linux-gnu. OK to commit? -Nathan gcc/ PR c++/45330 * params.def (CXX_MAX_NAMESPACES_FOR_DIAGNOSTIC_HELP): New parameter. * doc/invoke.texi (cxx-max-namespaces-for-diagnostic-help): Document. gcc/cp/ PR c++/45330 * cp-tree.h (suggest_alternatives_for): Declare. * error.c (dump_expr): Handle TYPE_DECL. * name-lookup.c (suggest_alternatives_for): New function. * lex.c (unqualified_name_lookup_error): Call it. gcc/testsuite/ PR c++/45330 * g++.dg/pr45330.C: New test. * g++.dg/ext/builtin3.C: Adjust. * g++.dg/lookup/error1.C: Adjust. * g++.dg/lookup/koenig5.C: Adjust. * g++.dg/overload/koenig1.C: Adjust. * g++.dg/parse/decl-specifier-1.C: Adjust. * g++.dg/template/static10.C: Adjust. * g++.old-deja/g++.mike/ns5.C: Adjust. * g++.old-deja/g++.mike/ns7.C: Adjust. * g++.old-deja/g++.ns/koenig5.C: Adjust. * g++.old-deja/g++.ns/koenig9.C: Adjust. * g++.old-deja/g++.other/lineno5.C: Adjust. diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 23f594c..8bee63d 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5639,6 +5639,9 @@ extern tree cxx_omp_clause_dtor (tree, tree); extern void cxx_omp_finish_clause (tree); extern bool cxx_omp_privatize_by_reference (const_tree); +/* in name-lookup.c */ +extern void suggest_alternatives_for (tree); + /* -- end of C++ */ #endif /* ! GCC_CP_TREE_H */ diff --git a/gcc/cp/error.c b/gcc/cp/error.c index 2676966..db01ba4 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -1699,6 +1699,7 @@ dump_expr (tree t, int flags) case NAMESPACE_DECL: case LABEL_DECL: case OVERLOAD: + case TYPE_DECL: case IDENTIFIER_NODE: dump_decl (t, (flags & ~TFF_DECL_SPECIFIERS) | TFF_NO_FUNCTION_ARGUMENTS); break; diff --git a/gcc/cp/lex.c b/gcc/cp/lex.c index 5484317..084a04b 100644 --- a/gcc/cp/lex.c +++ b/gcc/cp/lex.c @@ -449,7 +449,10 @@ unqualified_name_lookup_error (tree name) else { if (!objc_diagnose_private_ivar (name)) - error ("%qD was not declared in this scope", name); + { + error ("%qD was not declared in this scope", name); + suggest_alternatives_for (name); + } /* Prevent repeated error messages by creating a VAR_DECL with this NAME in the innermost block scope. */ if (current_function_decl) diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 3d19c08..9db6c13 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -29,8 +29,10 @@ along with GCC; see the file COPYING3. If not see #include "name-lookup.h" #include "timevar.h" #include "diagnostic-core.h" +#include "intl.h" #include "debug.h" #include "c-family/c-pragma.h" +#include "params.h" /* The bindings for a particular name in a particular scope. */ @@ -3916,6 +3918,74 @@ remove_hidden_names (tree fns) return fns; } +/* Suggest alternatives for NAME, an IDENTIFIER_NODE for which name + lookup failed. Search through all available namespaces and print out + possible candidates. */ + +void +suggest_alternatives_for (tree name) +{ + VEC(tree,heap) *candidates = NULL; + VEC(tree,heap) *namespaces_to_search = NULL; + int max_to_search = PARAM_VALUE (CXX_MAX_NAMESPACES_FOR_DIAGNOSTIC_HELP); + int n_searched = 0; + char *spaces; + const char *str; + tree t; + unsigned ix; + + VEC_safe_push (tree, heap, namespaces_to_search, global_namespace); + + while (!VEC_empty (tree, namespaces_to_search) + && n_searched < max_to_search) + { + tree scope = VEC_pop (tree, namespaces_to_search); + struct scope_binding binding = EMPTY_SCOPE_BINDING; + struct cp_binding_level *level = NAMESPACE_LEVEL (scope); + + /* Look in this namespace. */ + qualified_lookup_using_namespace (name, scope, &binding, 0); + + n_searched++; + + if (binding.value) + VEC_safe_push (tree, heap, candidates, binding.value); + + /* Add child namespaces. */ + for (t = level->namespaces; t; t = DECL_CHAIN (t)) + VEC_safe_push (tree, heap, namespaces_to_search, t); + } + + /* If we stopped before we could examine all namespaces, inform the + user. Do this even if we don't have any candidates, since there + might be more candidates further down that we weren't able to + find. */ + if (n_searched >= max_to_search) + inform (input_location, + "maximum limit of %d namespaces searched for %qE", + max_to_search, name); + + VEC_free (tree, heap, namespaces_to_search); + + /* Nothing useful to report. */ + if (VEC_empty (tree, candidates)) + return; + + str = (VEC_length(tree, candidates) > 1 + ? _("suggested alternatives:") + : _("suggested alternative:")); + spaces = NULL; + + FOR_EACH_VEC_ELT (tree, candidates, ix, t) + { + inform (input_location, "%s %qE", (spaces ? spaces : str), t); + spaces = spaces ? spaces : get_spaces (str); + } + + VEC_free (tree, heap, candidates); + free (spaces); +} + /* Unscoped lookup of a global: iterate over current namespaces, considering using-directives. */ diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 94e8160..b03629c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -8817,6 +8817,10 @@ Size of minimal paritition for WHOPR (in estimated instructions). This prevents expenses of splitting very small programs into too many partitions. +@item cxx-max-namespaces-for-diagnostic-help +The maximum number of namespaces to consult for suggestions when C++ +name lookup fails for an identifier. The default is 1000. + @end table @end table diff --git a/gcc/params.def b/gcc/params.def index 6b6e055..2ea0013 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -855,6 +855,15 @@ DEFPARAM (MIN_PARTITION_SIZE, "lto-min-partition", "Size of minimal paritition for WHOPR (in estimated instructions)", 1000, 0, 0) + +/* Diagnostic parameters. */ + +DEFPARAM (CXX_MAX_NAMESPACES_FOR_DIAGNOSTIC_HELP, + "cxx-max-namespaces-for-diagnostic-help", + "Maximum number of namespaces to search for alternatives when " + "name lookup fails", + 1000, 0, 0) + /* Local variables: mode:c diff --git a/gcc/testsuite/g++.dg/ext/builtin3.C b/gcc/testsuite/g++.dg/ext/builtin3.C index 3d06dd7..b2ec9be 100644 --- a/gcc/testsuite/g++.dg/ext/builtin3.C +++ b/gcc/testsuite/g++.dg/ext/builtin3.C @@ -10,4 +10,5 @@ extern "C" int printf(char*, ...); void foo() { printf("abc"); // { dg-error "not declared" } + // { dg-message "std::printf" "suggested alternative" { target *-*-* } 12 } } diff --git a/gcc/testsuite/g++.dg/lookup/error1.C b/gcc/testsuite/g++.dg/lookup/error1.C index 2264b23..1778dbb 100644 --- a/gcc/testsuite/g++.dg/lookup/error1.C +++ b/gcc/testsuite/g++.dg/lookup/error1.C @@ -4,6 +4,7 @@ namespace N { int i; } void foo() { i; } // { dg-error "not declared" } + // { dg-message "N::i" "suggested alternative" { target *-*-* } 6 } using namespace N; void bar() { i; } diff --git a/gcc/testsuite/g++.dg/lookup/koenig5.C b/gcc/testsuite/g++.dg/lookup/koenig5.C index 6ecc25d..1202cd7 100644 --- a/gcc/testsuite/g++.dg/lookup/koenig5.C +++ b/gcc/testsuite/g++.dg/lookup/koenig5.C @@ -32,10 +32,12 @@ void g (N::A *a, M::B *b, O::C *c) One (a); // ok One (a, b); // ok One (b); // { dg-error "not declared" } + // { dg-message "\[NM\]::One" "suggested alternative" { target *-*-* } 34 } Two (c); // ok Two (a, c); // ok Two (a); // { dg-error "not declared" } + // { dg-message "\[NMO\]::Two" "suggested alternative" { target *-*-* } 39 } Two (a, a); // error masked by earlier error Two (b); // error masked by earlier error Two (a, b); // error masked by earlier error @@ -43,4 +45,5 @@ void g (N::A *a, M::B *b, O::C *c) Three (b); // ok Three (a, b); // ok Three (a); // { dg-error "not declared" } + // { dg-message "\[NM\]::Three" "suggested alternative" { target *-*-* } 47 } } diff --git a/gcc/testsuite/g++.dg/overload/koenig1.C b/gcc/testsuite/g++.dg/overload/koenig1.C index 1ed7bce..0a64674 100644 --- a/gcc/testsuite/g++.dg/overload/koenig1.C +++ b/gcc/testsuite/g++.dg/overload/koenig1.C @@ -14,5 +14,6 @@ void g () B *bp; N::A *ap; f (bp); // { dg-error "not declared" } + // { dg-message "N::f" "suggested alternative" { target *-*-* } 16 } f (ap); } diff --git a/gcc/testsuite/g++.dg/parse/decl-specifier-1.C b/gcc/testsuite/g++.dg/parse/decl-specifier-1.C index e81fbab..092af1e 100644 --- a/gcc/testsuite/g++.dg/parse/decl-specifier-1.C +++ b/gcc/testsuite/g++.dg/parse/decl-specifier-1.C @@ -13,4 +13,5 @@ N::X X; // { dg-error "" "" } int main() { return sizeof(X); // { dg-error "" "" } + // { dg-message "N::X" "suggested alternative" { target *-*-* } 15 } } diff --git a/gcc/testsuite/g++.dg/pr45330.C b/gcc/testsuite/g++.dg/pr45330.C new file mode 100644 index 0000000..a4b22f9 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr45330.C @@ -0,0 +1,19 @@ +// { dg-do compile } +// Search std, __cxxabiv1, and global namespaces, plus one more. +// { dg-options "--param cxx-max-namespaces-for-diagnostic-help=4" } + +#define NSPACE(NAME) namespace NAME { int foo; } + +NSPACE(A) +NSPACE(B) +NSPACE(C) +NSPACE(D) +NSPACE(E) + +int bar() +{ + return foo; // { dg-error "was not declared" } + // { dg-message "maximum limit of 4 namespaces searched" "" { target *-*-* } 15 } + // We don't know which namespace will come up first, so accept any. + // { dg-message "\[ABCDE\]::foo" "" { target *-*-* } 15 } +} diff --git a/gcc/testsuite/g++.dg/template/static10.C b/gcc/testsuite/g++.dg/template/static10.C index ab857bd..ce8ed9e 100644 --- a/gcc/testsuite/g++.dg/template/static10.C +++ b/gcc/testsuite/g++.dg/template/static10.C @@ -20,4 +20,5 @@ namespace std { template<> void vector >::swap(vector >&) { } // { dg-error "" } + // { dg-message "std::allocator" "suggested alternative" { target *-*-* } 22 } } diff --git a/gcc/testsuite/g++.old-deja/g++.mike/ns5.C b/gcc/testsuite/g++.old-deja/g++.mike/ns5.C index 9d806ca..f95821d 100644 --- a/gcc/testsuite/g++.old-deja/g++.mike/ns5.C +++ b/gcc/testsuite/g++.old-deja/g++.mike/ns5.C @@ -4,3 +4,4 @@ namespace A { } int j = i; // { dg-error "" } + // { dg-message "A::i" "suggested alternative" { target *-*-* } 6 } diff --git a/gcc/testsuite/g++.old-deja/g++.mike/ns7.C b/gcc/testsuite/g++.old-deja/g++.mike/ns7.C index 57008db..7fd2b6f 100644 --- a/gcc/testsuite/g++.old-deja/g++.mike/ns7.C +++ b/gcc/testsuite/g++.old-deja/g++.mike/ns7.C @@ -6,4 +6,5 @@ namespace A { namespace B { int j = i; // { dg-error "" } + // { dg-message "A::i" "suggested alternative" { target *-*-* } 8 } } diff --git a/gcc/testsuite/g++.old-deja/g++.ns/koenig5.C b/gcc/testsuite/g++.old-deja/g++.ns/koenig5.C index 33061ad..43bcd37 100644 --- a/gcc/testsuite/g++.old-deja/g++.ns/koenig5.C +++ b/gcc/testsuite/g++.old-deja/g++.ns/koenig5.C @@ -15,4 +15,5 @@ void g() // foo variable first, and therefore do not // perform argument-dependent lookup. bar(new X); // { dg-error "not declared" } + // { dg-message "A::bar" "suggested alternative" { target *-*-* } 17 } } diff --git a/gcc/testsuite/g++.old-deja/g++.ns/koenig9.C b/gcc/testsuite/g++.old-deja/g++.ns/koenig9.C index 78b0e8b..0515217 100644 --- a/gcc/testsuite/g++.old-deja/g++.ns/koenig9.C +++ b/gcc/testsuite/g++.old-deja/g++.ns/koenig9.C @@ -10,4 +10,5 @@ void foo(const char*,...); inline void bar() { foo("",count); // { dg-error "" } multiple overloaded count functions + // { dg-message "std::count" "suggested alternative" { target *-*-* } 12 } } diff --git a/gcc/testsuite/g++.old-deja/g++.other/lineno5.C b/gcc/testsuite/g++.old-deja/g++.other/lineno5.C index d14bd90..11c8d9f 100644 --- a/gcc/testsuite/g++.old-deja/g++.other/lineno5.C +++ b/gcc/testsuite/g++.old-deja/g++.other/lineno5.C @@ -16,4 +16,5 @@ namespace tmp { class A { public: int kaka(tmp::B = b); // { dg-error "" } no b in scope + // { dg-message "tmp::b" "suggested alternative" { target *-*-* } 18 } };