I just thought about something..
Earlier I said that ALL line_table issues were resolved after this
patch (as it ignores the re-included headers that were guarded, as the
non-pph compiler does naturally).
One problem remains however, I'm pretty sure that re-included
non-pph'ed header's line_table entries are still showing up multiple
times (as the direct non-pph childs of a given pph_include have their
line_table entries copied one by one from the pph file).
I think we were talking about somehow remembering guards context in
which DECLs were declared and then ignoring DECLs streamed in if they
belong to a given "header guard type" that was previously seen in a
prior include using the same non-pph header, allowing us to ignore
those DECLs that are re-declared when they should have been guarded
out the second time.
I'm not sure whether there is machinery to handle non-pph re-includes
yet... but... at the very least, I'm pretty sure those non-pph entries
still show up multiple times in the line_table.
Now, we can't just remove/ignore those entries either... doing so
would alter the expected location offset (pph_loc_offset) applied to
all tokens streamed in directly from the pph header.
What we could potentially do is:
- ignore the repeated non-pph entry
- remember the number of locations this entry was "supposed" to take
(call that pph_loc_ignored_offset)
- then for DECLs imported after it we would then need an offset of
pph_loc_offset - pph_loc_ignored_offset, to compensate for the missing
entries in the line_table
The problem here obviously is that I don't think we have a way of
knowing which DECLs come before, inside, and after a given non-pph
header included in the parent pph header which we are currently
reading.
Furthermore, a DECL coming after the non-pph header could potentially
refer to something inside the ignored non-pph header and the
source_location of the referred token would now be invalid (although
that might already be fixed by the cache hit which would redirect that
token reference to the same token in the first included copy of that
same header which wasn't actually skipped as it was first and which is
valid)
On Tue, Oct 11, 2011 at 4:26 PM, Diego Novillo <dnovi...@google.com> wrote:
> @@ -328,8 +327,6 @@ pph_in_line_table_and_includes (pph_stream *stream)
> int entries_offset = line_table->used - PPH_NUM_IGNORED_LINE_TABLE_ENTRIES;
> enum pph_linetable_marker next_lt_marker = pph_in_linetable_marker (stream);
>
> - pph_reading_includes++;
> -
> for (first = true; next_lt_marker != PPH_LINETABLE_END;
> next_lt_marker = pph_in_linetable_marker (stream))
> {
> @@ -373,19 +370,33 @@ pph_in_line_table_and_includes (pph_stream *stream)
> else
> lm->included_from += entries_offset;
>
Also, if we do ignore some non-pph entries, the included_from
calculation is going to need some trickier logic as well (it's fine
for the pph includes though as each child calculates it's own offset)
> - gcc_assert (lm->included_from < (int) line_table->used);
> -
Also, I think this slipped in my previous comment, but I don't see how
this assert could trigger in the current code. If it did trigger
something was definitely wrong as it asserts the offseted
included_from is referring to an entry that is actually in the
line_table...
> lm->start_location += pph_loc_offset;
Cheers,
Gab
> --
> This patch is available for review at http://codereview.appspot.com/5235061
>
