[pph] Enable nested namespaces

[Lawrence Crowl, 2011-04-27 17:57:16]

Add a flag -fpph-dump-tree to dump the namespace trees read and written by PPH.
This dump could use some clarification, but it can wait for another day.

Replace PPH calls to pushdecl_with_scope with pushdecl_into_namespace.  It
turns out that pushdecl_with_scope always inserts into the current namespace,
not the given namespace.  Enable recursive call for nested namespace.

The pushdecl_into_namespace function works by saving and restoring
the current namespace around calls to pushdecl_with_scope, There may
be a better way, such as the disabled call to pushdecl_maybe_friend,
available at the time of cleanup.  There are subsequent segfaults still.

Fix formatting of a conditional in pph-streamer.c.

In libcpp/directives.c, make the too_many_directives_for_bitfield
static assert into an extern to avoid allocating storage.

Index: gcc/c-family/ChangeLog.pph

2011-04-20  Lawrence Crowl  <crowl@google.com>

	* c.opt (fpph-dump-tree): Add.

Index: gcc/cp/ChangeLog.pph

2011-04-26  Lawrence Crowl <crowl@google.com>

	* cp-tree.h (pushdecl_into_namespace): Add.
	* name-lookup.c (pushdecl_into_namespace): Add.
	* pph.c: Add include of tree-dump.h.
	(pph_dump_tree_name): Add.
	(pph_dump_namespace): Add.
	(pph_write_file_contents): Call pph_dump_namespace when appropriate.
	(pph_read_file_contents): Call pph_dump_namespace when appropriate.
	(pph_add_names_to_namespace): Use pushdecl_into_namespace rather than
	pushdecl_with_scope.  Make associated changes.  Enable recursive call
	for nested namespace.
	* pph-streamer.c (pph_stream_trace): Reformat conditionals.

Index: libcpp/ChangeLog.pph

2011-04-17  Lawrence Crowl  <crowl@google.com>

	* directives.c (too_many_directives_for_bitfield): Make static assert
	extern to avoid allocating any storage.


Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 172998)
+++ gcc/c-family/c.opt	(working copy)
@@ -937,6 +937,10 @@ fpph-decls-debug=
 C++ Joined RejectNegative UInteger Var(flag_pph_decls_debug)
 -fpph-decls=N   Enable declaration identifier output at level N from PPH
support
 
+fpph-dump-tree
+C++ Var(flag_pph_dump_tree)
+-fpph-dump-tree	Dump global namespace tree around PPH reads/writes.
+
 fpph-hdr=
 C++ ObjC++ Joined MissingArgError(missing filename after %qs)
 -fpph-hdr=<base-name>   A mapping from <base-name>.h to <base-name>.pph
Index: gcc/cp/pph.c
===================================================================
--- gcc/cp/pph.c	(revision 172998)
+++ gcc/cp/pph.c	(working copy)
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  
 #include "fixed-value.h"
 #include "md5.h"
 #include "tree-pass.h"
+#include "tree-dump.h"
 #include "tree-inline.h"
 #include "tree-pretty-print.h"
 #include "parser.h"
@@ -1863,12 +1864,65 @@ pth_file_change (cpp_reader *reader, con
 }
 
 
+/* Dump a complicated name for tree T to FILE using FLAGS.
+   See TDF_* in tree-pass.h for flags.  */
+
+static void
+pph_dump_tree_name (FILE *file, tree t, int flags)
+{
+  enum tree_code code = TREE_CODE (t);
+  fprintf (file, "%s\t", tree_code_name[code]);
+  if (code == FUNCTION_TYPE || code == METHOD_TYPE)
+    {
+      dump_function_to_file (t, file, flags);
+    }
+  else
+    {
+      print_generic_expr (file, TREE_TYPE (t), flags);
+      /* FIXME pph: fprintf (file, " ", cxx_printable_name (t, 0)); */
+      fprintf (file, " " );
+      print_generic_expr (file, t, flags);
+    }
+  fprintf (file, "\n");
+}
+
+
+/* Dump namespace NS for PPH.  */
+
+static void
+pph_dump_namespace (FILE *file, tree ns)
+{
+  struct cp_binding_level *level;
+  tree t, chain;
+  level = NAMESPACE_LEVEL (ns);
+
+  fprintf (file, "namespace ");
+  print_generic_expr (file, ns, 0);
+  fprintf (file, " {\n");
+  for (t = level->names; t; t = chain)
+    {
+      chain = DECL_CHAIN (t);
+      if (!DECL_IS_BUILTIN (t))
+        pph_dump_tree_name (file, t, 0);
+    }
+  for (t = level->namespaces; t; t = chain)
+    {
+      chain = DECL_CHAIN (t);
+      if (!DECL_IS_BUILTIN (t))
+        pph_dump_namespace (file, t);
+    }
+  fprintf (file, "}\n");
+}
+
+
 /* Write PPH output symbols and IDENTS_USED to STREAM as an object.  */
 
 static void
 pph_write_file_contents (pph_stream *stream, cpp_idents_used *idents_used)
 { 
   pth_save_identifiers (idents_used, stream);
+  if (flag_pph_dump_tree)
+    pph_dump_namespace (pph_logfile, global_namespace);
   pph_output_tree (stream, global_namespace, false);
 }
 
