This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH,c++] fix PR 45330, suggest alternatives for failed name lookups
On Sun, Dec 05, 2010 at 10:09:09AM -0600, Mark Mitchell wrote:
> On 12/3/2010 10:41 AM, Nathan Froyd wrote:
> >> As we are gradually moving away from using the global input_location
> >> variable for better diagnostics maybe suggest_alternative could take a
> >> location parameter that ...
> >> ... would be used here instead of using the global input_location.
>
> > I can do this pretty easily, but I think it will require unstatic'ing
> > location_of from error.c as my patch for PR 45329 does.
>
> Please make these changes and commit; the patch is OK with those changes.
I tried making Dodji's changes (though I grab the location from the tree
passed to suggest_alternatives, rather than passing the location in from
above), which prompted me to make a couple more:
- We should really be using inform_n, rather than some hacky
format-already-translated-strings-into-messages scheme;
- We shouldn't tell the user that we hit the max namespace limit if
there aren't any more namespaces to search.
- When suggesting alternatives, we should indicate the location of the
alternative we're suggesting rather than the location of the name we
failed to lookup.
That is, for, say, g++.old-deja/g++.mike/ns5.C:
// { dg-do assemble }
namespace A {
int i = 1;
}
int j = i;
We should output:
ns5.C:6:9: error: 'i' was not declared in this scope
ns5.C:6:9: note: suggested alternative:
ns5.C:3:7: note: 'A::i'
instead of:
ns5.C:6:9: error: 'i' was not declared in this scope
ns5.C:6:9: note: suggested alternative:
ns5.C:6:9: note: 'A::i'
I think these three changes are non-controversial. Please let me know
if you feel differently.
However, this causes a problem for g++.old-deja/g++.other/koenig9.C,
since we now report a location in system headers and AFAICS, there's no
good way to handle that in DejaGNU. Looking a little further, it
appears that the test actually wanted:
koenig9.C:13: error: overloaded function with no contextual type information
rather than a complaint about an undeclared name. I guess at some point
in the past, <algorithm> automatically use'd std; now that it no longer
does this, we don't test the same thing we were testing before. I have
changed the test to use locally-declared count functions, which I think
keeps the spirit of the test better.
OK with the above changes?
-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, location_of): Declare.
* error.c (dump_expr): Handle TYPE_DECL.
(location_of): Unstaticize.
* 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 59342e3..aba8dfd 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4907,6 +4907,7 @@ extern void print_instantiation_context (void);
extern void maybe_warn_variadic_templates (void);
extern void maybe_warn_cpp0x (cpp0x_warn_str str);
extern bool pedwarn_cxx98 (location_t, int, const char *, ...) ATTRIBUTE_GCC_DIAG(3,4);
+extern location_t location_of (tree);
/* in except.c */
extern void init_exception_processing (void);
@@ -5639,6 +5640,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 ed168c4..4fb47dc 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -98,7 +98,6 @@ static void cp_print_error_function (diagnostic_context *, diagnostic_info *);
static bool cp_printer (pretty_printer *, text_info *, const char *,
int, bool, bool, bool);
-static location_t location_of (tree);
void
init_error (void)
@@ -1700,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;
@@ -2487,7 +2487,7 @@ lang_decl_name (tree decl, int v, bool translate)
/* Return the location of a tree passed to %+ formats. */
-static location_t
+location_t
location_of (tree t)
{
if (TREE_CODE (t) == PARM_DECL && DECL_CONTEXT (t))
diff --git a/gcc/cp/lex.c b/gcc/cp/lex.c
index 684803f..5a2ae41 100644
--- a/gcc/cp/lex.c
+++ b/gcc/cp/lex.c
@@ -450,7 +450,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..4cf1380 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,71 @@ 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;
+ tree t;
+ unsigned ix;
+ location_t name_location;
+
+ 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);
+ }
+
+ name_location = location_of (name);
+
+ /* 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
+ && !VEC_empty (tree, namespaces_to_search))
+ inform (name_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;
+
+ inform_n (name_location, VEC_length (tree, candidates),
+ "suggested alternative:",
+ "suggested alternatives:");
+
+ FOR_EACH_VEC_ELT (tree, candidates, ix, t)
+ inform (location_of (t), " %qE", t);
+
+ VEC_free (tree, heap, candidates);
+}
+
/* 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 2813532..4e3d002 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8821,6 +8821,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..001d5f7 100644
--- a/gcc/testsuite/g++.dg/ext/builtin3.C
+++ b/gcc/testsuite/g++.dg/ext/builtin3.C
@@ -5,9 +5,10 @@
// { dg-options "" }
namespace std {
-extern "C" int printf(char*, ...);
+extern "C" int printf(char*, ...); // { dg-message "std::printf" }
}
void foo() {
printf("abc"); // { dg-error "not declared" }
+ // { dg-message "suggested alternative" "suggested alternative" { target *-*-* } 12 }
}
diff --git a/gcc/testsuite/g++.dg/lookup/error1.C b/gcc/testsuite/g++.dg/lookup/error1.C
index 2264b23..3eb4b97 100644
--- a/gcc/testsuite/g++.dg/lookup/error1.C
+++ b/gcc/testsuite/g++.dg/lookup/error1.C
@@ -2,8 +2,9 @@
// Origin: <papadopo@shfj.cea.fr>
// { dg-do compile }
-namespace N { int i; }
+namespace N { int i; } // { dg-message "N::i" }
void foo() { i; } // { dg-error "not declared" }
+ // { dg-message "suggested alternative" "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..c44543b 100644
--- a/gcc/testsuite/g++.dg/lookup/koenig5.C
+++ b/gcc/testsuite/g++.dg/lookup/koenig5.C
@@ -8,23 +8,23 @@
namespace N
{
struct A {};
- void One (...);
- void (*Two) (...);
- namespace Three {}
+ void One (...); // { dg-message "N::One" }
+ void (*Two) (...); // { dg-message "N::Two" }
+ namespace Three {} // { dg-message "N::Three" }
}
namespace M
{
struct B {};
- struct One {};
- void (*Two) (...);
- void Three (...);
+ struct One {}; // { dg-message "M::One" }
+ void (*Two) (...); // { dg-message "M::Two" }
+ void Three (...); // { dg-message "M::Three" }
}
namespace O
{
struct C {};
- void Two (...);
+ void Two (...); // { dg-message "O::Two" }
}
void g (N::A *a, M::B *b, O::C *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 "suggested alternatives" "suggested alternative for One" { target *-*-* } 34 }
Two (c); // ok
Two (a, c); // ok
Two (a); // { dg-error "not declared" }
+ // { dg-message "suggested alternatives" "suggested alternative for Two" { 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 "suggested alternatives" "suggested alternative for Three" { target *-*-* } 47 }
}
diff --git a/gcc/testsuite/g++.dg/overload/koenig1.C b/gcc/testsuite/g++.dg/overload/koenig1.C
index 1ed7bce..a461259 100644
--- a/gcc/testsuite/g++.dg/overload/koenig1.C
+++ b/gcc/testsuite/g++.dg/overload/koenig1.C
@@ -3,7 +3,7 @@
// valid call.
namespace N {
- template <class T> void f (T);
+ template <class T> void f (T); // { dg-message "N::f" }
struct A;
}
@@ -14,5 +14,6 @@ void g ()
B *bp;
N::A *ap;
f (bp); // { dg-error "not declared" }
+ // { dg-message "suggested alternative" "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..baf0fe7 100644
--- a/gcc/testsuite/g++.dg/parse/decl-specifier-1.C
+++ b/gcc/testsuite/g++.dg/parse/decl-specifier-1.C
@@ -5,7 +5,7 @@
namespace N
{
template<typename>
- struct X { };
+ struct X { }; // { dg-message "N::X" }
}
N::X X; // { dg-error "" "" }
@@ -13,4 +13,5 @@ N::X X; // { dg-error "" "" }
int main()
{
return sizeof(X); // { dg-error "" "" }
+ // { dg-message "suggested alternative" "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..02d9b3f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr45330.C
@@ -0,0 +1,37 @@
+// { 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; }
+
+namespace A
+{
+ int foo; // { dg-message "A::foo" "suggested alternative" }
+}
+
+namespace B
+{
+ int foo;
+}
+
+namespace C
+{
+ int foo;
+}
+
+namespace D
+{
+ int foo;
+}
+
+namespace E
+{
+ int foo;
+}
+
+int bar()
+{
+ return foo; // { dg-error "was not declared" }
+ // { dg-message "maximum limit of 4 namespaces" "maximum limit" { target *-*-* } 34 }
+ // { dg-message "suggested alternative" "suggested alternative" { target *-*-* } 34 }
+}
diff --git a/gcc/testsuite/g++.dg/template/static10.C b/gcc/testsuite/g++.dg/template/static10.C
index ab857bd..881db08 100644
--- a/gcc/testsuite/g++.dg/template/static10.C
+++ b/gcc/testsuite/g++.dg/template/static10.C
@@ -4,7 +4,7 @@ namespace __gnu_debug_def { }
namespace std
{
using namespace __gnu_debug_def;
- template<typename _Tp> class allocator {};
+ template<typename _Tp> class allocator {}; // { dg-message "std::allocator" }
}
namespace __gnu_debug_def
{
@@ -20,4 +20,5 @@ namespace std
{
template<> void
vector<int, allocator<int> >::swap(vector<int, allocator<int> >&) { } // { dg-error "" }
+ // { dg-message "suggested alternative" "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..fd1fbff 100644
--- a/gcc/testsuite/g++.old-deja/g++.mike/ns5.C
+++ b/gcc/testsuite/g++.old-deja/g++.mike/ns5.C
@@ -1,6 +1,7 @@
// { dg-do assemble }
namespace A {
- int i = 1;
+ int i = 1; // { dg-message "A::i" }
}
int j = i; // { dg-error "" }
+ // { dg-message "suggested alternative" "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..67d9e77 100644
--- a/gcc/testsuite/g++.old-deja/g++.mike/ns7.C
+++ b/gcc/testsuite/g++.old-deja/g++.mike/ns7.C
@@ -1,9 +1,10 @@
// { dg-do assemble }
namespace A {
- int i = 1;
+ int i = 1; // { dg-message "A::i" }
}
namespace B {
int j = i; // { dg-error "" }
+ // { dg-message "suggested alternative" "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..7c56d5c 100644
--- a/gcc/testsuite/g++.old-deja/g++.ns/koenig5.C
+++ b/gcc/testsuite/g++.old-deja/g++.ns/koenig5.C
@@ -3,7 +3,7 @@
namespace A{
void foo();
struct X{};
- void (*bar)(X*)=0;
+ void (*bar)(X*)=0; // { dg-message "A::bar" }
}
using A::X;
@@ -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 "suggested alternative" "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..46efcb7 100644
--- a/gcc/testsuite/g++.old-deja/g++.ns/koenig9.C
+++ b/gcc/testsuite/g++.old-deja/g++.ns/koenig9.C
@@ -3,11 +3,12 @@
// Copyright (C) 2000 Free Software Foundation, Inc.
// Contributed by Theodore.Papadopoulo 23 Jun 2000 <Theodore.Papadopoulo@sophia.inria.fr>
-#include <algorithm>
+int count (int);
+void *count (char *, char);
void foo(const char*,...);
inline void
bar() {
- foo("",count); // { dg-error "" } multiple overloaded count functions
+ foo("",count); // { dg-error "overloaded function" "multiple overloaded functions" }
}
diff --git a/gcc/testsuite/g++.old-deja/g++.other/lineno5.C b/gcc/testsuite/g++.old-deja/g++.other/lineno5.C
index d14bd90..d227339 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/lineno5.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/lineno5.C
@@ -10,10 +10,11 @@
namespace tmp {
typedef int B;
- B b;
+ B b; // { dg-message "tmp::b" }
}
class A {
public:
int kaka(tmp::B = b); // { dg-error "" } no b in scope
+ // { dg-message "suggested alternative" "suggested alternative" { target *-*-* } 18 }
};