[pph] Re-factor streaming of binding levels

[Diego Novillo, 2012-02-13 21:11:16]

This patch re-writes the streaming of binding levels to guarantee that
the whole tree of binding levels in each file is written and merged-in
before anything else.

With this re-factoring, we now write all the binding levels, the merge
keys for symbols/types and their other contents at the start of the
PPH image.  Additionally, we do not skip any namespaces when
traversing the binding level tree (we used to skip over builtin
namespaces, which causes problems when looking up things like
std::ptrdiff_t).

After all the binding levels have been merged-in, every other read of
a binding level is expected to be read as a reference (so that we
don't materialize a new binding level that has not been merged in).

With this change, I get significantly fewer name lookup failures in
our internal code base.  But this is still incomplete.  In chasing
down other failures, I found out that we should be also writing out
the table of canonical types (type_hash_table).  I'm getting a new
ICE, because two different types that compare the same fail the
TYPE_CANONICAL identity test.

Lawrence, to avoid too many merge conflicts with the patch you are
working with, I will be fixing this new failure in a subsequent patch.

Most of this patch is moving code around.  The old
pph_out/in_binding_level is now simply expecting a reference to a
binding level.

The merging into existing binding levels (e.g., the global binding
scope or binding levels for already existing namespaces) is done by
pph_in_binding_level_start.  This routine will take an existing
binding level as parameter and use it in two ways:

   1- If the record read from STREAM is a reference, the binding level
      in that reference must be identical to EXISTING_BL.

   2- If the record read from STREAM is a new instance, the binding
      level given in EXISTING_BL is registered in the cache at the
      slot location given by this record.  This way, subsesequent
      internal references to EXISTING_BL will resolve to EXISTING_BL.
      This is used for binding levels that are already set in the
      compilation (e.g., scope_chain->bindings).

2012-02-13   Diego Novillo  <dnovillo@google.com>

cp/ChangeLog.pph
	* pph-in.c (pph_in_binding_level_start): Move earlier into the
	file.
	Change return type to cp_binding_level *.
	Add argument EXISTING_BL and EXISTED_P.
	If EXISTING_BL is given, and a reference is read from STREAM,
	the reference read should be the same as EXISTING_BL.
	If EXISTING_BL is given and a new reference is started,
	do not allocate a new instance.  Rather, register EXISTING_BL
	in the cache.
	(pph_in_binding_level_ref): Rename from pph_in_binding_level.
	Assert that it always reads a reference record.
	Update all users.
	(pph_in_binding_level_1): Move body inside
	pph_in_merge_body_binding_level.
	(pph_in_merge_key_binding_level): Move earlier in the file.
	(pph_in_merge_body_binding_level): Move earlier in the file.
	(pph_in_merge_body_binding_level_1): Move body into
	pph_in_merge_body_binding_level.
	(pph_in_ld_ns): Call pph_in_binding_level_ref.
	(pph_ensure_namespace_binding_level): Move body into
	pph_in_merge_key_namespace_decl.  Update all users.
	(pph_in_merge_key_namespace_decl): Fix comment.
	(pph_in_global_binding): Call pph_in_binding_level_start.
	* pph-out.c (pph_out_tree_vec_unchain): Remove.
	(pph_out_chain_filtered): Remove.
	(pph_out_binding_level_ref): Rename from
	pph_out_binding_level.  Always expect to write a reference
	record.  Update all users.
	(pph_out_cxx_binding_1): Embed inside pph_out_merge_body_binding_level.
	Update all users.
	(pph_out_merge_key_binding_level): Do not filter
	BL->NAMESPACES.
	(pph_out_merge_body_binding_level): Likewise.

testsuite/ChangeLog.pph
	* g++.dg/pph/x7dynarray5.cc: Add expected failure due to
	type canonical mismatch.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/pph@184170
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/cp/ChangeLog.pph                    |   36 ++++
 gcc/cp/pph-in.c                         |  323 ++++++++++++++-----------------
 gcc/cp/pph-out.c                        |  183 +++++++-----------
 gcc/testsuite/ChangeLog.pph             |    5 +
 gcc/testsuite/g++.dg/pph/x7dynarray5.cc |    2 +
 5 files changed, 260 insertions(+), 289 deletions(-)

diff --git a/gcc/cp/ChangeLog.pph b/gcc/cp/ChangeLog.pph
index b078607..2fa3153 100644
--- a/gcc/cp/ChangeLog.pph
+++ b/gcc/cp/ChangeLog.pph
@@ -1,3 +1,39 @@
+2012-02-13   Diego Novillo  <dnovillo@google.com>
+
+	* pph-in.c (pph_in_binding_level_start): Move earlier into the
+	file.
+	Change return type to cp_binding_level *.
+	Add argument EXISTING_BL and EXISTED_P.
+	If EXISTING_BL is given, and a reference is read from STREAM,
+	the reference read should be the same as EXISTING_BL.
+	If EXISTING_BL is given and a new reference is started,
+	do not allocate a new instance.  Rather, register EXISTING_BL
+	in the cache.
+	(pph_in_binding_level_ref): Rename from pph_in_binding_level.
+	Assert that it always reads a reference record.
+	Update all users.
+	(pph_in_binding_level_1): Move body inside
+	pph_in_merge_body_binding_level.
+	(pph_in_merge_key_binding_level): Move earlier in the file.
+	(pph_in_merge_body_binding_level): Move earlier in the file.
+	(pph_in_merge_body_binding_level_1): Move body into
+	pph_in_merge_body_binding_level.
+	(pph_in_ld_ns): Call pph_in_binding_level_ref.
+	(pph_ensure_namespace_binding_level): Move body into
+	pph_in_merge_key_namespace_decl.  Update all users.
+	(pph_in_merge_key_namespace_decl): Fix comment.
+	(pph_in_global_binding): Call pph_in_binding_level_start.
+	* pph-out.c (pph_out_tree_vec_unchain): Remove.
+	(pph_out_chain_filtered): Remove.
+	(pph_out_binding_level_ref): Rename from
+	pph_out_binding_level.  Always expect to write a reference
+	record.  Update all users.
+	(pph_out_cxx_binding_1): Embed inside pph_out_merge_body_binding_level.
+	Update all users.
+	(pph_out_merge_key_binding_level): Do not filter
+	BL->NAMESPACES.
+	(pph_out_merge_body_binding_level): Likewise.
+
 2012-02-01   Diego Novillo  <dnovillo@google.com>
 
 	* error.c (dump_function_decl): Add missing ';'.
diff --git a/gcc/cp/pph-in.c b/gcc/cp/pph-in.c
index 2853f69..ea968c9 100644
--- a/gcc/cp/pph-in.c
+++ b/gcc/cp/pph-in.c
@@ -1017,10 +1017,83 @@ pph_merge_into_chain (tree expr, const char *name, tree
*chain)
 
 
 /* Forward declaration to break cyclic dependencies.  */
-static cp_binding_level *pph_in_binding_level (pph_stream *);
 static void pph_in_merge_body_namespace_decl (pph_stream *stream);
 static void pph_in_merge_key_namespace_decl (pph_stream *stream, tree *chain);
 
+
+/* Read the start of a binding level from STREAM.  If the binding
+   level had already been read, *EXISTED_P will be set to true,
+   otherwise it will be set to false.
+
+   If EXISTING_BL is given, it means that the caller already has a
+   binding level instance that it would like to use.  In that case,
+   this function will:
+
+   1- If the record read from STREAM is a reference, the binding level
+      in that reference must be identical to EXISTING_BL.
+
+   2- If the record read from STREAM is a new instance, the binding
+      level given in EXISTING_BL is registered in the cache at the
+      slot location given by this record.  This way, subsesequent
+      internal references to EXISTING_BL will resolve to EXISTING_BL.
+      This is used for binding levels that are already set in the
+      compilation (e.g., scope_chain->bindings).
+
+   If EXISTING_BL is NULL, a new one will be allocated, registered and
+   returned.  */
+
+static cp_binding_level *
+pph_in_binding_level_start (pph_stream *stream, cp_binding_level *existing_bl,
+                            bool *existed_p)
+{
+  unsigned ix, image_ix;
+  enum pph_record_marker marker;
+  cp_binding_level *new_bl;
+
+  marker = pph_in_start_record (stream, &image_ix, &ix, PPH_cp_binding_level);
+  if (marker == PPH_RECORD_END)
+    {
+      *existed_p = true;
+      gcc_assert (existing_bl == NULL);
+      return NULL;
+    }
+  else if (pph_is_reference_marker (marker))
+    {
+      *existed_p = true;
+      new_bl = (cp_binding_level *) pph_cache_find (stream, marker, image_ix,
+						    ix, PPH_cp_binding_level);
+      gcc_assert (existing_bl == NULL || new_bl == existing_bl);
+      return new_bl;
+    }
+
+  /* This is the first time we try to read this binding level, allocate a new
+     instance, if the caller did not provide one.  If *BLP is NULL, it
+     means that the caller does not have a binding level and wants to
+     allocate a new one.  */
+  *existed_p = false;
+  new_bl = (existing_bl) ? existing_bl : ggc_alloc_cleared_cp_binding_level ();
+  pph_cache_insert_at (&stream->cache, new_bl, ix, PPH_cp_binding_level);
+  return new_bl;
+}
+
+
+/* Read and return a cp_binding_level from STREAM.  This function
+   expects to find a reference to a binding level, as it is only
+   called after all the binding levels in STREAM have been
+   instantiated and merged.  */
+
+static cp_binding_level *
+pph_in_binding_level_ref (pph_stream *stream)
+{
+  cp_binding_level *bl;
+  bool existed_p;
+
+  bl = pph_in_binding_level_start (stream, NULL, &existed_p);
+  gcc_assert (existed_p);
+  return bl;
+}
+
+
 /* Helper for pph_in_cxx_binding.  Read and return a cxx_binding
    instance from STREAM.  */
 
@@ -1047,7 +1120,7 @@ pph_in_cxx_binding_1 (pph_stream *stream)
   type = pph_in_tree (stream);
   ALLOC_AND_REGISTER (&stream->cache, ix, PPH_cxx_binding, cb,
                       cxx_binding_make (value, type));
-  cb->scope = pph_in_binding_level (stream);
+  cb->scope = pph_in_binding_level_ref (stream);
   bp = pph_in_bitpack (stream);
   cb->value_is_inherited = bp_unpack_value (&bp, 1);
   cb->is_local = bp_unpack_value (&bp, 1);
@@ -1133,14 +1206,59 @@ pph_in_label_binding (pph_stream *stream)
 }
 
 
