Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[gchare]

So it looks like my mail using the upload script didn't make it out...
let me retype it...

On 2011/06/22 20:51:58, Diego Novillo wrote:
> http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c
> File gcc/cp/pph-streamer-in.c (right):


http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c#newcode1003
> gcc/cp/pph-streamer-in.c:1003: namespace.
>   1001       /* FIXME pph: this carried over from
pph_add_names_to_namespace,
>   1002 »        it only makes sense to use this when merging names in
an existing
>   1003 »        namespace.

> pph_add_names_to_namespace does not exist anymore.  I don't understand
this
> comment.

What I meant is that pph_add_bindings_to_namespace is essentially just a
modified version
of pph_add_names_to_namespace which used to do this recursive call
(which was useless as it
only makes sense to add the bindings from the "streamed in" namespace if
we found a corresponding
existing namespace, adding the bindings streamed in to itself makes no
sense (as they're already there)... and commenting out that line in the
old code wouldn't change anything in the test results, proving my
point.)

I fixed the comment: removing it and elaborating on the FIXME above it
mentioning that
we need to look if the namespace already exists.


> http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c
> File gcc/cp/pph-streamer-out.c (right):


http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c#newcode947
> gcc/cp/pph-streamer-out.c:947: gcc_assert ( ss->old_namespace ==
> global_namespace
>    945   /* old_namespace should be global_namespace and all entries
listed below
>    946      should be NULL or 0; otherwise the header parsed was
incomplete.  */
>    947   gcc_assert ( ss->old_namespace == global_namespace

> No space after '('.

FIXED.



http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c#newcode949
> gcc/cp/pph-streamer-out.c:949: || ss->function_decl ||
ss->template_parms ||
> ss->x_saved_tree
>    948       && !(ss->class_name || ss->class_type ||
ss->access_specifier
>    949            || ss->function_decl || ss->template_parms ||
ss->x_saved_tree

> Align '&&' vertically with the open brace.

FIXED.


> http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer.c
> File gcc/cp/pph-streamer.c (right):


http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer.c#newcode34
> gcc/cp/pph-streamer.c:34: #include "name-lookup.h"
>    33 #include "cppbuiltin.h"
> + 34 #include "name-lookup.h"

> You also need to add cp/name-lookup.h to the list of dependencies for
cp/pph.o
> in cp/Make-lang.in.

I removed the include, I originally included it because that's where
global_namespace is defined, but it compiles without it (which I guess
is fine if we don't need to stick to "include what you use").


http://codereview.appspot.com/4661045/

Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[Diego Novillo]

On Wed, Jun 22, 2011 at 18:25,  <gchare@google.com> wrote:

> I fixed the comment: removing it and elaborating on the FIXME above it
> mentioning that
> we need to look if the namespace already exists.

Ah, OK.  Thanks.

> I removed the include, I originally included it because that's where
> global_namespace is defined, but it compiles without it (which I guess
> is fine if we don't need to stick to "include what you use").

Yeah, we don't have that rule in GCC.


Diego.

Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[Diego Novillo]

I've made a couple of minor edits to comments and formatting and
committed to the branch (final patch below).


Diego.

commit 2f0fa40cd3e0c9debb4efae6c65530a7c6d3fb0f
Author: dnovillo <dnovillo@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Jun 23 17:18:27 2011 +0000

    2011-06-22  Gabriel Charette  <gchare@google.com>

        * pph-streamer-in.c (pph_add_names_to_namespace): Replaced by
        pph_add_bindings_to_namespace.
        (pph_add_bindings_to_namespace): New.
        (pph_in_scope_chain): New.
        (pph_read_file_contents): Remove unused variable file_ns.
        (pph_read_file_contents): Call pph_in_scope_chain.
        * pph-streamer-out.c (pph_out_scope_chain): New.
        (pph_write_file_contents): Call pph_out_scope_chain.
        * pph-streamer.c (pph_preload_common_nodes):
        Call lto_streamer_cache_append.

    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph@175346
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/cp/ChangeLog.pph b/gcc/cp/ChangeLog.pph
index 8cdc575..24e1584 100644
--- a/gcc/cp/ChangeLog.pph
+++ b/gcc/cp/ChangeLog.pph
@@ -1,5 +1,18 @@
 2011-06-22  Gabriel Charette  <gchare@google.com>

+       * pph-streamer-in.c (pph_add_names_to_namespace): Replaced by
+       pph_add_bindings_to_namespace.
+       (pph_add_bindings_to_namespace): New.
+       (pph_in_scope_chain): New.
+       (pph_read_file_contents): Remove unused variable file_ns.
+       (pph_read_file_contents): Call pph_in_scope_chain.
+       * pph-streamer-out.c (pph_out_scope_chain): New.
+       (pph_write_file_contents): Call pph_out_scope_chain.
+       * pph-streamer.c (pph_preload_common_nodes):
+       Call lto_streamer_cache_append.
+
+2011-06-22  Gabriel Charette  <gchare@google.com>
+
        * pph-streamer-out.c (pph_out_lang_specific): Removed extra space.
        (pph_write_tree): Removed extra space.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index 0e8c6bf..7d501ef 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -977,15 +977,14 @@ pph_in_lang_type (pph_stream *stream)
 }


-/* Add all the new names declared in NEW_NS to NS.  */
+/* Add all bindings declared in BL to NS.  */


 static void
-pph_add_names_to_namespace (tree ns, tree new_ns)
+pph_add_bindings_to_namespace (struct cp_binding_level *bl, tree ns)
 {
   tree t, chain;
-  struct cp_binding_level *level = NAMESPACE_LEVEL (new_ns);

-  for (t = level->names; t; t = chain)
+  for (t = bl->names; t; t = chain)
     {
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
         Preserve it.  */
@@ -993,18 +992,34 @@ pph_add_names_to_namespace (tree ns, tree new_ns)
       pushdecl_into_namespace (t, ns);
     }

-  for (t = level->namespaces; t; t = chain)
+  for (t = bl->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);
+
+      /* FIXME pph: we should first check to see if it isn't already there.
+        If it is, we should use this function recursively to merge
+        the bindings in T in the corresponding namespace.  */
       pushdecl_into_namespace (t, ns);
-      pph_add_names_to_namespace (t, t);
     }
 }


+/* Merge scope_chain bindings from STREAM into global_namespace. */
+
+static void
+pph_in_scope_chain (pph_stream *stream)
+{
+  struct cp_binding_level *pph_bindings;
+
+  pph_bindings = pph_in_binding_level (stream);
+
+  /* Merge the bindings obtained from STREAM in the global namespace.  */
+  pph_add_bindings_to_namespace (pph_bindings, global_namespace);
+}
+
+
 /* Wrap a macro DEFINITION for printing in an error.  */

 static char *
@@ -1129,7 +1144,6 @@ pph_read_file_contents (pph_stream *stream)
   cpp_ident_use *bad_use;
   const char *cur_def;
   cpp_idents_used idents_used;
-  tree file_ns;

   pth_load_identifiers (&idents_used, stream);

@@ -1142,16 +1156,16 @@ pph_read_file_contents (pph_stream *stream)
   /* Re-instantiate all the pre-processor symbols defined by STREAM.  */
   cpp_lt_replay (parse_in, &idents_used);

-  /* Read global_namespace from STREAM and add all the names defined
-     there to the current global_namespace.  */
-  file_ns = pph_in_tree (stream);
+  /* Read the bindings from STREAM and merge them with the current
bindings.  */
+  pph_in_scope_chain (stream);
+
   if (flag_pph_dump_tree)
-    pph_dump_namespace (pph_logfile, file_ns);
-  pph_add_names_to_namespace (global_namespace, file_ns);

+    pph_dump_namespace (pph_logfile, global_namespace);
+
   keyed_classes = pph_in_tree (stream);
-  unemitted_tinfo_decls = pph_in_tree_vec (stream);
   /* FIXME pph: This call replaces the tinfo, we should merge instead.
-     See pph_in_tree_VEC.  */
+     See pph_in_tree_vec.  */
+  unemitted_tinfo_decls = pph_in_tree_vec (stream);
 }


diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index f219cef..dd26571 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -912,6 +912,35 @@ pph_out_lang_type (pph_stream *stream, tree type,
bool ref_p)
 }


