C++ PATCH: PR 20293

[Mark Mitchell <mark at codesourcery dot com>]

This patch fixes PR c++/20293, wherein the diagnostic for an ambiguous
namespace reference indicated, confusingly, that there was no
declaration, rather than two.  The key idea in the patch is that the
name lookup routines should not be reporting errors about ambiguity;
that job lies with the parser, and, perhaps, in rare circumstances,
the semantic analysis routines.  The name lookup routines should just
provide the list of ambiguous candidates so that the caller can
provide an appropriate diagnostic.

Tested on x86_64-unknown-linux-gnu, applied on the mainline.  This
diagnostic problem also occurs on the 3.4 and 4.0 branches, but I
don't think the problem is sufficiently severe to take the risk or
spend the effort to backport the patch; we still issue a diagnostic,
and there's enough information in the diagnostic that people affected
will be able to work out the problem.

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

2005-11-13  Mark Mitchell  <mark@codesourcery.com>

	PR c++/20293
	* cxx-pretty-print.c (pp_cxx_statement): Print qualifying scopes
	for namespaces.
	(pp_cxx_original_namespace_definition): Likewise.
	* name-lookup.c (ambiguous_decl): Don't issue error messages;
	instead return lists of ambiguous candidates.
	(select_decl): Handle ambiguous namespace lookups.
	* parser.c (cp_token): Add ambiguous_p.
	(cp_lexer_get_preprocessor_token): Set it.
	(cp_parser_diagnose_invalid_type_name): Avoid duplicate messages
	when a qualified name uses an invalid scope. 
	(cp_parser_primary_expression): Print ambiguous candidates.
	(cp_parser_type_parameter): Adjust comment to reflect new
	parameter name for cp_parser_lookup_name.
	(cp_parser_template_argument): Likewise.
	(cp_parser_elaborated_type_specifier): Likewise.
	(cp_parser_namespace_name): Likewise.
	(cp_parser_class_name): Print ambiguous candidates.
	(cp_parser_lookup_name): Rename ambiguous_p parameter to
	ambiguous_decls.  Use it to return a list of ambiguous candiates
	when a lookup is ambiguous.
	(cp_parser_lookup_name_simple): Adjust comment to reflect new
	parameter name for cp_parser_lookup_name.
	
2005-11-13  Mark Mitchell  <mark@codesourcery.com>

	PR c++/20293
	* g++.dg/parse/ambig4.C: New test.
	* g++.dg/tc1/dr101.C: Adjust error markers.
	* g++.dg/lookup/strong-using-2.C: Likewise.
	* g++.dg/lookup/ambig5.C: Likewise.
	* g++.dg/lookup/ambig4.C: Likewise.
	* g++.dg/parse/crash22.C: Likewise.

