[C++,Patch/RFC] PR 60385 and other issues about wrongly named namespaces (eg, c++/68723)

Hi,

we have these long standing issues with code like (c++/60385):

float foo4();

namespace foo4
{
    // ....
}

where the name of the namespace conflicts with an existing declaration. 
Error recovery is currently suboptimal, for example, c++/60385 is about

     struct bar6
     {
     friend wchar_t bar1();
     };

inside the namespace: due to the code in do_friend:

           push_nested_namespace (ns);
           decl = pushdecl_namespace_level (decl, /*is_friend=*/true);
           pop_nested_namespace (ns);

we issue duplicate diagnostic about the wrong name and later we crash in 
pop_namespace (the second time in push_namespace, 
IDENTIFIER_NAMESPACE_VALUE isn't found set for the malformed namespace, 
thus need_new is true, pushdecl is called...)

Now, I'm wondering how far we want to go with error recovery for such 
snippets. Certainly, in analogy with the code at the beginning of 
cp_parser_class_specifier_1, we can completely skip the body of such 
malformed namespaces. That would be the first attached patchlet. Or we 
can go on in cp_parser_namespace_definition but remember that 
push_namespace didn't really succeed and keep things consistent, thus 
avoid crashing in pop_namespace later, as currently happens. That would 
be second patchlet. Both ideas pass testing and work for c++/68723 too 
(as expected, the first patchlet leads to particularly neat diagnostic 
for the very broken snippet in c++/68723, only the error about the wrong 
namespace name, as for c++/60385).

Thanks!
Paolo.

//////////////////////

Index: name-lookup.c
===================================================================
--- name-lookup.c	(revision 236799)
+++ name-lookup.c	(working copy)
@@ -3685,7 +3685,7 @@ handle_namespace_attrs (tree ns, tree attributes)
 /* Push into the scope of the NAME namespace.  If NAME is NULL_TREE, then we
    select a name that is unique to this compilation unit.  */
 
-void
+bool
 push_namespace (tree name)
 {
   tree d = NULL_TREE;
@@ -3759,7 +3759,11 @@ push_namespace (tree name)
 	TREE_PUBLIC (d) = 0;
       else
 	TREE_PUBLIC (d) = 1;
-      pushdecl (d);
+      if (pushdecl (d) == error_mark_node)
+	{
+	  timevar_cond_stop (TV_NAME_LOOKUP, subtime);
+	  return false;
+	}
       if (anon)
 	{
 	  /* Clear DECL_NAME for the benefit of debugging back ends.  */
@@ -3777,6 +3781,7 @@ push_namespace (tree name)
   current_namespace = d;
 
   timevar_cond_stop (TV_NAME_LOOKUP, subtime);
+  return true;
 }
 
 /* Pop from the scope of the current namespace.  */
Index: name-lookup.h
===================================================================
--- name-lookup.h	(revision 236799)
+++ name-lookup.h	(working copy)
@@ -312,7 +312,7 @@ extern tree push_inner_scope (tree);
 extern void pop_inner_scope (tree, tree);
 extern void push_binding_level (cp_binding_level *);
 
-extern void push_namespace (tree);
+extern bool push_namespace (tree);
 extern void pop_namespace (void);
 extern void push_nested_namespace (tree);
 extern void pop_nested_namespace (tree);
Index: parser.c
===================================================================
--- parser.c	(revision 236799)
+++ parser.c	(working copy)
@@ -17549,7 +17549,7 @@ cp_parser_namespace_definition (cp_parser* parser)
     }
 
   /* Start the namespace.  */
-  push_namespace (identifier);
+  bool ok = push_namespace (identifier);
 
   /* Parse any nested namespace definition. */
   if (cp_lexer_next_token_is (parser->lexer, CPP_SCOPE))
@@ -17582,7 +17582,7 @@ cp_parser_namespace_definition (cp_parser* parser)
 
   /* "inline namespace" is equivalent to a stub namespace definition
      followed by a strong using directive.  */
-  if (is_inline)
+  if (is_inline && ok)
     {
       tree name_space = current_namespace;
       /* Set up namespace association.  */
@@ -17610,7 +17610,8 @@ cp_parser_namespace_definition (cp_parser* parser)
     pop_namespace ();
 
   /* Finish the namespace.  */
-  pop_namespace ();
+  if (ok)
+    pop_namespace ();
   /* Look for the final `}'.  */
   cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE);
 }

Let's go with the second patch.

Jason