+/* Write saved_scope information stored in SS into STREAM.
+   This does NOT output all fields, it is meant to be used for the
+   global variable "scope_chain" only.  Output bindings as references
+   if REF_P is true.  */
+
+static void
+pph_out_scope_chain (pph_stream *stream, struct saved_scope *ss, bool ref_p)
+{
+  /* old_namespace should be global_namespace and all entries listed below
+     should be NULL or 0; otherwise the header parsed was incomplete.  */
+  gcc_assert (ss->old_namespace == global_namespace
+             && !(ss->class_name || ss->class_type || ss->access_specifier
+                  || ss->function_decl || ss->template_parms
+                  || ss->x_saved_tree || ss->class_bindings || ss->prev
+                  || ss->unevaluated_operand
+                  || ss->inhibit_evaluation_warnings
+                  || ss->x_processing_template_decl
+                  || ss->x_processing_specialization
+                  || ss->x_processing_explicit_instantiation
+                  || ss->need_pop_function_context
+                  || ss->x_stmt_tree.x_cur_stmt_list
+                  || ss->x_stmt_tree.stmts_are_full_exprs_p));
+
+  /* We only need to write out the bindings, everything else should
+     be NULL or be some temporary disposable state.  */
+  pph_out_binding_level (stream, ss->bindings, ref_p);
+}
+
+
 /* Save the IDENTIFIERS to the STREAM.  */

 static void