-/* Read the contents of binding level BL from STREAM.  */
+/* Read all the merge keys from STREAM into the cp_binding_level *BLP.  */
 
 static void
-pph_in_binding_level_1 (pph_stream *stream, cp_binding_level *bl)
+pph_in_merge_key_binding_level (pph_stream *stream, cp_binding_level **blp)
 {
-  unsigned i, num;
+  unsigned count, num;
+  bool existed_p;
+  cp_binding_level *bl;
+
+  /* Get a new binding level instance.  If *BLP was not given,
+     a new one will be created.  */
+  bl = pph_in_binding_level_start (stream, *blp, &existed_p);
+  if (!existed_p && *blp == NULL)
+    *blp = bl;
+
+  gcc_assert (*blp);
+
+  num = pph_in_hwi (stream);
+  for (count = 0; count < num; ++count)
+    pph_in_merge_key_namespace_decl (stream, &bl->namespaces);
+  pph_in_merge_key_chain (stream, &bl->names);
+  pph_in_merge_key_chain (stream, &bl->usings);
+  pph_in_merge_key_chain (stream, &bl->using_directives);
+}
+
+
+/* Read all the merge bodies from STREAM into the cp_binding_level BL.  */
+
+static void
+pph_in_merge_body_binding_level (pph_stream *stream, cp_binding_level *bl)
+{
+  unsigned i, count, num;
+  cp_binding_level *new_bl;
   struct bitpack_d bp;
+  bool existed_p;
 
+  /* We should have already registered BL.  Make sure we are reading a
+     reference to it.  */
+  new_bl = pph_in_binding_level_start (stream, bl, &existed_p);
+  gcc_assert (existed_p && new_bl == bl);
+
+  /* Merge the bodies of BL->NAMESPACES, BL->NAMES, BL->USINGS and
+     BL->USING_DIRECTIVES.  */
+  num = pph_in_hwi (stream);
+  for (count = 0; count < num; ++count)
+    pph_in_merge_body_namespace_decl (stream);
+  pph_in_merge_body_chain (stream);
+  pph_in_merge_body_chain (stream);
+  pph_in_merge_body_chain (stream);
+  pph_union_into_tree_vec (&bl->static_decls, pph_in_tree_vec (stream));
+
+  /* Read the remaining fields in BL.  FIXME pph: The following is probably too
+     aggressive in overwriting.  */
   bl->this_entity = pph_in_tree (stream);
 
   num = pph_in_uint (stream);
@@ -1162,7 +1280,7 @@ pph_in_binding_level_1 (pph_stream *stream,
cp_binding_level *bl)
     }
 
   bl->blocks = pph_in_tree (stream);
