From patchwork Tue Dec 29 20:53:52 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mike Stump X-Patchwork-Id: 561620 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 6F783140C1C for ; Wed, 30 Dec 2015 07:55:57 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=ZrXXzYqw; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; q=dns; s= default; b=k27uvMtv7jtwHXqEjvlpp6Nn88cxZdWmzHfZ0X+DMs83OeskwwCIq fVYasUa3gX5r8w//aI4nK4ORSZ/5YYbvECXhHQnzbXuBvjHsp8BvZFeGk7vpVcCp +xYYejGbxF2DBOdOys4gOJesHn3zjmonkbMAdSO2v2YQVh/NEATQQA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=default; bh=c+DDc8qXTJdZJKTxAK4UWgo0+Kc=; b=ZrXXzYqwuNXL9enTnIw4vQMgs0Qs +g0ed51UKfUKeFNl9tKZC9JOySGW356qx7HDNajig3pW8p4ZlDMgC4GkwlxRi9bZ jhhmRSOX5IWDRBkL7jRcUrxkjFr0ammsmNB90WxACHmhC+PJhiJYWtyzu7yUGDUj hDChRzJTTzc5U70= Received: (qmail 21009 invoked by alias); 29 Dec 2015 20:55:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 20999 invoked by uid 89); 29 Dec 2015 20:55:51 -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, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=classes, validate, expansions, funnier X-HELO: resqmta-po-07v.sys.comcast.net Received: from resqmta-po-07v.sys.comcast.net (HELO resqmta-po-07v.sys.comcast.net) (96.114.154.166) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 29 Dec 2015 20:55:50 +0000 Received: from resomta-po-04v.sys.comcast.net ([96.114.154.228]) by resqmta-po-07v.sys.comcast.net with comcast id zYvo1r0014vw8ds01Yvo7g; Tue, 29 Dec 2015 20:55:48 +0000 Received: from [IPv6:2001:558:6045:a4:40c6:7199:cd03:b02d] ([IPv6:2001:558:6045:a4:40c6:7199:cd03:b02d]) by resomta-po-04v.sys.comcast.net with comcast id zYvm1r00P2ztT3H01Yvndk; Tue, 29 Dec 2015 20:55:48 +0000 Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Subject: Re: [PATCH] v3 of diagnostic_show_locus and rich_location (was Re: [PATCH 2/5] Reimplement diagnostic_show_locus, introducing rich_location classes (v2)) From: Mike Stump In-Reply-To: <1443211881.30732.121.camel@surprise> Date: Tue, 29 Dec 2015 12:53:52 -0800 Cc: GCC Patches Message-Id: <5E862042-E96A-4883-8FCB-0DD3A2C541CC@comcast.net> References: <1442957171-22904-1-git-send-email-dmalcolm@redhat.com> <1442957171-22904-3-git-send-email-dmalcolm@redhat.com> <86h9miswzc.fsf@seketeli.org> <1443211881.30732.121.camel@surprise> To: David Malcolm X-IsSubscribed: yes On Sep 25, 2015, at 1:11 PM, David Malcolm wrote: > +layout::layout (diagnostic_context * context, >> + const diagnostic_info *diagnostic) >> >> [...] >> >> + if (loc_range->m_finish.file != m_exploc.file) >> + continue; >> + if (loc_range->m_show_caret_p) >> + if (loc_range->m_finish.file != m_exploc.file) >> + continue; >> >> I think the second if clause is redundant. > > Good catch; thanks. And one other nit. You don’t validate that the range finishes on or after the start. Later in the code, you require this to be the case: bool layout_range::contains_point (int row, int column) const { gcc_assert (m_start.m_line <= m_finish.m_line); this code cannot tolerate a range with that property. So, either, such a range should never be generated, or, if it is to be generated, at least we should declare it awkward. The below patch declares it to be awkward. Without this, we ice on completely sane and normal code: #define max(i, j) sel((j), (i), (i) < (j)) yu = max(a2, a3); giving a valid warning. In the code, we start on the last line, and finish on the first line. The underlying problem is that we don’t track macro contexts properly. sel is a compiler built-in, so, it might be funnier that just a normal function call. This is from a trunk compiler from 20151201. So, I was wondering if the problem has been fixed, or, if we should put the below in now, or, would you prefer to try and do up the changes to better track macro expansions? diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index 9e51b95..bea8423 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -455,6 +455,9 @@ layout::layout (diagnostic_context * context, if (loc_range->m_show_caret_p) if (loc_range->m_caret.file != m_exploc.file) continue; + /* A range that finishes before it starts is awkward. */ + if (loc_range->m_start.line > loc_range->m_finish.line) + continue; /* Passed all the tests; add the range to m_layout_ranges so that it will be printed. */