Re: [pph] Add initial support for including nested pph images (issue4847044)

[Diego Novillo <dnovillo at google dot com>]

On Thu, Aug 4, 2011 at 17:47,  <gchare@google.com> wrote:
> So if I understand correctly, the reference system doesn't work yet, you
> just put out the design in this patch and you'll fill up functionality
> in an upcoming patch so that's it's clearer what you're adding?

Right, the 'if (0)' in pph_in_includes() disables support for it.

> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph-streamer-in.c
> File gcc/cp/pph-streamer-in.c (right):
>
> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph-streamer-in.c#newcode1491
> gcc/cp/pph-streamer-in.c:1491: pph_add_include (stream);
> does this mean the included pph has the include information for all of
> the headers in it's transitive closure? i.e. we don't need to load child
> information of a child? if so, isn't this bad from a dependency point of
> view?

Hmm?  Each header knows what other images it includes, so before
reading its own content it reads the contents from the others.  The
dependencies don't change.  The parent will not have the physical
representation of the ASTs from the children, just references to them.

> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph.c
> File gcc/cp/pph.c (right):
>
> http://codereview.appspot.com/4847044/diff/1/gcc/cp/pph.c#newcode164
> gcc/cp/pph.c:164: if (pph_out_file != NULL)
> shouldn't you use your new pph_enabled_p function here?

No.  I need to know whether we are *creating* a PPH image.  Perhaps I
should add a different predicate.

> http://codereview.appspot.com/4847044/diff/1/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc
> File gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc (right):
>
> http://codereview.appspot.com/4847044/diff/1/gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc#newcode1
> gcc/testsuite/g++.dg/pph/c1pr44948-1a.cc:1: /* { dg-options "-O
> -Wno-psabi -mtune=generic" } */
> Is this related to the rest of the changes in this patch? I don't see it
> mentioned in your patch comments

Gah.  I was going to commit this separately but forgot.  Sorry about that.


Diego.