-  bl->level_chain = pph_in_binding_level (stream);
+  bl->level_chain = pph_in_binding_level_ref (stream);
   bl->dead_vars_from_for = pph_in_tree_vec (stream);
   bl->statement_list = pph_in_chain (stream);
   bl->binding_depth = pph_in_uint (stream);
@@ -1175,115 +1293,6 @@ pph_in_binding_level_1 (pph_stream *stream,
cp_binding_level *bl)
 }
 
 
-/* Read the start of a binding level.  Return true when processing is done.  */
-
-static bool
-pph_in_binding_level_start (pph_stream *stream, cp_binding_level **blp,
-			    unsigned *ixp)
-{
-  unsigned image_ix;
-  enum pph_record_marker marker;
-
-  marker = pph_in_start_record (stream, &image_ix, ixp, PPH_cp_binding_level);
-  if (marker == PPH_RECORD_END)
-    {
-      *blp = NULL;
-      return true;
-    }
-  else if (pph_is_reference_marker (marker))
-    {
-      *blp = (cp_binding_level *)
-          pph_cache_find (stream, marker, image_ix, *ixp,
PPH_cp_binding_level);
-      return true;
-    }
-  return false;
-}
-
-
-/* Read and return an instance of cp_binding_level from STREAM.
-   This function is for use when we will not be merging the binding level.  */
-
-static cp_binding_level *
-pph_in_binding_level (pph_stream *stream)
-{
-  unsigned ix;
-  cp_binding_level *bl;
-
-  if (pph_in_binding_level_start (stream, &bl, &ix))
-    return bl;
-
-  bl = ggc_alloc_cleared_cp_binding_level ();
-  pph_cache_insert_at (&stream->cache, bl, ix, PPH_cp_binding_level);
-
-  bl->names = pph_in_chain (stream);
-  bl->namespaces = pph_in_chain (stream);
-  bl->usings = pph_in_chain (stream);
-  bl->using_directives = pph_in_chain (stream);
-  bl->static_decls = pph_in_tree_vec (stream);
-  pph_in_binding_level_1 (stream, bl);
-
-  return bl;
-}
-
-
-/* Read all the merge keys from STREAM into the cp_binding_level BL.
-   If BL is NULL, it means that this is the first time we read BL,
-   so no merging is actually required.  In this case, the merge keys
-   for every symbol and type will be read and cache entries will be
-   created for them, so that their bodies can be read after we finish
-   reading all the merge keys for BL.  */
-
-static void
-pph_in_merge_key_binding_level (pph_stream *stream, cp_binding_level *bl)
-{
-  unsigned count, num;
-  num = pph_in_hwi (stream);
-  for (count = 0; count < num; ++count)
-    pph_in_merge_key_namespace_decl (stream, &bl->namespaces);
-  /* FIXME pph:  The null check should be unnecessary. */
-  pph_in_merge_key_chain (stream, (bl) ? &bl->names : NULL);
-  pph_in_merge_key_chain (stream, (bl) ? &bl->usings : NULL);
-  pph_in_merge_key_chain (stream, (bl) ? &bl->using_directives : NULL);
-}
-
-
-/* Read all the merge bodies from STREAM into the cp_binding_level BL.  */
-
-static void
-pph_in_merge_body_binding_level_1 (pph_stream *stream, cp_binding_level *bl)
-{
-  unsigned count, num;
-  num = pph_in_hwi (stream);
-  for (count = 0; count < num; ++count)
-    pph_in_merge_body_namespace_decl (stream);
-  pph_in_merge_body_chain (stream);
-  pph_in_merge_body_chain (stream);
-  pph_in_merge_body_chain (stream);
-  pph_union_into_tree_vec (&bl->static_decls, pph_in_tree_vec (stream));
-  /* FIXME pph: The following is probably too aggressive in overwriting.  */
-  pph_in_binding_level_1 (stream, bl);
-}
-
-/* Read all the merge bodies from STREAM into the cp_binding_level BL.  */
-
-static void
-pph_in_merge_body_binding_level (pph_stream *stream, cp_binding_level *bl)
-{
-  unsigned ix;
-  cp_binding_level *new_bl;
-
-  if (pph_in_binding_level_start (stream, &new_bl, &ix))
-    {
-      gcc_assert (new_bl == bl);
-      return;
-    }
-
-  /* The binding level is already allocated.  */
-  pph_cache_insert_at (&stream->cache, bl, ix, PPH_cp_binding_level);
-  pph_in_merge_body_binding_level_1 (stream, bl);
-}
-
-
 /********************************************************** tree aux classes */
 
 
