Re: [pph] Stream merging information (issue 5090041)
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)
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)
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/