@@ -1943,34 +1997,26 @@ report_validation_error (const char *fil
 static void
 pph_add_names_to_namespace (tree ns, tree new_ns)
 {
-  struct cp_binding_level *new_level, *level;
   tree t, chain;
+  struct cp_binding_level *level = NAMESPACE_LEVEL (new_ns);
 
-  level = NAMESPACE_LEVEL (ns);
-  new_level = NAMESPACE_LEVEL (new_ns);
-
-  for (t = new_level->names; t; t = chain)
+  for (t = level->names; t; t = chain)
     {
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
 	 Preserve it.  */
       chain = DECL_CHAIN (t);
-      pushdecl_with_scope (t, level, /*is_friend=*/false);
+      pushdecl_into_namespace (t, ns);
     }
 
-  for (t = new_level->namespaces; t; t = chain)
+  for (t = level->namespaces; t; t = chain)
     {
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
 	 Preserve it.  */
       /* FIXME pph: we should first check to see if it isn't already there.  */
       chain = DECL_CHAIN (t);
-      pushdecl_with_scope (t, level, /*is_friend=*/false);
-      /* FIXME pph: The change above enables the namespace,
-         but its symbols are still missing.
-         The recursive call below causes multiple errors.
+      pushdecl_into_namespace (t, ns);
       pph_add_names_to_namespace (t, t);
-      */
     }
-
 }
 
 
@@ -1999,6 +2045,8 @@ pph_read_file_contents (pph_stream *stre
   /* Read global_namespace from STREAM and add all the names defined
      there to the current global_namespace.  */
   file_ns = pph_input_tree (stream);
+  if (flag_pph_dump_tree)
+    pph_dump_namespace (pph_logfile, file_ns);
   pph_add_names_to_namespace (global_namespace, file_ns);
 }
 
Index: gcc/cp/pph-streamer.c
===================================================================
--- gcc/cp/pph-streamer.c	(revision 172998)
+++ gcc/cp/pph-streamer.c	(working copy)
@@ -140,7 +140,8 @@ pph_stream_trace (pph_stream *stream, co
                            "bitpack" };
 
   if ((type == PPH_TRACE_TREE || type == PPH_TRACE_CHAIN)
-      && !data && flag_pph_tracer <= 3)
+      && !data
+      && flag_pph_tracer <= 3)
     return;
 
   fprintf (pph_logfile, "*** %s: %s%s/%u, value=",
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 172998)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -4756,6 +4756,7 @@ extern tree perform_qualification_conver
 /* in name-lookup.c */
 extern tree pushdecl				(tree);
 extern tree pushdecl_maybe_friend		(tree, bool);
+extern tree pushdecl_into_namespace		(tree, tree);
 extern void maybe_push_cleanup_level		(tree);
 extern tree pushtag				(tree, tree, tag_scope);
 extern tree make_anon_name			(void);
Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 172998)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -1181,6 +1181,24 @@ pushdecl_maybe_friend (tree x, bool is_f
   return ret;
 }
 
+tree
+pushdecl_into_namespace (tree dcl, tree nsp)
+{
+  tree ret;
+  /* FIXME pph: There might be a better way to do this...  */
+  struct cp_binding_level *level = NAMESPACE_LEVEL (nsp);
+  tree saved = current_namespace;
+  current_namespace = nsp;
+#if 0
+  ret = pushdecl_maybe_friend (dcl, /*is_friend=*/false);
+#else
+  ret = pushdecl_with_scope (dcl, level, /*is_friend=*/false);
+#endif
+  current_namespace = saved;
+  return ret;
+}
+
+
 /* Record a decl-node X as belonging to the current lexical scope.  */
 
 tree
Index: libcpp/directives.c
===================================================================
--- libcpp/directives.c	(revision 172998)
+++ libcpp/directives.c	(working copy)
@@ -181,7 +181,7 @@ enum
 /* Make sure the bitfield directive_index in include/cpplib.h is large
    enough to index the entire table.  */
 
-unsigned char too_many_directives_for_bitfield[
+extern unsigned char too_many_directives_for_bitfield[
         N_DIRECTIVES <= (1 << CPP_HASHNODE_INDEX_BITS)
         ? 1 : -1];
 

--
This patch is available for review at http://codereview.appspot.com/4431071

[Diego Novillo, 2011-04-29 22:23:06]

Looks OK.  Some comments below.

http://codereview.appspot.com/4431071/diff/1/gcc/c-family/c.opt
File gcc/c-family/c.opt (right):

http://codereview.appspot.com/4431071/diff/1/gcc/c-family/c.opt#newcode943
gcc/c-family/c.opt:943: 
+fpph-dump-tree
+C++ Var(flag_pph_dump_tree)
+-fpph-dump-tree	Dump global namespace tree around PPH reads/writes.
+

We should just add to the existing -fdump-translation-unit.  Maybe not so
important now.

http://codereview.appspot.com/4431071/diff/1/gcc/cp/name-lookup.c
File gcc/cp/name-lookup.c (right):

http://codereview.appspot.com/4431071/diff/1/gcc/cp/name-lookup.c#newcode1186
gcc/cp/name-lookup.c:1186: {
+tree
+pushdecl_into_namespace (tree dcl, tree nsp)
+{

Needs comment.