@@ -1323,7 +1332,7 @@ pph_in_language_function (pph_stream *stream)
 
   /* FIXME pph.  We are not reading lf->x_named_labels.  */
 
-  lf->bindings = pph_in_binding_level (stream);
+  lf->bindings = pph_in_binding_level_ref (stream);
   lf->x_local_names = pph_in_tree_vec (stream);
 
   /* FIXME pph.  We are not reading lf->extern_decl_map.  */
@@ -1537,10 +1546,7 @@ pph_in_ld_fn (pph_stream *stream, struct lang_decl_fn
*ldf)
 static void
 pph_in_ld_ns (pph_stream *stream, struct lang_decl_ns *ldns)
 {
-  if (ldns->level == NULL)
-    ldns->level = pph_in_binding_level (stream);
-  else
-    pph_in_merge_body_binding_level (stream, ldns->level);
+  ldns->level = pph_in_binding_level_ref (stream);
 }
 
 
@@ -2244,19 +2250,6 @@ pph_in_tree_header (pph_stream *stream, bool
*fully_read_p)
 }
 
 
-/* Ensure that a binding level exists for a namespace DECL. */
-
-static void
-pph_ensure_namespace_binding_level (tree decl)
-{
-  if (!DECL_LANG_SPECIFIC (decl))
-    {
-      retrofit_lang_decl (decl);
-      NAMESPACE_LEVEL (decl) = ggc_alloc_cleared_cp_binding_level ();
-    }
-}
-
-
 /* Read all the merge keys for the names under namespace DECL from
    STREAM.  */
 
