This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 }
 };


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]