@@ -968,9 +997,9 @@ static void
 pph_write_file_contents (pph_stream *stream, cpp_idents_used *idents_used)
 {
   pth_save_identifiers (idents_used, stream);
+  pph_out_scope_chain (stream, scope_chain, false);
   if (flag_pph_dump_tree)
     pph_dump_namespace (pph_logfile, global_namespace);
-  pph_out_tree (stream, global_namespace, false);
   pph_out_tree (stream, keyed_classes, false);
   pph_out_tree_vec (stream, unemitted_tinfo_decls, false);
 }
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 7c9b862..e919baf 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -78,6 +78,8 @@ pph_preload_common_nodes (struct lto_streamer_cache_d *cache)
   for (i = 0; i < CTI_MAX; i++)
     if (c_global_trees[i])
       lto_streamer_cache_append (cache, c_global_trees[i]);

+
+  lto_streamer_cache_append (cache, global_namespace);
 }

Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[Diego Novillo]

On Thu, Jun 23, 2011 at 13:21, Diego Novillo <dnovillo@google.com> wrote:
> I've made a couple of minor edits to comments and formatting and
> committed to the branch (final patch below).

Incidentally, did you fill-in the svn write access form?  You've
produced enough good patches already.  Time for you to be able to
commit your own.


Diego.

Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[Gabriel Charette]

Yes I did fill the form, included you as an approver, haven't heard
back from it yet.

Gab

On Thu, Jun 23, 2011 at 10:23 AM, Diego Novillo <dnovillo@google.com> wrote:
>
> On Thu, Jun 23, 2011 at 13:21, Diego Novillo <dnovillo@google.com> wrote:
> > I've made a couple of minor edits to comments and formatting and
> > committed to the branch (final patch below).
>
> Incidentally, did you fill-in the svn write access form?  You've
> produced enough good patches already.  Time for you to be able to
> commit your own.
>
>
> Diego.

[pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[Gabriel Charette]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7980 bytes --]