@@ -2316,18 +2309,11 @@ pph_in_merge_key_namespace_decl (pph_stream *stream,
tree *chain)
   is_namespace_alias = pph_in_bool (stream);
   if (!is_namespace_alias)
     {
-      /* FIXME pph this comment is broken.  */
-      /* If this is the first time we see DECL, it will not have
-	 a decl_lang nor a binding level associated with it.  This
-	 means that we do not actually need to do any merging, so
-	 we just read all the merge keys for its binding level.
-
-	 This will create cache entries for every symbol and type
-	 under DECL, but it will not try to attempt any merging.
-	 This is fine.  The namespace will be populated once we read
-	 DECL's body.  */
-      pph_ensure_namespace_binding_level (decl);
-      pph_in_merge_key_binding_level (stream, NAMESPACE_LEVEL (decl));
+      /* If this is the first time we see DECL, it will not have a decl_lang
+	 structure associated with it.  Create it, if needed.  */
+      if (!DECL_LANG_SPECIFIC (decl))
+	retrofit_lang_decl (decl);
+      pph_in_merge_key_binding_level (stream, &NAMESPACE_LEVEL (decl));
     }
 
   if (flag_pph_tracer)
@@ -2392,7 +2378,7 @@ pph_in_merge_body_namespace_decl (pph_stream *stream)
     {
       gcc_assert (DECL_LANG_SPECIFIC (decl));
       gcc_assert (NAMESPACE_LEVEL (decl));
-      pph_in_merge_body_binding_level_1 (stream, NAMESPACE_LEVEL (decl));
+      pph_in_merge_body_binding_level (stream, NAMESPACE_LEVEL (decl));
     }
 
   if (flag_pph_tracer)
@@ -2871,46 +2857,35 @@ pph_in_identifiers (pph_stream *stream, cpp_idents_used
*identifiers)
 }
 
 
-/* Read global bindings from STREAM into scope_chain->bindings.  Note
-   that this does not call pph_in_binding_level because that would
-   overwrite the fields merged by pph_in_merge_keys.  */
+/* Read global bindings from STREAM and merge them into
+   scope_chain->bindings.  Bindings are merged at every level starting
+   at the global bindings from STREAM.  */
 
 static void
 pph_in_global_binding (pph_stream *stream)
 {
-  unsigned image_ix, ix;
-  enum pph_record_marker marker;
-  cp_binding_level *bl;
+  cp_binding_level *bl, *other_bl;
+  bool existed_p;
 
   bl = scope_chain->bindings;
-  marker = pph_in_start_record (stream, &image_ix, &ix, PPH_cp_binding_level);
-  gcc_assert (marker != PPH_RECORD_END);
+  other_bl = pph_in_binding_level_start (stream, bl, &existed_p);
 
-  if (pph_is_reference_marker (marker))
-    {
-      /* If we found a reference to BL, it should be the same.  This happens
-	 when we pull BL from a nested PPH image.  */
-      cp_binding_level *other_bl;
-      other_bl = (cp_binding_level *) pph_cache_find (stream, marker, image_ix,
-						      ix, PPH_cp_binding_level);
-      gcc_assert (other_bl == bl);
-    }
-  else
-    {
-      /* Note that here we do not allocate a new binding level.  Since we
-	 are reading into the global one, we register the current
-	 scope_chain->bindings into the cache.  Otherwise, we would be
-	 associating these symbols into the global binding level from
-	 STREAM, instead of the current translation unit.  */
-      pph_cache_insert_at (&stream->cache, bl, ix, PPH_cp_binding_level);
-    }
+  /* If we found a reference to scope_chain->bindings, it should be
+     the same instance.  This happens when we pull BL from a nested
+     PPH image.  Otherwise, the call pph_in_binding_level_start should
+     have registered scope_chain->bindings in the cache.  */
+  gcc_assert (existed_p || other_bl == bl);
 
   /* Read the merge keys and merge them into the current compilation
      context.  Since we have registered scope_chain->bindings in the
      same slot IX that the writer used, the trees read now will be
      bound to scope_chain->bindings.  */
-  pph_in_merge_key_binding_level (stream, bl);
-  pph_in_merge_body_binding_level_1 (stream, bl);
+  pph_in_merge_key_binding_level (stream, &bl);
+
+  /* Once all the symbols and types at every binding level have been
+     merged to the corresponding binding levels in the current
+     compilation, read all the bodies.  */
+  pph_in_merge_body_binding_level (stream, bl);
 }
 
 
