Re: [pph] Stream merging information (issue 5090041)

2011-09-26 Thread dnovillo


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

http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-out.c#newcode1924
gcc/cp/pph-streamer-out.c:1924: tree enclosing_namespace )
 1922 void
 1923 pph_write_namespace_tree (pph_stream *stream, tree expr,
 1924   tree enclosing_namespace )

Just found this while changing other code.  This needs a comment.  no
space before ')'.

http://codereview.appspot.com/5090041/


Re: [pph] Stream merging information (issue 5090041)

2011-09-22 Thread Lawrence Crowl
On 9/21/11, dnovi...@google.com dnovi...@google.com wrote:
 http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-in.c
 File gcc/cp/pph-streamer-in.c (right):

 http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-in.c#newcode2146
 gcc/cp/pph-streamer-in.c:2146: pph_read_namespace_chain (pph_stream
 *stream, tree enclosing_namespace)
   2142 /* Read a chain of tree nodes from input block IB. DATA_IN
 contains
   2143tables and descriptors for the file being read.  */
   2144
   2145 tree
   2146 pph_read_namespace_chain (pph_stream *stream, tree
 enclosing_namespace)

 ENCLOSING_NAMESPACE needs documenting.

Copy/paste/edit failure.

 Would it be better to have the original pph_read_chain() get
 this argument?  Not crazy about this duplication of code.

There is no origional pph_read_chain.  The pph_in_chain uses
streamer_read_chain.  I didn't think altering that API was the
right thing to do for a pph-specific feature.

 Same comment applies to the other two routines that the patch
 duplicates.

In the end, I decided that the duplication was likely to be worth it
to avoid all the pointer passing and checking.  Most trees streamed
will not be direct children of namespaces.

-- 
Lawrence Crowl


Re: [pph] Stream merging information (issue 5090041)

2011-09-21 Thread dnovillo


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

http://codereview.appspot.com/5090041/diff/1/gcc/cp/pph-streamer-in.c#newcode2146
gcc/cp/pph-streamer-in.c:2146: pph_read_namespace_chain (pph_stream
*stream, tree enclosing_namespace)
 2142 /* Read a chain of tree nodes from input block IB. DATA_IN
contains
 2143tables and descriptors for the file being read.  */
 2144
 2145 tree
 2146 pph_read_namespace_chain (pph_stream *stream, tree
enclosing_namespace)

ENCLOSING_NAMESPACE needs documenting.  Would it be better to have the
original pph_read_chain() get this argument?  Not crazy about this
duplication of code.

Same comment applies to the other two routines that the patch
duplicates.

http://codereview.appspot.com/5090041/