public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: gchare@google.com (Gabriel Charette)
To: reply@codereview.appspotmail.com, crowl@google.com,
	dnovillo@google.com,        gcc-patches@gcc.gnu.org
Subject: [pph] Fix cxx_binding streaming logic (issue4646041)
Date: Thu, 16 Jun 2011 19:13:00 -0000	[thread overview]
Message-ID: <20110616185715.CDA091C3548@gchare.mtv.corp.google.com> (raw)

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

             reply	other threads:[~2011-06-16 18:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-16 19:13 Gabriel Charette [this message]
2011-06-17 12:26 ` Diego Novillo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110616185715.CDA091C3548@gchare.mtv.corp.google.com \
    --to=gchare@google.com \
    --cc=crowl@google.com \
    --cc=dnovillo@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=reply@codereview.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).