Index: gcc/cp/cxx-pretty-print.c
===================================================================
--- gcc/cp/cxx-pretty-print.c	(revision 106878)
+++ gcc/cp/cxx-pretty-print.c	(working copy)
@@ -1514,6 +1514,8 @@ pp_cxx_statement (cxx_pretty_printer *pp
     case USING_STMT:
       pp_cxx_identifier (pp, "using");
       pp_cxx_identifier (pp, "namespace");
+      if (DECL_CONTEXT (t))
+	pp_cxx_nested_name_specifier (pp, DECL_CONTEXT (t));
       pp_cxx_qualified_id (pp, USING_STMT_NAMESPACE (t));
       break;
 
@@ -1701,6 +1703,8 @@ static void
 pp_cxx_original_namespace_definition (cxx_pretty_printer *pp, tree t)
 {
   pp_cxx_identifier (pp, "namespace");
+  if (DECL_CONTEXT (t))
+    pp_cxx_nested_name_specifier (pp, DECL_CONTEXT (t));
   if (DECL_NAME (t))
     pp_cxx_unqualified_id (pp, t);
   pp_cxx_whitespace (pp);
@@ -1723,10 +1727,15 @@ static void
 pp_cxx_namespace_alias_definition (cxx_pretty_printer *pp, tree t)
 {
   pp_cxx_identifier (pp, "namespace");
+  if (DECL_CONTEXT (t))
+    pp_cxx_nested_name_specifier (pp, DECL_CONTEXT (t));
   pp_cxx_unqualified_id (pp, t);
   pp_cxx_whitespace (pp);
   pp_equal (pp);
   pp_cxx_whitespace (pp);
+  if (DECL_CONTEXT (DECL_NAMESPACE_ALIAS (t)))
+    pp_cxx_nested_name_specifier (pp, 
+				  DECL_CONTEXT (DECL_NAMESPACE_ALIAS (t)));
   pp_cxx_qualified_id (pp, DECL_NAMESPACE_ALIAS (t));
   pp_cxx_semicolon (pp);
 }
Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 106878)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -3400,20 +3400,9 @@ ambiguous_decl (tree name, struct scope_
 	old->value = merge_functions (old->value, val);
       else
 	{
-	  /* Some declarations are functions, some are not.  */
-	  if (flags & LOOKUP_COMPLAIN)
-	    {
-	      /* If we've already given this error for this lookup,
-		 old->value is error_mark_node, so let's not
-		 repeat ourselves.  */
-	      if (old->value != error_mark_node)
-		{
-		  error ("use of %qD is ambiguous", name);
-		  error ("  first declared as %q+#D here", old->value);
-		}
-	      error ("  also declared as %q+#D here", val);
-	    }
-	  old->value = error_mark_node;
+	  old->value = tree_cons (NULL_TREE, old->value,
+				  build_tree_list (NULL_TREE, new->value));
+	  TREE_TYPE (old->value) = error_mark_node;
 	}
     }
   /* ... and copy the type.  */
@@ -3609,7 +3598,8 @@ select_decl (const struct scope_binding 
   if (LOOKUP_NAMESPACES_ONLY (flags))
     {
       /* We are not interested in types.  */
-      if (val && TREE_CODE (val) == NAMESPACE_DECL)
+      if (val && (TREE_CODE (val) == NAMESPACE_DECL
+		  || TREE_CODE (val) == TREE_LIST))
 	POP_TIMEVAR_AND_RETURN (TV_NAME_LOOKUP, val);
       POP_TIMEVAR_AND_RETURN (TV_NAME_LOOKUP, NULL_TREE);
     }
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 106878)
+++ gcc/cp/parser.c	(working copy)
@@ -59,6 +59,10 @@ typedef struct cp_token GTY (())
   BOOL_BITFIELD in_system_header : 1;
   /* True if this token is from a context where it is implicitly extern "C" */
   BOOL_BITFIELD implicit_extern_c : 1;
+  /* True for a CPP_NAME token that is not a keyword (i.e., for which
+     KEYWORD is RID_MAX) iff this name was looked up and found to be
+     ambiguous.  An error has already been reported.  */
+  BOOL_BITFIELD ambiguous_p : 1;
   /* The value associated with this token, if any.  */
   tree value;
   /* The location at which this token was found.  */
@@ -401,18 +405,22 @@ cp_lexer_get_preprocessor_token (cp_lexe
   token->implicit_extern_c = is_extern_c > 0;
 
   /* Check to see if this token is a keyword.  */
-  if (token->type == CPP_NAME
-      && C_IS_RESERVED_WORD (token->value))
+  if (token->type == CPP_NAME)
     {
-      /* Mark this token as a keyword.  */
-      token->type = CPP_KEYWORD;
-      /* Record which keyword.  */
-      token->keyword = C_RID_CODE (token->value);
-      /* Update the value.  Some keywords are mapped to particular
-	 entities, rather than simply having the value of the
-	 corresponding IDENTIFIER_NODE.  For example, `__const' is
-	 mapped to `const'.  */
-      token->value = ridpointers[token->keyword];
+      if (C_IS_RESERVED_WORD (token->value))
+	{
+	  /* Mark this token as a keyword.  */
+	  token->type = CPP_KEYWORD;
+	  /* Record which keyword.  */
+	  token->keyword = C_RID_CODE (token->value);
+	  /* Update the value.  Some keywords are mapped to particular
+	     entities, rather than simply having the value of the
+	     corresponding IDENTIFIER_NODE.  For example, `__const' is
+	     mapped to `const'.  */
+	  token->value = ridpointers[token->keyword];
+	}
+      else
+	token->ambiguous_p = false;
     }
   /* Handle Objective-C++ keywords.  */
   else if (token->type == CPP_AT_NAME)
@@ -1699,7 +1707,7 @@ static tree cp_parser_objc_statement
 /* Utility Routines */
 
 static tree cp_parser_lookup_name
-  (cp_parser *, tree, enum tag_types, bool, bool, bool, bool *);
+  (cp_parser *, tree, enum tag_types, bool, bool, bool, tree *);
 static tree cp_parser_lookup_name_simple
   (cp_parser *, tree);
 static tree cp_parser_maybe_treat_template_as_class
@@ -2052,7 +2060,7 @@ cp_parser_diagnose_invalid_type_name (cp
   if (TREE_CODE (decl) == TEMPLATE_DECL)
     error ("invalid use of template-name %qE without an argument list",
       decl);
-  else if (!parser->scope || parser->scope == error_mark_node)
+  else if (!parser->scope)
     {
       /* Issue an error message.  */
       error ("%qE does not name a type", id);
@@ -2099,7 +2107,7 @@ cp_parser_diagnose_invalid_type_name (cp
     }
   /* Here we diagnose qualified-ids where the scope is actually correct,
      but the identifier does not resolve to a valid type name.  */
-  else
+  else if (parser->scope != error_mark_node)
     {
       if (TREE_CODE (parser->scope) == NAMESPACE_DECL)
 	error ("%qE in namespace %qE does not name a type",
@@ -2994,17 +3002,17 @@ cp_parser_primary_expression (cp_parser 
 	/* Look up the name.  */
 	else
 	  {
-	    bool ambiguous_p;
+	    tree ambiguous_decls;
 
 	    decl = cp_parser_lookup_name (parser, id_expression,
 					  none_type,
 					  template_p,
 					  /*is_namespace=*/false,
 					  /*check_dependency=*/true,
-					  &ambiguous_p);
+					  &ambiguous_decls);
 	    /* If the lookup was ambiguous, an error will already have
 	       been issued.  */
-	    if (ambiguous_p)
+	    if (ambiguous_decls)
 	      return error_mark_node;
 
 	    /* In Objective-C++, an instance variable (ivar) may be preferred
@@ -3610,16 +3618,32 @@ cp_parser_nested_name_specifier_opt (cp_
 	      token = cp_lexer_consume_token (parser->lexer);
 	      if (!error_p)
 		{
-		  tree decl;
+		  if (!token->ambiguous_p)
+		    {
+		      tree decl;
+		      tree ambiguous_decls;
 
-		  decl = cp_parser_lookup_name_simple (parser, token->value);
-		  if (TREE_CODE (decl) == TEMPLATE_DECL)
-		    error ("%qD used without template parameters", decl);
-		  else
-		    cp_parser_name_lookup_error
-		      (parser, token->value, decl,
-		       "is not a class or namespace");
-		  parser->scope = NULL_TREE;
+		      decl = cp_parser_lookup_name (parser, token->value,
+						    none_type,
+						    /*is_template=*/false,
+						    /*is_namespace=*/false,
+						    /*check_dependency=*/true,
+						    &ambiguous_decls);
+		      if (TREE_CODE (decl) == TEMPLATE_DECL)
+			error ("%qD used without template parameters", decl);
+		      else if (ambiguous_decls)
+			{
+			  error ("reference to %qD is ambiguous", 
+				 token->value);
+			  print_candidates (ambiguous_decls);
+			  decl = error_mark_node;
+			}
+		      else
+			cp_parser_name_lookup_error
+			  (parser, token->value, decl,
+			   "is not a class or namespace");
+		    }
+		  parser->scope = error_mark_node;
 		  error_p = true;
 		  /* Treat this as a successful nested-name-specifier
 		     due to:
@@ -8457,7 +8481,7 @@ cp_parser_type_parameter (cp_parser* par
 					 /*is_template=*/is_template,
 					 /*is_namespace=*/false,
 					 /*check_dependency=*/true,
-					 /*ambiguous_p=*/NULL);
+					 /*ambiguous_decls=*/NULL);
 	    /* See if the default argument is valid.  */
 	    default_argument
 	      = check_template_template_default_arg (default_argument);
@@ -8813,7 +8837,7 @@ cp_parser_template_name (cp_parser* pars
 				/*is_template=*/false,
 				/*is_namespace=*/false,
 				check_dependency_p,
-				/*ambiguous_p=*/NULL);
+				/*ambiguous_decls=*/NULL);
   decl = maybe_get_template_decl_from_type_decl (decl);
 
   /* If DECL is a template, then the name was a template-name.  */
@@ -9014,7 +9038,7 @@ cp_parser_template_argument (cp_parser* 
 					  /*is_template=*/template_p,
 					  /*is_namespace=*/false,
 					  /*check_dependency=*/true,
-					  /*ambiguous_p=*/NULL);
+					  /*ambiguous_decls=*/NULL);
       if (TREE_CODE (argument) != TEMPLATE_DECL
 	  && TREE_CODE (argument) != UNBOUND_CLASS_TEMPLATE)
 	cp_parser_error (parser, "expected template-name");
@@ -9937,7 +9961,7 @@ cp_parser_elaborated_type_specifier (cp_
 					/*is_template=*/false,
 					/*is_namespace=*/false,
 					/*check_dependency=*/true,
-					/*ambiguous_p=*/NULL);
+					/*ambiguous_decls=*/NULL);
 
 	  /* If we are parsing friend declaration, DECL may be a
 	     TEMPLATE_DECL tree node here.  However, we need to check
@@ -10233,7 +10257,7 @@ cp_parser_namespace_name (cp_parser* par
 					  /*is_template=*/false,
 					  /*is_namespace=*/true,
 					  /*check_dependency=*/true,