gcc/cp/pph-streamer-in.c:1003: namespace.
 1001       /* FIXME pph: this carried over from pph_add_names_to_namespace,
 1002 »        it only makes sense to use this when merging names in an existing
 1003 »        namespace.

pph_add_names_to_namespace does not exist anymore.  I don't understand this
comment.

What I meant is that pph_add_bindings_to_namespace is just a modified version of
the old pph_add_names_to_namespace in which this recursive call was made (and was
already useless before, i.e. commenting it out wouldn't introduce any changes in the
test results...).

I changed the comment, adding it to the FIXME just above it which mentions we should
check if this namespace already exists (i.e. it only makes sense to add bindings
in the "streamed in" namespace to the actual namespace IF they are NOT the same object
(otherwise the bindings are already part of the "streamed in" namespace so this call is useless).

######################################
gcc/cp/pph-streamer-out.c:947: gcc_assert ( ss->old_namespace ==
global_namespace
  945   /* old_namespace should be global_namespace and all entries listed below
  946      should be NULL or 0; otherwise the header parsed was incomplete.  */
  947   gcc_assert ( ss->old_namespace == global_namespace

No space after '('.

FIXED.

######################################
gcc/cp/pph-streamer-out.c:949: || ss->function_decl || ss->template_parms ||
ss->x_saved_tree
  948       && !(ss->class_name || ss->class_type || ss->access_specifier
  949            || ss->function_decl || ss->template_parms || ss->x_saved_tree

Align '&&' vertically with the open brace.

FIXED.

######################################
gcc/cp/pph-streamer.c:34: #include "name-lookup.h"
  33 #include "cppbuiltin.h"
+ 34 #include "name-lookup.h"

You also need to add cp/name-lookup.h to the list of dependencies for cp/pph.o
in cp/Make-lang.in.

I simply removed the include, I originally added it because it holds global_namespace, but it
compiles without it (which I guess is fine if we don't try to stick to "include what you use").

2011-06-22  Gabriel Charette  <gchare@google.com>

	* gcc/cp/pph-streamer-in.c (pph_add_names_to_namespace): Replaced by
	pph_add_bindings_to_namespace.
	(pph_add_bindings_to_namespace): New.
	(pph_in_scope_chain): New.
	(pph_read_file_contents): Remove unused variable file_ns.
	(pph_read_file_contents): Call pph_in_scope_chain.
	* gcc/cp/pph-streamer-out.c (pph_out_scope_chain): New.
	(pph_write_file_contents): Call pph_out_scope_chain.
	* gcc/cp/pph-streamer.c (pph_preload_common_nodes):
	Call lto_streamer_cache_append.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index e71f744..c16b88d 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -976,15 +976,14 @@ pph_in_lang_type (pph_stream *stream)
 }
 
 
-/* Add all the new names declared in NEW_NS to NS.  */
+/* Add all bindings declared in BL to NS.  */
 
 static void
-pph_add_names_to_namespace (tree ns, tree new_ns)
+pph_add_bindings_to_namespace (struct cp_binding_level *bl, tree ns)
 {
   tree t, chain;
-  struct cp_binding_level *level = NAMESPACE_LEVEL (new_ns);
 
-  for (t = level->names; t; t = chain)
+  for (t = bl->names; t; t = chain)
     {
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
 	 Preserve it.  */
@@ -992,18 +991,33 @@ pph_add_names_to_namespace (tree ns, tree new_ns)
       pushdecl_into_namespace (t, ns);
     }
 
-  for (t = level->namespaces; t; t = chain)
+  for (t = bl->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.  */
+      /* FIXME pph: we should first check to see if it isn't already there.  
+		    If it is, we should use this function recursively to merge
+		    the bindings in T in the corresponding namespace.  */
       chain = DECL_CHAIN (t);
       pushdecl_into_namespace (t, ns);
-      pph_add_names_to_namespace (t, t);
     }
 }
 
 
+/* Merge scope_chain bindings from the stream into SS. */
+
+static void
+pph_in_scope_chain (pph_stream *stream)
+{
+  struct cp_binding_level *pph_bindings;
+
+  pph_bindings = pph_in_binding_level (stream);
+
+  /* Merge the bindings obtained from STREAM in the global namespace.  */
+  pph_add_bindings_to_namespace (pph_bindings, global_namespace);
+}
+
+
 /* Wrap a macro DEFINITION for printing in an error.  */
 
 static char *
@@ -1128,7 +1142,6 @@ pph_read_file_contents (pph_stream *stream)
   cpp_ident_use *bad_use;
   const char *cur_def;
   cpp_idents_used idents_used;
-  tree file_ns;
 
   pth_load_identifiers (&idents_used, stream);
 
@@ -1141,12 +1154,12 @@ pph_read_file_contents (pph_stream *stream)
   /* Re-instantiate all the pre-processor symbols defined by STREAM.  */
   cpp_lt_replay (parse_in, &idents_used);
 
-  /* Read global_namespace from STREAM and add all the names defined
-     there to the current global_namespace.  */
-  file_ns = pph_in_tree (stream);
+  /* Read the bindings from STREAM and merge them with the current bindings.  */
+  pph_in_scope_chain (stream);
+
   if (flag_pph_dump_tree)
-    pph_dump_namespace (pph_logfile, file_ns);
-  pph_add_names_to_namespace (global_namespace, file_ns);
+    pph_dump_namespace (pph_logfile, global_namespace);
+
   keyed_classes = pph_in_tree (stream);
   unemitted_tinfo_decls = pph_in_tree_vec (stream);
   /* FIXME pph: This call replaces the tinfo, we should merge instead.
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 3187338..4c3e4a5 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -935,6 +935,34 @@ pph_out_lang_type (pph_stream *stream, tree type, bool ref_p)
 }
 
 
+/* Write saved_scope information stored in SS, does NOT output all fields,
+   meant to be used for the global variable "scope_chain" only.
+   Output bindings as references if REF_P is true.  */
+
+static void
+pph_out_scope_chain (pph_stream *stream, struct saved_scope *ss, bool ref_p)
+{
+  /* old_namespace should be global_namespace and all entries listed below
+     should be NULL or 0; otherwise the header parsed was incomplete.  */
+  gcc_assert (ss->old_namespace == global_namespace
+	      && !(ss->class_name || ss->class_type || ss->access_specifier
+		   || ss->function_decl || ss->template_parms
+		   || ss->x_saved_tree || ss->class_bindings || ss->prev
+		   || ss->unevaluated_operand
+		   || ss->inhibit_evaluation_warnings
+		   || ss->x_processing_template_decl
+		   || ss->x_processing_specialization
+		   || ss->x_processing_explicit_instantiation
+		   || ss->need_pop_function_context
+		   || ss->x_stmt_tree.x_cur_stmt_list
+		   || ss->x_stmt_tree.stmts_are_full_exprs_p));
+
+  /* We only need to write out the bindings, everything else should
+     be NULL or be some temporary disposable state.  */
+  pph_out_binding_level (stream, ss->bindings, ref_p);
+}
+
+
 /* Save the IDENTIFIERS to the STREAM.  */
 
 static void
@@ -991,9 +1019,9 @@ static void
 pph_write_file_contents (pph_stream *stream, cpp_idents_used *idents_used)
 { 
   pth_save_identifiers (idents_used, stream);
+  pph_out_scope_chain (stream, scope_chain, false);
   if (flag_pph_dump_tree)
     pph_dump_namespace (pph_logfile, global_namespace);
-  pph_out_tree (stream, global_namespace, false);
   pph_out_tree (stream, keyed_classes, false);
   pph_out_tree_vec (stream, unemitted_tinfo_decls, false);
 }
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 7c9b862..e919baf 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -78,6 +78,8 @@ pph_preload_common_nodes (struct lto_streamer_cache_d *cache)
   for (i = 0; i < CTI_MAX; i++)
     if (c_global_trees[i])
       lto_streamer_cache_append (cache, c_global_trees[i]);
+
+  lto_streamer_cache_append (cache, global_namespace);
 }
 
 

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

Re: [pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[dnovillo]

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c
File gcc/cp/pph-streamer-in.c (right):

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c#newcode1003
gcc/cp/pph-streamer-in.c:1003: namespace.
  1001       /* FIXME pph: this carried over from
pph_add_names_to_namespace,
  1002 »        it only makes sense to use this when merging names in an
existing
  1003 »        namespace.

pph_add_names_to_namespace does not exist anymore.  I don't understand
this comment.

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c
File gcc/cp/pph-streamer-out.c (right):

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c#newcode947
gcc/cp/pph-streamer-out.c:947: gcc_assert ( ss->old_namespace ==
global_namespace
   945   /* old_namespace should be global_namespace and all entries
listed below
   946      should be NULL or 0; otherwise the header parsed was
incomplete.  */
   947   gcc_assert ( ss->old_namespace == global_namespace

No space after '('.

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c#newcode949
gcc/cp/pph-streamer-out.c:949: || ss->function_decl ||
ss->template_parms || ss->x_saved_tree
   948       && !(ss->class_name || ss->class_type ||
ss->access_specifier
   949            || ss->function_decl || ss->template_parms ||
ss->x_saved_tree

Align '&&' vertically with the open brace.

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer.c
File gcc/cp/pph-streamer.c (right):

http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer.c#newcode34
gcc/cp/pph-streamer.c:34: #include "name-lookup.h"
   33 #include "cppbuiltin.h"
+ 34 #include "name-lookup.h"

You also need to add cp/name-lookup.h to the list of dependencies for
cp/pph.o in cp/Make-lang.in.

http://codereview.appspot.com/4661045/

[pph] Stream scope_chain->bindings instead of global namespace (issue4661045)

[Gabriel Charette]

We were streaming out the whole global namespace tree and only using its
bindings on the way in. These bindings (defined by NAMESPACE_LEVEL (global_namespace))
are the same as scope_chain->bindings.

Stream those bindings only instead.

This also allows us to append the global_namespace itself to the lto cache
early so that anything pointing to global namespace in the underlying
structure of the bindings (and anything else potentially?!) will point to the
actual global namespace when streamed back in (as opposed to the stale version
we were streaming in).

Can someone confirm that we really didn't need to stream anything but the
bindings from the global_namespace?

This patch also changes the behaviour of the dumper (used for debug only) in
that it dumps the global namespace on read AFTER the bindings were merged with
the current global namespace; as opposed to the prior behaviour which was
to dump the namespace READ in.
This is actually a good thing, because I just realized some of the bindings
are read, but not merged correctly (working on that next), and this exposes
it.

2011-06-22  Gabriel Charette  <gchare@google.com>

	* gcc/cp/pph-streamer-in.c (pph_add_names_to_namespace): Replaced by
	pph_add_bindings_to_namespace.
	(pph_add_bindings_to_namespace): New.
	(pph_in_scope_chain): New.
	(pph_read_file_contents): Remove unused variable file_ns.
	(pph_read_file_contents): Call pph_in_scope_chain.
	* gcc/cp/pph-streamer-out.c (pph_out_scope_chain): New.
	(pph_write_file_contents): Call pph_out_scope_chain.
	* gcc/cp/pph-streamer.c (pph_preload_common_nodes):
	Call lto_streamer_cache_append.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index e71f744..2e3545a 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -976,15 +976,14 @@ pph_in_lang_type (pph_stream *stream)
 }
 
 
-/* Add all the new names declared in NEW_NS to NS.  */
+/* Add all bindings declared in BL to NS.  */
 
 static void
-pph_add_names_to_namespace (tree ns, tree new_ns)
+pph_add_bindings_to_namespace (struct cp_binding_level *bl, tree ns)
 {
   tree t, chain;
-  struct cp_binding_level *level = NAMESPACE_LEVEL (new_ns);
 
-  for (t = level->names; t; t = chain)
+  for (t = bl->names; t; t = chain)
     {
       /* Pushing a decl into a scope clobbers its DECL_CHAIN.
 	 Preserve it.  */
@@ -992,18 +991,35 @@ pph_add_names_to_namespace (tree ns, tree new_ns)
       pushdecl_into_namespace (t, ns);
     }
 
-  for (t = level->namespaces; t; t = chain)
+  for (t = bl->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_into_namespace (t, ns);
-      pph_add_names_to_namespace (t, t);
+      /* FIXME pph: this carried over from pph_add_names_to_namespace,
+	 it only makes sense to use this when merging names in an existing
+	 namespace.
+      pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t);  */
     }
 }
 
 
+/* Merge scope_chain bindings from the stream into SS. */
+
+static void
+pph_in_scope_chain (pph_stream *stream)
+{
+  struct cp_binding_level *pph_bindings;
+
+  pph_bindings = pph_in_binding_level (stream);
+
+  /* Merge the bindings obtained from STREAM in the global namespace.  */
+  pph_add_bindings_to_namespace (pph_bindings, global_namespace);
+}
+
+
 /* Wrap a macro DEFINITION for printing in an error.  */
 
 static char *
@@ -1128,7 +1144,6 @@ pph_read_file_contents (pph_stream *stream)
   cpp_ident_use *bad_use;
   const char *cur_def;
   cpp_idents_used idents_used;
-  tree file_ns;
 
   pth_load_identifiers (&idents_used, stream);
 
@@ -1141,12 +1156,12 @@ pph_read_file_contents (pph_stream *stream)
   /* Re-instantiate all the pre-processor symbols defined by STREAM.  */
   cpp_lt_replay (parse_in, &idents_used);
 
-  /* Read global_namespace from STREAM and add all the names defined
-     there to the current global_namespace.  */
-  file_ns = pph_in_tree (stream);
+  /* Read the bindings from STREAM and merge them with the current bindings.  */
+  pph_in_scope_chain (stream);
+
   if (flag_pph_dump_tree)
-    pph_dump_namespace (pph_logfile, file_ns);
-  pph_add_names_to_namespace (global_namespace, file_ns);
+    pph_dump_namespace (pph_logfile, global_namespace);
+
   keyed_classes = pph_in_tree (stream);
   unemitted_tinfo_decls = pph_in_tree_vec (stream);
   /* FIXME pph: This call replaces the tinfo, we should merge instead.
diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
index 3187338..691cfdd 100644
--- a/gcc/cp/pph-streamer-out.c
+++ b/gcc/cp/pph-streamer-out.c
@@ -935,6 +935,33 @@ pph_out_lang_type (pph_stream *stream, tree type, bool ref_p)
 }
 
 
+/* Write saved_scope information stored in SS, does NOT output all fields,
+   meant to be used for the global variable "scope_chain" only.
+   Output bindings as references if REF_P is true.  */
+
+static void
+pph_out_scope_chain (pph_stream *stream, struct saved_scope *ss, bool ref_p)
+{
+  /* old_namespace should be global_namespace and all entries listed below
+     should be NULL or 0; otherwise the header parsed was incomplete.  */
+  gcc_assert ( ss->old_namespace == global_namespace
+      && !(ss->class_name || ss->class_type || ss->access_specifier
+	   || ss->function_decl || ss->template_parms || ss->x_saved_tree
+	   || ss->class_bindings || ss->prev || ss->unevaluated_operand
+	   || ss->inhibit_evaluation_warnings
+	   || ss->x_processing_template_decl
+	   || ss->x_processing_specialization
+	   || ss->x_processing_explicit_instantiation
+	   || ss->need_pop_function_context
+	   || ss->x_stmt_tree.x_cur_stmt_list
+	   || ss->x_stmt_tree.stmts_are_full_exprs_p));
+
+  /* We only need to write out the bindings, everything else should
+     be NULL or be some temporary disposable state.  */
+  pph_out_binding_level (stream, ss->bindings, ref_p);
+}
+
+
 /* Save the IDENTIFIERS to the STREAM.  */
 
 static void
@@ -991,9 +1018,9 @@ static void
 pph_write_file_contents (pph_stream *stream, cpp_idents_used *idents_used)
 { 
   pth_save_identifiers (idents_used, stream);
+  pph_out_scope_chain (stream, scope_chain, false);
   if (flag_pph_dump_tree)
     pph_dump_namespace (pph_logfile, global_namespace);
-  pph_out_tree (stream, global_namespace, false);
   pph_out_tree (stream, keyed_classes, false);
   pph_out_tree_vec (stream, unemitted_tinfo_decls, false);
 }
diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
index 7c9b862..1e0a8c4 100644
--- a/gcc/cp/pph-streamer.c
+++ b/gcc/cp/pph-streamer.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-pass.h"
 #include "version.h"
 #include "cppbuiltin.h"
+#include "name-lookup.h"
 
 /* Return true if the given tree T is streamable.  */
 
@@ -78,6 +79,8 @@ pph_preload_common_nodes (struct lto_streamer_cache_d *cache)
   for (i = 0; i < CTI_MAX; i++)
     if (c_global_trees[i])
       lto_streamer_cache_append (cache, c_global_trees[i]);
+
+  lto_streamer_cache_append (cache, global_namespace);
 }
 
 

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