From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106786 invoked by alias); 21 Mar 2016 22:58:46 -0000 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 Received: (qmail 106773 invoked by uid 89); 21 Mar 2016 22:58:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=filenames, xh, worthwhile, *2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 21 Mar 2016 22:58:44 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id C104EC00B8D1 for ; Mon, 21 Mar 2016 22:58:42 +0000 (UTC) Received: from localhost.localdomain (vpn1-7-193.ams2.redhat.com [10.36.7.193]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2LMwetM020668; Mon, 21 Mar 2016 18:58:42 -0400 Subject: Re: Fix 69650, bogus line numbers from libcpp To: David Malcolm , GCC Patches References: <56E12FFF.6050800@t-online.de> <56E132FC.2030404@redhat.com> <1457734166.5425.43.camel@redhat.com> <56E6BAB1.6030804@redhat.com> <1458591323.9902.81.camel@redhat.com> From: Bernd Schmidt Message-ID: <56F07CA0.5040607@redhat.com> Date: Tue, 22 Mar 2016 01:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <1458591323.9902.81.camel@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg01224.txt.bz2 On 03/21/2016 09:15 PM, David Malcolm wrote: > On Mon, 2016-03-14 at 14:20 +0100, Bernd Schmidt wrote: >> On 03/11/2016 11:09 PM, David Malcolm wrote: >>>> + cpp_error (pfile, CPP_DL_ERROR, >>>> + "file \"%s\" left but not entered", >>>> new_file); >>> ^^^^^^^^ >>> Although it looks like you're preserving the existing behavior from >>> when this was in linemap_add, shouldn't this be >>> ORDINARY_MAP_FILE_NAME (from) >>> rather than new_file? (i.e. shouldn't it report the name of the >>> file >>> being *left*, rather than the one being entered?) > > I realize now that I was wrong here: "new_file" refers to the filename > given in the linemarker directive, which, depending on the (optional) > flag could be a directive to enter or leave a linemap. > > Sorry about that; you may want to disregard my earlier worries. No, I think you were mostly on point: new_file is the one in the #line directive, which AFAICT is the file being reentered. so the wording is in fact misleading. Including a file called "t.h" from "v.c" produces this pattern: # 1 "t.h" 1 int t; # 2 "v.c" 2 > I was thinking of something like the attached, which makes things more > explicit; I felt the first dg-error in your patch was a little too > concise. No problem, but I do think we want to change the wording of the message to something like "file %s unexpectedly reentered". > I'm very familiar with the code for tracking ranges and issuing > diagnostics, but less familiar with other parts of libcpp, so I'm not > sure I'm fully qualified to approve the patch. That said, the patch > looks sane, mostly... The remaining thing I have a concern about > relates to the other question I had, which I don't think you addressed: > is it possible to construct a testcase that triggers the logic in the > non-MAIN_FILE_P clause? Are you thinking of anything more complex than the following? # 1 "v.c" # 1 "t.h" 1 # 1 "t2.h" 1 int t; # 2 "t.h" 2 # 2 "v.c" 2 Change any of the filenames of the "^#.*2$" lines and you'll see error messages. For example, changing "t.h" to "x.h" in the second to last line: In file included from t.h:1:0, from v.c:1: t2.h:2:13: error: file "x.h" left but not entered t2.h:3:12: error: file "v.c" left but not entered Of course any such error is likely to have a large number of follow-on errors. Not sure how to avoid this given that the input file most likely has a completely messed up structure. > (How much existing test coverage do we have for line-markers? I found > about 15 existing testcases that use them in a crude search with grep, > but these are all apparently only incidentally as part of testing other > functionality; is it worth me adding some more general test coverage > for this?) It might be worthwhile but I'm not planning to for this issue. Bernd