-					  /*ambiguous_p=*/NULL);
+					  /*ambiguous_decls=*/NULL);
   /* If it's not a namespace, issue an error.  */
   if (namespace_decl == error_mark_node
       || TREE_CODE (namespace_decl) != NAMESPACE_DECL)
@@ -12469,9 +12493,13 @@ cp_parser_class_name (cp_parser *parser,
   if (token->type == CPP_NAME
       && !cp_parser_nth_token_starts_template_argument_list_p (parser, 2))
     {
+      cp_token *identifier_token;
       tree identifier;
+      bool ambiguous_p;
 
       /* Look for the identifier.  */
+      identifier_token = cp_lexer_peek_token (parser->lexer);
+      ambiguous_p = identifier_token->ambiguous_p;
       identifier = cp_parser_identifier (parser);
       /* If the next token isn't an identifier, we are certainly not
 	 looking at a class-name.  */
@@ -12483,6 +12511,15 @@ cp_parser_class_name (cp_parser *parser,
 	decl = identifier;
       else
 	{
+	  tree ambiguous_decls;
+	  /* If we already know that this lookup is ambiguous, then
+	     we've already issued an error message; there's no reason
+	     to check again.  */
+	  if (ambiguous_p)
+	    {
+	      cp_parser_simulate_error (parser);
+	      return error_mark_node;
+	    }
 	  /* If the next token is a `::', then the name must be a type
 	     name.
 
@@ -12499,7 +12536,18 @@ cp_parser_class_name (cp_parser *parser,
 					/*is_template=*/false,
 					/*is_namespace=*/false,
 					check_dependency_p,
-					/*ambiguous_p=*/NULL);
+					&ambiguous_decls);
+	  if (ambiguous_decls)
+	    {
+	      error ("reference to %qD is ambiguous", identifier);
+	      print_candidates (ambiguous_decls);
+	      if (cp_parser_parsing_tentatively (parser))
+		{
+		  identifier_token->ambiguous_p = true;
+		  cp_parser_simulate_error (parser);
+		}
+	      return error_mark_node;
+	    }
 	}
     }
   else
