Re: [pph] Make libcpp symbol validation a warning (issue5235061)

[Diego Novillo <dnovillo@google.com>]

On Fri, Oct 14, 2011 at 20:27, Gabriel Charette <gcharette1@gmail.com> wrote:
> Yes, I understand that.
>
> But when the second 2.pph is skipped when reading foo.pph, the reading
> of its line_table is also skipped (as foo.pph doesn't contain the
> line_table information for 2.h, 2.pph does and adds it when its
> included as a child, but if it's skipped, the line_table info for 2.h
> should never make it in the line_table), so I don't see why this is an
> issue for the line_table (other than the assert about the number of
> line table entries read). What I was suggesting is that as far as the
> assert is concerned it would be stronger to count the number of
> skipped child headers on read and assert num_read+num_skipped ==
> num_expected_childs basically (it is still only an assert so no big
> deal I guess).

Ah, I see what you are saying.  I didn't really bother too much with
that assert.  Since I was not reading the line table again, I figured
both asserts were triggering because of the different values coming
from the skipped file, so I left them out.

>
> Essentially this patch fixes the last bug we had in the line_table
> merging (i.e. that guarded out headers in the non-pph version weren't
> guarded out in the pph version) and this is a good thing. I'm just
> being picky about weakening asserts!
>
>
> I still think it would be nice to have a way to test constructs like
> the line_table at the end of parsing (maybe a new flag, as I was
> suggesting in my previous email, as gcc doesn't allow for modular
> testing) and compare pph and non-pph versions. Testing at this level
> would potentially be much better than trying to understand tricky test
> failures from the ground up. This is beyond the scope of this patch of
> course, but something to keep in mind I think.

Yeah.  I'll come back to it at a later point.


Diego.