On 10/20/11, Gabriel Charette <gcharet...@gmail.com> wrote: > 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;
I'm wondering if we shouldn't just whitelist the problematic cases that we know about in the system/standard headers. It seems that all others we could reasonably complain to the maintainers of the code. -- Lawrence Crowl