diff --git a/gcc/cp/pph-out.c b/gcc/cp/pph-out.c
index 11a749a..84fcaf8 100644
--- a/gcc/cp/pph-out.c
+++ b/gcc/cp/pph-out.c
@@ -939,10 +939,8 @@ pph_out_token_cache (pph_stream *f, cp_token_cache *cache)
 /******************************************************************* vectors */
 
 /* Note that we use the same format used by streamer_write_chain.
-   This is to support pph_out_chain_filtered, which writes the
-   filtered chain as a VEC.  Since the reader always reads chains
-   using streamer_read_chain, we have to write VECs in exactly the
-   same way as tree chains.  */
+   Since the reader always reads chains using streamer_read_chain, we
+   have to write VECs in exactly the same way as tree chains.  */
 
 /* Return true if T matches FILTER for STREAM.  */
 
@@ -1004,26 +1002,6 @@ pph_out_tree_vec (pph_stream *stream, VEC(tree,gc) *v)
 }
 
 
-/* Write all the trees in VEC V to STREAM.
-   Clear TREE_CHAIN on every element written out (this is to support
-   writing chains, as they are supposed to be re-chained by the reader).  */
-
-static void
-pph_out_tree_vec_unchain (pph_stream *stream, VEC(tree,gc) *v)
-{
-  unsigned i;
-  tree t;
-  pph_out_hwi (stream, VEC_length (tree, v));
-  FOR_EACH_VEC_ELT (tree, v, i, t)
-    {
-      tree prev = TREE_CHAIN (t);
-      TREE_CHAIN (t) = NULL;
-      pph_out_tree (stream, t);
-      TREE_CHAIN (t) = prev;
-    }
-}
-
-
 /* Write all the merge keys for trees in VEC V to STREAM.  The keys
    must go out in declaration order, i.e. reversed.  */
 
@@ -1151,18 +1129,6 @@ pph_out_chain (pph_stream *stream, tree first)
 }
 
 
-/* Write a chain of trees to stream starting with FIRST.  Only write
-   the trees that match FILTER.  */
-
-static void
-pph_out_chain_filtered (pph_stream *stream, tree first, unsigned filter)
-{
-  VEC(tree,heap) *w = chain2vec_filter (stream, first, filter);
-  pph_out_tree_vec_unchain (stream, (VEC(tree,gc) *)w);
-  VEC_free (tree, heap, w);
-}
-
-
 /* Write, in reverse, a chain of merge keys to STREAM starting
    with the last element of CHAIN.  Only write the trees that match
    FILTER.  */
@@ -1201,11 +1167,22 @@ pph_out_merge_body_chain (pph_stream *stream, tree
chain, unsigned filter)
 
 
 /* Forward declaration to break cyclic dependencies.  */
-static void pph_out_binding_level (pph_stream *, cp_binding_level *, unsigned);
 static void pph_out_merge_key_namespace_decl (pph_stream *stream, tree ns);
 static void pph_out_merge_body_namespace_decl (pph_stream *stream, tree ns);
 
 
+/* Write a reference to binding level BL to STREAM.  */
+
+static void
+pph_out_binding_level_ref (pph_stream *stream, cp_binding_level *bl)
+{
+  enum pph_record_marker marker;
+  marker = pph_out_start_record (stream, bl, PPH_cp_binding_level);
+  gcc_assert (pph_is_reference_or_end_marker (marker));
+  return;
+}
+
+
 /* Helper for pph_out_cxx_binding.  STREAM and CB are as in
    pph_out_cxx_binding.  */
 
@@ -1224,7 +1201,7 @@ pph_out_cxx_binding_1 (pph_stream *stream, cxx_binding
*cb)
 
   pph_out_tree (stream, cb->value);
   pph_out_tree (stream, cb->type);
-  pph_out_binding_level (stream, cb->scope, PPHF_NONE);
+  pph_out_binding_level_ref (stream, cb->scope);
   bp = bitpack_create (stream->encoder.w.ob->main_stream);
   bp_pack_value (&bp, cb->value_is_inherited, 1);
   bp_pack_value (&bp, cb->is_local, 1);
@@ -1290,67 +1267,6 @@ pph_out_label_binding (pph_stream *stream,
cp_label_binding *lb)
 }
 
 
