[pph] Fix cxx_binding streaming logic (issue4646041)

[pph] Fix cxx_binding streaming logic (issue4646041)

[Gabriel Charette]

We were losing bindings when reassigning prev bindings with the current
streaming logic.

This doesn't fix any currently exposed bugs, but again helps me as part of
fixing the bigger bug I'm on.

Tested with bootstrap build and pph regression testing.

At the end of pph_out_cxx_binding I could also simply do 
pph_out_uchar (stream, PPH_RECORD_END);
instead of
pph_out_cxx_binding_1 (stream, NULL, ref_p);
which has the exact same effect.

I just thought it might clearer to the reader and also more robust code
if we eventually decide to change the way caching works internally.

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

	* gcc/cp/pph-streamer-in.c (pph_in_cxx_binding): Fix streaming logic.
	* gcc/cp/pph-streamer-out.c (pph_out_cxx_binding): Fix streaming logic.

Index: gcc/cp/pph-streamer-in.c
===================================================================
--- gcc/cp/pph-streamer-in.c	(revision 175106)
+++ gcc/cp/pph-streamer-in.c	(working copy)
@@ -353,24 +353,18 @@
 static cxx_binding *
 pph_in_cxx_binding (pph_stream *stream)
 {
-  unsigned i, num_bindings;
-  cxx_binding *curr, *cb;
+  cxx_binding *curr, *prev, *cb;
 
+  /* Read the current binding first.  */
+  cb = pph_in_cxx_binding_1 (stream);
+
   /* Read the list of previous bindings.  */
-  num_bindings = pph_in_uint (stream);
-  for (curr = NULL, i = 0; i < num_bindings; i++)
+  for (curr = cb; curr; curr = prev)
     {
-      cxx_binding *prev = pph_in_cxx_binding_1 (stream);
-      if (curr)
-	curr->previous = prev;
-      curr = prev;
+      prev = pph_in_cxx_binding_1 (stream);
+      curr->previous = prev;
     }
 
-  /* Read the current binding at the end.  */
-  cb = pph_in_cxx_binding_1 (stream);
-  if (cb)
-    cb->previous = curr;
-
   return cb;
 }
 
Index: gcc/cp/pph-streamer-out.c
===================================================================
--- gcc/cp/pph-streamer-out.c	(revision 175106)
+++ gcc/cp/pph-streamer-out.c	(working copy)
@@ -339,21 +339,18 @@
 static void
 pph_out_cxx_binding (pph_stream *stream, cxx_binding *cb, bool ref_p)
 {
-  unsigned num_bindings;
   cxx_binding *prev;
 
-  num_bindings = 0;
-  for (prev = cb ? cb->previous : NULL; prev; prev = prev->previous)
-    num_bindings++;
+  /* Write the current binding first.  */
+  pph_out_cxx_binding_1 (stream, cb, ref_p);
 
   /* Write the list of previous bindings.  */
-  pph_out_uint (stream, num_bindings);
-  if (num_bindings > 0)
-    for (prev = cb->previous; prev; prev = prev->previous)
-      pph_out_cxx_binding_1 (stream, prev, ref_p);
+  for (prev = cb ? cb->previous : NULL; prev; prev = prev->previous)
+    pph_out_cxx_binding_1 (stream, prev, ref_p);
 
-  /* Write the current binding at the end.  */
-  pph_out_cxx_binding_1 (stream, cb, ref_p);
+  /* Mark the end of the list (if there was a list).  */
+  if (cb)
+    pph_out_cxx_binding_1 (stream, NULL, ref_p);
 }
 
 

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

Re: [pph] Fix cxx_binding streaming logic (issue4646041)

[Diego Novillo]

On Thu, Jun 16, 2011 at 18:57, Gabriel Charette <gchare@google.com> wrote:

> At the end of pph_out_cxx_binding I could also simply do
> pph_out_uchar (stream, PPH_RECORD_END);
> instead of
> pph_out_cxx_binding_1 (stream, NULL, ref_p);
> which has the exact same effect.

That's fine.  I kind of prefer the way you did it.

> 2011-06-16  Gabriel Charette  <gchare@google.com>
>
>        * gcc/cp/pph-streamer-in.c (pph_in_cxx_binding): Fix streaming logic.
>        * gcc/cp/pph-streamer-out.c (pph_out_cxx_binding): Fix streaming logic.

OK, committed in rev 175143.


Diego.