From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21978 invoked by alias); 15 Oct 2011 00:28:02 -0000 Received: (qmail 21970 invoked by uid 22791); 15 Oct 2011 00:28:02 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-yx0-f173.google.com (HELO mail-yx0-f173.google.com) (209.85.213.173) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 15 Oct 2011 00:27:41 +0000 Received: by yxk38 with SMTP id 38so1887186yxk.18 for ; Fri, 14 Oct 2011 17:27:41 -0700 (PDT) Received: by 10.146.181.41 with SMTP id d41mr2329070yaf.11.1318638461065; Fri, 14 Oct 2011 17:27:41 -0700 (PDT) MIME-Version: 1.0 Received: by 10.147.38.4 with HTTP; Fri, 14 Oct 2011 17:27:11 -0700 (PDT) In-Reply-To: <4E982809.1010302@google.com> References: <20111011202636.D76B41DA1D2@topo.tor.corp.google.com> <4E982809.1010302@google.com> From: Gabriel Charette Date: Sat, 15 Oct 2011 08:00:00 -0000 Message-ID: Subject: Re: [pph] Make libcpp symbol validation a warning (issue5235061) To: Diego Novillo Cc: reply@codereview.appspotmail.com, crowl@google.com, gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-10/txt/msg01363.txt.bz2 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 =3D=3D num_expected_childs basically (it is still only an assert so no big deal I guess). 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. Gab On Fri, Oct 14, 2011 at 8:16 AM, Diego Novillo wrote: > > On 11-10-13 17:55 , Gabriel Charette wrote: > >> I'm not sure exactly how you skip headers already parsed now (we >> didn't used to when I wrote this code and that was the only problem >> remaining in the line_table (i.e. duplicate entries for guarded >> headers in the non-pph compile)), but couldn't you count the number of >> skipped entries and assert (line_table->used - used_before) + >> numSkipped =3D=3D expected_in) ? > > The problem is that the compilation process of foo.h -> foo.pph may gener= ate different line tables than a compile that includes foo.pph. For instanc= e, > > foo.h: > #include "1.pph" > #include "2.pph" > #include "3.pph" > > foo.cc: > #include "2.pph" > #include "foo.pph" > > > When we compile foo.h, the line table incorporates the effects of includi= ng 2.pph, and that's what we save to foo.pph. =C2=A0However, when compiling= foo.cc, the first thing we do is include 2.pph, so when processing the inc= lude for foo.pph, we will completely skip over 2.pph. > > That's why we cannot really have the same line table that we had when we = generated foo.pph. > > > Diego.