-/* Helper for pph_out_binding_level.  Write the fields of BL to
-   STREAM.  Do not emit any nodes in BL that do not match FILTER.  */
-
-static void
-pph_out_binding_level_1 (pph_stream *stream, cp_binding_level *bl,
-		         unsigned filter)
-{
-  unsigned i;
-  cp_class_binding *cs;
-  cp_label_binding *sl;
-  struct bitpack_d bp;
-
-  pph_out_tree (stream, bl->this_entity);
-
-  pph_out_uint (stream, VEC_length (cp_class_binding, bl->class_shadowed));
-  FOR_EACH_VEC_ELT (cp_class_binding, bl->class_shadowed, i, cs)
-    pph_out_class_binding (stream, cs);
-
-  pph_out_tree (stream, bl->type_shadowed);
-
-  pph_out_uint (stream, VEC_length (cp_label_binding, bl->shadowed_labels));
-  FOR_EACH_VEC_ELT (cp_label_binding, bl->shadowed_labels, i, sl)
-    pph_out_label_binding (stream, sl);
-
-  pph_out_tree (stream, bl->blocks);
-  pph_out_binding_level (stream, bl->level_chain, filter);
-  pph_out_tree_vec (stream, bl->dead_vars_from_for);
-  pph_out_chain (stream, bl->statement_list);
-  pph_out_uint (stream, bl->binding_depth);
-
-  bp = bitpack_create (stream->encoder.w.ob->main_stream);
-  bp_pack_value (&bp, bl->kind, 4);
-  bp_pack_value (&bp, bl->keep, 1);
-  bp_pack_value (&bp, bl->more_cleanups_ok, 1);
-  bp_pack_value (&bp, bl->have_cleanups, 1);
-  pph_out_bitpack (stream, &bp);
-}
-
-
-/* Write all the fields of cp_binding_level instance BL to STREAM for the
-   non-merging case.  Do not emit any nodes in BL that do not match FILTER.  */
-
-static void
-pph_out_binding_level (pph_stream *stream, cp_binding_level *bl,
-		       unsigned filter)
-{
-  enum pph_record_marker marker;
-
-  marker = pph_out_start_record (stream, bl, PPH_cp_binding_level);
-  if (pph_is_reference_or_end_marker (marker))
-    return;
-
-  pph_out_chain_filtered (stream, bl->names, filter);
-  pph_out_chain_filtered (stream, bl->namespaces, filter);
-  pph_out_chain_filtered (stream, bl->usings, filter);
-  pph_out_chain_filtered (stream, bl->using_directives, filter);
-  pph_out_tree_vec_filtered (stream, bl->static_decls, filter);
-  pph_out_binding_level_1 (stream, bl, filter);
-}
-
-
 /* Iterate over a chain FIRST, apply FILTER, and call output FUNCTION.  */
 
 static void
