So it looks like my mail using the upload script didn't make it out...
let me retype it...
On 2011/06/22 20:51:58, Diego Novillo wrote:
http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c
File gcc/cp/pph-streamer-in.c (right):
http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-in.c#newcode1003
gcc/cp/pph-streamer-in.c:1003: namespace.
1001 /* FIXME pph: this carried over from
pph_add_names_to_namespace,
1002 » it only makes sense to use this when merging names in
an existing
1003 » namespace.
pph_add_names_to_namespace does not exist anymore. I don't understand
this
comment.
What I meant is that pph_add_bindings_to_namespace is essentially just a
modified version
of pph_add_names_to_namespace which used to do this recursive call
(which was useless as it
only makes sense to add the bindings from the "streamed in" namespace if
we found a corresponding
existing namespace, adding the bindings streamed in to itself makes no
sense (as they're already there)... and commenting out that line in the
old code wouldn't change anything in the test results, proving my
point.)
I fixed the comment: removing it and elaborating on the FIXME above it
mentioning that
we need to look if the namespace already exists.
http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c
File gcc/cp/pph-streamer-out.c (right):
http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c#newcode947
gcc/cp/pph-streamer-out.c:947: gcc_assert ( ss->old_namespace ==
global_namespace
945 /* old_namespace should be global_namespace and all entries
listed below
946 should be NULL or 0; otherwise the header parsed was
incomplete. */
947 gcc_assert ( ss->old_namespace == global_namespace
No space after '('.
FIXED.
http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer-out.c#newcode949
gcc/cp/pph-streamer-out.c:949: || ss->function_decl ||
ss->template_parms ||
ss->x_saved_tree
948 && !(ss->class_name || ss->class_type ||
ss->access_specifier
949 || ss->function_decl || ss->template_parms ||
ss->x_saved_tree
Align '&&' vertically with the open brace.
FIXED.
http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer.c
File gcc/cp/pph-streamer.c (right):
http://codereview.appspot.com/4661045/diff/1/gcc/cp/pph-streamer.c#newcode34
gcc/cp/pph-streamer.c:34: #include "name-lookup.h"
33 #include "cppbuiltin.h"
+ 34 #include "name-lookup.h"
You also need to add cp/name-lookup.h to the list of dependencies for
cp/pph.o
in cp/Make-lang.in.
I removed the include, I originally included it because that's where
global_namespace is defined, but it compiles without it (which I guess
is fine if we don't need to stick to "include what you use").
http://codereview.appspot.com/4661045/
