From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103900 invoked by alias); 8 Feb 2016 16:14:42 -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 103887 invoked by uid 89); 8 Feb 2016 16:14:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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, 08 Feb 2016 16:14:40 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 7D3B6C0C2376 for ; Mon, 8 Feb 2016 16:14:39 +0000 (UTC) Received: from slagheap.utah.redhat.com (ovpn-113-84.phx2.redhat.com [10.3.113.84]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u18GEcWZ016265; Mon, 8 Feb 2016 11:14:39 -0500 Subject: Re: [PATCH] PR preprocessor/69664: fix rich_location::override_column To: David Malcolm , gcc-patches@gcc.gnu.org References: <1454626307-45040-1-git-send-email-dmalcolm@redhat.com> From: Jeff Law Message-ID: <56B8BEEE.1000202@redhat.com> Date: Mon, 08 Feb 2016 16:14: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: <1454626307-45040-1-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00570.txt.bz2 On 02/04/2016 03:51 PM, David Malcolm wrote: > In gcc 5 and earlier, struct diagnostic_info had a field: > unsigned int override_column; > which could be set by the macro: > diagnostic_override_column > > This was only used by the frontends' callbacks for handling errors > from libcpp: c_cpp_error for the c-family, and cb_cpp_error for Fortran: > if (column_override) > diagnostic_override_column (&diagnostic, column_override); > based on a column_override value passed by libcpp to the callback. > > I removed the field when introducing rich_location (in r229884), > instead adding a rich_location::override_column method, called from > libcpp immediately before calling the frontend's error-handling > callback (the callback takes a rich_location *). > > I got the implementation of rich_location::override_column wrong > (sorry), and PR preprocessor/69664 is a symptom of this. > > Specifically, I was only overriding the column within > m_expanded_location, which affects some parts of > diagnostic-show-locus.c, but I was not overriding the column within > the various expanded_location instances within the rich_location's > m_ranges[0]. > > Hence the wrong column information is printed for diagnostics > emitted by libcpp where the column is overridden. > This happens for any of the "_with_line" diagnostics calls in > libcpp that are passed a non-zero column override. > I believe these are all confined to libcpp/lex.c. > > The attached patch fixes this, by ensuring that all relevant > columns are updated by rich_location::override_column, and > also adding the missing conditional to only call it for non-zero > column_override values (this time in libcpp before calling the > error-handling callback, rather than from within the error-handling > callbacks). > > It also adds expected column numbers to some pre-existing tests, > giving us test coverage for this. > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu; > OK for trunk? > > gcc/testsuite/ChangeLog: > PR preprocessor/69664 > * gcc.dg/cpp/trad/comment-2.c: Add expected column number. > * gcc.dg/cpp/warn-comments.c: Likewise. > > libcpp/ChangeLog: > PR preprocessor/69664 > * errors.c (cpp_diagnostic_with_line): Only call > rich_location::override_column if the column is non-zero. > * line-map.c (rich_location::override_column): Update columns > within m_ranges[0]. Add assertions to verify that doing so is > sane. OK Thanks, Jeff