@@ -1374,13 +1290,18 @@ pph_foreach_out_chain (pph_stream *stream, tree first,
unsigned filter,
 static void
 pph_out_merge_key_binding_level (pph_stream *stream, cp_binding_level *bl)
 {
-  unsigned filter1 = PPHF_NO_PREFS | PPHF_NO_BUILTINS;
-  unsigned filter2 = PPHF_NO_XREFS | PPHF_NO_PREFS | PPHF_NO_BUILTINS;
-  pph_foreach_out_chain (stream, bl->namespaces, filter1,
+  unsigned filter;
+  enum pph_record_marker marker;
+
+  marker = pph_out_start_record (stream, bl, PPH_cp_binding_level);
+  gcc_assert (marker != PPH_RECORD_END);
+
+  pph_foreach_out_chain (stream, bl->namespaces, PPHF_NONE,
 			 pph_out_merge_key_namespace_decl);
-  pph_out_merge_key_chain (stream, bl->names, filter2);
-  pph_out_merge_key_chain (stream, bl->usings, filter2);
-  pph_out_merge_key_chain (stream, bl->using_directives, filter2);
+  filter = PPHF_NO_XREFS | PPHF_NO_PREFS | PPHF_NO_BUILTINS;
+  pph_out_merge_key_chain (stream, bl->names, filter);
+  pph_out_merge_key_chain (stream, bl->usings, filter);
+  pph_out_merge_key_chain (stream, bl->using_directives, filter);
 }
 
 
@@ -1390,15 +1311,47 @@ pph_out_merge_key_binding_level (pph_stream *stream,
cp_binding_level *bl)
 static void
 pph_out_merge_body_binding_level (pph_stream *stream, cp_binding_level *bl)
 {
-  unsigned filter1 = PPHF_NO_PREFS | PPHF_NO_BUILTINS;
-  unsigned filter2 = PPHF_NO_XREFS | PPHF_NO_PREFS | PPHF_NO_BUILTINS;
-  pph_foreach_out_chain (stream, bl->namespaces, filter1,
+  unsigned i, filter;
+  cp_class_binding *cs;
+  cp_label_binding *sl;
+  struct bitpack_d bp;
+
+  pph_out_binding_level_ref (stream, bl);
+
+  /* Write the fields that need merge reads.  */
+  pph_foreach_out_chain (stream, bl->namespaces, PPHF_NONE,
 			 pph_out_merge_body_namespace_decl);
-  pph_out_merge_body_chain (stream, bl->names, filter2);
-  pph_out_merge_body_chain (stream, bl->usings, filter2);
-  pph_out_merge_body_chain (stream, bl->using_directives, filter2);
-  pph_out_tree_vec_filtered (stream, bl->static_decls, filter2);
-  pph_out_binding_level_1 (stream, bl, filter2);
+  filter = PPHF_NO_XREFS | PPHF_NO_PREFS | PPHF_NO_BUILTINS;
+  pph_out_merge_body_chain (stream, bl->names, filter);
+  pph_out_merge_body_chain (stream, bl->usings, filter);
+  pph_out_merge_body_chain (stream, bl->using_directives, filter);
+  pph_out_tree_vec_filtered (stream, bl->static_decls, filter);
+
+  /* Write the remaining fields.  */
+  pph_out_tree (stream, bl->this_entity);
+
+  pph_out_uint (stream, VEC_length (cp_class_binding, bl->class_shadowed));
+  FOR_EACH_VEC_ELT (cp_class_binding, bl->class_shadowed, i, cs)
+    pph_out_class_binding (stream, cs);
+
+  pph_out_tree (stream, bl->type_shadowed);
+
+  pph_out_uint (stream, VEC_length (cp_label_binding, bl->shadowed_labels));
+  FOR_EACH_VEC_ELT (cp_label_binding, bl->shadowed_labels, i, sl)
+    pph_out_label_binding (stream, sl);
+
+  pph_out_tree (stream, bl->blocks);
+  pph_out_binding_level_ref (stream, bl->level_chain);
+  pph_out_tree_vec (stream, bl->dead_vars_from_for);
+  pph_out_chain (stream, bl->statement_list);
+  pph_out_uint (stream, bl->binding_depth);
+
+  bp = bitpack_create (stream->encoder.w.ob->main_stream);
+  bp_pack_value (&bp, bl->kind, 4);
+  bp_pack_value (&bp, bl->keep, 1);
+  bp_pack_value (&bp, bl->more_cleanups_ok, 1);
+  bp_pack_value (&bp, bl->have_cleanups, 1);
+  pph_out_bitpack (stream, &bp);
 }
 
 
@@ -1439,7 +1392,7 @@ pph_out_language_function (pph_stream *stream, struct
language_function *lf)
 
   /* FIXME pph.  We are not writing lf->x_named_labels.  */
 
-  pph_out_binding_level (stream, lf->bindings, PPHF_NONE);
+  pph_out_binding_level_ref (stream, lf->bindings);
   pph_out_tree_vec (stream, lf->x_local_names);
 
   /* FIXME pph.  We are not writing lf->extern_decl_map.  */
@@ -1621,7 +1574,7 @@ pph_out_ld_fn (pph_stream *stream, struct lang_decl_fn
*ldf)
 static void
 pph_out_ld_ns (pph_stream *stream, struct lang_decl_ns *ldns)
 {
-  pph_out_binding_level (stream, ldns->level, PPHF_NONE);
+  pph_out_binding_level_ref (stream, ldns->level);
 }
 
 
diff --git a/gcc/testsuite/ChangeLog.pph b/gcc/testsuite/ChangeLog.pph
index 3248e91..c68f4db 100644
--- a/gcc/testsuite/ChangeLog.pph
+++ b/gcc/testsuite/ChangeLog.pph
@@ -1,3 +1,8 @@
+2012-02-13   Diego Novillo  <dnovillo@google.com>
+
+	* g++.dg/pph/x7dynarray5.cc: Add expected failure due to
+	type canonical mismatch.
+
 2012-02-06   Diego Novillo  <dnovillo@google.com>
 
 	* g++.dg/pph/x0string-escapes.h: New.
diff --git a/gcc/testsuite/g++.dg/pph/x7dynarray5.cc
b/gcc/testsuite/g++.dg/pph/x7dynarray5.cc
index 740c10d..8c050fc 100644
--- a/gcc/testsuite/g++.dg/pph/x7dynarray5.cc
+++ b/gcc/testsuite/g++.dg/pph/x7dynarray5.cc
@@ -1,4 +1,6 @@
 // { dg-xfail-if "BOGUS POSSIBLY DROPPING SYMBOLS " { "*-*-*" } {
"-fpph-map=pph.map" } }
+// { dg-bogus "internal compiler error: canonical types differ for identical
types" "" { xfail *-*-* } 0 }
+// { dg-excess-errors "type mismatch errors due to TYPE_CANONICAL problems." }
 
 #include "x0dynarray4.h"
 #include "x6dynarray5.h"
-- 
1.7.7.3


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