@@ -14438,8 +14486,9 @@ cp_parser_label_declaration (cp_parser* 
    If CHECK_DEPENDENCY is TRUE, names are not looked up in dependent
    types.
 
-   If AMBIGUOUS_P is non-NULL, it is set to true if name-lookup
-   results in an ambiguity, and false otherwise.  */
+   If AMBIGUOUS_DECLS is non-NULL, *AMBIGUOUS_DECLS is set to a
+   TREE_LIST of candiates if name-lookup results in an ambiguity, and
+   NULL_TREE otherwise.  */ 
 
 static tree
 cp_parser_lookup_name (cp_parser *parser, tree name,
@@ -14447,7 +14496,7 @@ cp_parser_lookup_name (cp_parser *parser
 		       bool is_template, 
 		       bool is_namespace,
 		       bool check_dependency,
-		       bool *ambiguous_p)
+		       tree *ambiguous_decls)
 {
   int flags = 0;
   tree decl;
@@ -14457,8 +14506,8 @@ cp_parser_lookup_name (cp_parser *parser
     flags |= LOOKUP_COMPLAIN;
 
   /* Assume that the lookup will be unambiguous.  */
-  if (ambiguous_p)
-    *ambiguous_p = false;
+  if (ambiguous_decls)
+    *ambiguous_decls = NULL_TREE;
 
   /* Now that we have looked up the name, the OBJECT_TYPE (if any) is
      no longer valid.  Note that if we are parsing tentatively, and
@@ -14615,8 +14664,8 @@ cp_parser_lookup_name (cp_parser *parser
   /* If it's a TREE_LIST, the result of the lookup was ambiguous.  */
   if (TREE_CODE (decl) == TREE_LIST)
     {
-      if (ambiguous_p)
-	*ambiguous_p = true;
+      if (ambiguous_decls)
+	*ambiguous_decls = decl;
       /* The error message we have to print is too complicated for
 	 cp_parser_error, so we incorporate its actions directly.  */
       if (!cp_parser_simulate_error (parser))
@@ -14658,7 +14707,7 @@ cp_parser_lookup_name_simple (cp_parser*
 				/*is_template=*/false,
 				/*is_namespace=*/false,
 				/*check_dependency=*/true,
-				/*ambiguous_p=*/NULL);
+				/*ambiguous_decls=*/NULL);
 }
 
 /* If DECL is a TEMPLATE_DECL that can be treated like a TYPE_DECL in
Index: gcc/testsuite/g++.dg/tc1/dr101.C
===================================================================
--- gcc/testsuite/g++.dg/tc1/dr101.C	(revision 106878)
+++ gcc/testsuite/g++.dg/tc1/dr101.C	(working copy)
@@ -17,10 +17,10 @@ namespace Test1 {
 
 namespace Test2 {
 
-  typedef unsigned int X;   // { dg-bogus "declared" "" { xfail *-*-* } }
+  typedef unsigned int X;   // { dg-bogus "X" "" { xfail *-*-* } }
   extern "C" int f2();
   namespace N {
-    typedef unsigned int X; // { dg-bogus "declared" "" { xfail *-*-* } }
+    typedef unsigned int X; // { dg-bogus "X" "" { xfail *-*-* } }
     extern "C" int f2();
   }
   using namespace N;
Index: gcc/testsuite/g++.dg/lookup/strong-using-2.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/strong-using-2.C	(revision 106878)
+++ gcc/testsuite/g++.dg/lookup/strong-using-2.C	(working copy)
@@ -3,10 +3,10 @@
 // { dg-do compile }
 
 namespace foo_impl {
-  class T; // { dg-error "first declared" "" }
+  class T; // { dg-error "T" "" }
 }
 namespace bar_impl {
-  class T; // { dg-error "also declared" "" }
+  class T; // { dg-error "T" "" }
 }
 namespace foo {
   using namespace foo_impl __attribute__((strong));
Index: gcc/testsuite/g++.dg/lookup/ambig5.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/ambig5.C	(revision 106878)
+++ gcc/testsuite/g++.dg/lookup/ambig5.C	(working copy)
@@ -4,10 +4,10 @@
 
 namespace N
 {
-  namespace M {}    // { dg-error "declared" }
+  namespace M {}    // { dg-error "M" }
 }
 
-namespace M {}      // { dg-error "declared" }
+namespace M {}      // { dg-error "M" }
 
 using namespace N;
 using namespace M;  // { dg-error "namespace-name|ambiguous" }
Index: gcc/testsuite/g++.dg/lookup/ambig4.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/ambig4.C	(revision 106878)
+++ gcc/testsuite/g++.dg/lookup/ambig4.C	(working copy)
@@ -4,11 +4,11 @@
 
 namespace N
 {
-  int i;            // { dg-error "declared" }
+  int i;            // { dg-error "i" }
 }
 
-int i;              // { dg-error "declared" }
+int i;              // { dg-error "i" }
 
 using namespace N;
 
-void foo() { i; }   // { dg-error "in this scope|ambiguous" }
+void foo() { i; }   // { dg-error "ambiguous" }
Index: gcc/testsuite/g++.dg/parse/crash22.C
===================================================================
--- gcc/testsuite/g++.dg/parse/crash22.C	(revision 106878)
+++ gcc/testsuite/g++.dg/parse/crash22.C	(working copy)
@@ -4,17 +4,17 @@
 // PR 19030: ICE
 // Origin: Volker Reichelt <reichelt@gcc.gnu.org>
 
-struct A;
+struct A; // { dg-error "A" }
 
 namespace N
 {
-  struct A;
+  struct A; // { dg-error "A" }
 }
 
 using namespace N;
 
-int A::i; // { dg-error "not been declared|declared here" "" }
-int A::i; // { dg-error "not been declared|redefinition of" "" }
+int A::i; // { dg-error "ambiguous|declared here" "" }
+int A::i; // { dg-error "ambiguous|redefinition of" "" }
 
 namespace N
 {
Index: gcc/testsuite/g++.dg/parse/ambig4.C
===================================================================
--- gcc/testsuite/g++.dg/parse/ambig4.C	(revision 0)
+++ gcc/testsuite/g++.dg/parse/ambig4.C	(revision 0)
@@ -0,0 +1,18 @@
+// PR c++/20293
+
+namespace hide { // { dg-error "hide" }
+  int k;
+}
+
+namespace {
+  int i; 
+  namespace hide { // { dg-error "hide" }
+    int j; 
+  }
+}
+
+void F(int) {}
+
+int main() {
+  F(hide::j); // { dg-error "ambiguous" }
+}