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

[Diego Novillo <dnovillo at google dot com>]

On Fri, Aug 5, 2011 at 14:10, Gabriel Charette <gchare@google.com> wrote:
> On Fri, Aug 5, 2011 at 9:42 AM, Diego Novillo <dnovillo@google.com> wrote:
>> 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.
>>
>
> What I mean is if you have the following include structure A.c -> B.h
> -> C.h -> D.h
>
> Would B.h hold references to both C.h and D.h or only to C which in
> turn knows about D?

Ah, no.  B.pph only knows about C.pph.  When we load C.pph, it then
proceeds to load D.pph.
So, we shouldn't be loading D.pph twice when including B.pph.  I think
this now doesn't work and we will try to load D twice (which is a
bug).


>> Gah. ÂI was going to commit this separately but forgot. ÂSorry about that.
>>
> K, why does this fix the test? seems weird that we had an infinite
> loop before and changing the flags gets rid of it?

It wasn't an infinite loop but an ICE in the tree caches.  The
different options were producing different ASTs, so the tree cache had
different trees on write-out and read-in.


Diego.