From patchwork Tue Apr 10 19:55:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 151688 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]) by ozlabs.org (Postfix) with SMTP id 4497BB6FF7 for ; Wed, 11 Apr 2012 05:56:14 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1334692575; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:From:To:Cc:Subject:References:Date:In-Reply-To: Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=LXs4gOAyuP6oNNSLeV1s9+o65xk=; b=nmQM9W8Svc9BKJLcycGHNd3LoWTVUvI1Dt+2w/rxE0vPly5fVIUY51psOwNbjN jMHjYLUVMM/uLAV8IiIMp/9GMfVddGvzEaGwEL60iUkYgqEBU8se3N2amk+JHYRa Q6ZPjzPF4h6Jkpq8tD7wpS4jlPq9X7sP5FxJB3S+AOX4k= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:From:To:Cc:Subject:References:X-URL:Date:In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=KBd2Pt0QA3S4vw3u0IT0P2Xw/wlxZPwMhhRf/Z38hd9fXoRfeEhO71Hom9UK+G IyEHBzFZYB+QLLQDQ/FvVrlQf8icIm+VZWJuv673CVx3JvT0MCF+lVTcOw1uP7JC XPB+KIwdS4OPgttcWbvaLnh9qxGoL9ECikCDjqikSn/lo=; Received: (qmail 17559 invoked by alias); 10 Apr 2012 19:56:09 -0000 Received: (qmail 17547 invoked by uid 22791); 10 Apr 2012 19:56:06 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_HELO_PASS, TW_LR, TW_VF, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 Apr 2012 19:55:47 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q3AJtklM013070 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 10 Apr 2012 15:55:46 -0400 Received: from localhost (ovpn-116-19.ams2.redhat.com [10.36.116.19]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q3AJtjJi012113 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 10 Apr 2012 15:55:46 -0400 Received: by localhost (Postfix, from userid 500) id BE82129C046; Tue, 10 Apr 2012 21:55:44 +0200 (CEST) From: Dodji Seketeli To: GCC Patches Cc: Tom Tromey , Jason Merrill , Gabriel Dos Reis Subject: [PATCH 06/11] Strip "" loc from displayed expansion context References: X-URL: http://www.redhat.com Date: Tue, 10 Apr 2012 21:55:44 +0200 In-Reply-To: (Dodji Seketeli's message of "Tue, 10 Apr 2012 16:53:12 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 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 Now that diagnostics for tokens coming from macro expansions point to the spelling location of the relevant token (and then displays the context of the expansion), some ugly (not so seldom) corner cases can happen. When the relevant token is a built-in token (which means the location of that token is BUILTINS_LOCATION) the location prefix displayed to the user in the diagnostic line is the ":0:0" string. For instance: :0:0: warning: conversion to 'float' alters 'int' constant value For the user, I think this is surprising and useless. A more user-friendly approach would be to refer to the first location that (in the reported macro expansion context) is for a location in real source code, like what is shown in the new test case gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C accompanying this patch. To do this, I am making the line-map module provide a new linemap_unwind_to_first_non_reserved_loc function that resolves a virtual location to the first spelling location that is in real source code. I am then using that facility in the diagnostics printing module and in the macro unwinder to avoid printing diagnostics lines that refer to the locations for built-ins or more generally for reserved locations. Note that when I start the dance of skipping a built-in location I also skip locations that are in system headers, because it turned out that a lot of those built-ins are actually used in system headers (e.g, "#define INT_MAX __INT_MAX__" where __INT_MAX__ is a built-in). Besides the user-friendliness gain, this patch allows a number of regression tests to PASS unchanged with and without -ftrack-macro-expansion. Tested and bootstrapped on x86_64-unknown-linux-gnu against trunk. Note that the bootstrap with -ftrack-macro-expansion exhibits other separate issues that are addressed in subsequent patches. This patch just fixes one class of problems. The patch does pass bootstrap with -ftrack-macro-expansion turned off, though. libcpp/ * include/line-map.h (linemap_unwind_toward_expansion): Fix typo in comment. (linemap_unwind_to_first_non_reserved_loc): Declare new function. * line-map.c (linemap_unwind_to_first_non_reserved_loc): Define new function. gcc/ * input.c (expand_location_1): When expanding to spelling location in a context of a macro expansion, skip reserved system header locations. Update comments. * tree-diagnostic.c (maybe_unwind_expanded_macro_loc): Likewise. gcc/testsuite/ * g++.dg/warn/Wconversion-real-integer2.C: New test. * g++.dg/warn/Wconversion-real-integer-3.C: Likewise. * g++.dg/warn/conversion-real-integer-3.h: New header used by the new test above. --- gcc/input.c | 38 ++++++++++++--- .../g++.dg/warn/Wconversion-real-integer-3.C | 20 ++++++++ .../g++.dg/warn/Wconversion-real-integer2.C | 33 +++++++++++++ .../g++.dg/warn/conversion-real-integer-3.h | 3 + gcc/tree-diagnostic.c | 12 +++++ libcpp/include/line-map.h | 20 ++++++++- libcpp/line-map.c | 49 ++++++++++++++++++++ 7 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C create mode 100644 gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h diff --git a/gcc/input.c b/gcc/input.c index dcd348b..8edf05b 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -35,7 +35,14 @@ struct line_maps *line_table; location is set to the string "". If EXPANSION_POINT_P is TRUE and LOC is virtual, then it is resolved to the expansion point of the involved macro. Otherwise, it is resolved to the - spelling location of the token. */ + spelling location of the token. + + When resolving to the spelling location of the token, if the + resulting location is for a built-in location (that is, it has no + associated line/column) in the context of a macro expansion, the + returned location is the first one (while unwinding the macro + location towards its expansion point) that is in real source + code. */ static expanded_location expand_location_1 (source_location loc, @@ -43,12 +50,29 @@ expand_location_1 (source_location loc, { expanded_location xloc; const struct line_map *map; - - loc = linemap_resolve_location (line_table, loc, - expansion_point_p - ? LRK_MACRO_EXPANSION_POINT - : LRK_SPELLING_LOCATION, &map); - xloc = linemap_expand_location (line_table, map, loc); + enum location_resolution_kind lrk = LRK_MACRO_EXPANSION_POINT; + + memset (&xloc, 0, sizeof (xloc)); + + if (loc >= RESERVED_LOCATION_COUNT) + { + if (!expansion_point_p) + { + /* We want to resolve LOC to its spelling location. + + But if that spelling location is a reserved location that + appears in the context of a macro expansion (like for a + location for a built-in token), let's consider the first + location (toward the expansion point) that is not reserved; + that is, the first location that is in real source code. */ + loc = linemap_unwind_to_first_non_reserved_loc (line_table, + loc, &map); + lrk = LRK_SPELLING_LOCATION; + } + loc = linemap_resolve_location (line_table, loc, + lrk, &map); + xloc = linemap_expand_location (line_table, map, loc); + } if (loc <= BUILTINS_LOCATION) xloc.file = loc == UNKNOWN_LOCATION ? NULL : _(""); diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C new file mode 100644 index 0000000..a4df010 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer-3.C @@ -0,0 +1,20 @@ +// { dg-do compile } +// { dg-options "-Wconversion -ftrack-macro-expansion=2" } +// { dg-require-effective-target int32plus } + +#include "conversion-real-integer-3.h" + +float vfloat; + +void h (void) +{ + // We want to trigger an error on the token INT_MAX below, that is + // a macro that expands to the built-in __INT_MAX__. Furthermore, + // INT_MAX is defined inside a system header. + // + // The behaviour we want is that the diagnostic should point to + // the locus that inside the source code here, at the relevant + // line below, even with -ftrack-macro-expansion. We don't want + // it to point to the any locus that is inside the system header. + vfloat = INT_MAX; // { dg-warning "conversion to .float. alters .int. constant value" } +} diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C new file mode 100644 index 0000000..29130f1 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C @@ -0,0 +1,33 @@ +/* { dg-do compile } +/* { dg-options "-Wconversion -ftrack-macro-expansion=2" } */ +/* { dg-require-effective-target int32plus } */ + +// Before the fix that came with this test, we'd output an error for +// the __INT_MAX__ token. That token has a BUILTINS_LOCATION +// location, so the the location prefix in the warning message would +// be: +// :0:0: warning: conversion to 'float' alters 'int' constant value +// +// Note the useless and confusing :0:0 prefix. This is +// because '__INT_MAX__' being an internal macro token, it has a +// BUILTINS_LOCATION location. +// +// In this case, we want the error message to refer to the first +// location (in the macro expansion context) that is not a location +// for a built-in token. That location would be the one for where (in +// real source code) the __INT_MAX__ macro has been expanded. +// +// That would be something like: +// +// gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C:21:17: warning: conversion to 'float' alters 'int' constant value +// +// That is more useful. + +#define INT_MAX __INT_MAX__ // { dg-warning "conversion to .float. alters .int. constant value" } + +float vfloat; + +void h (void) +{ + vfloat = INT_MAX; // { dg-message "expanded from here" } +} diff --git a/gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h b/gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h new file mode 100644 index 0000000..6ed5b2c --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/conversion-real-integer-3.h @@ -0,0 +1,3 @@ +#pragma GCC system_header + +#define INT_MAX __INT_MAX__ diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c index b4b60dc..064fcec 100644 --- a/gcc/tree-diagnostic.c +++ b/gcc/tree-diagnostic.c @@ -168,6 +168,18 @@ maybe_unwind_expanded_macro_loc (diagnostic_context *context, linemap_resolve_location (line_table, iter->where, LRK_MACRO_DEFINITION_LOCATION, NULL); + /* Don't print trace for locations that are reserved or from + within a system header. */ + { + const struct line_map *m = NULL; + source_location l = linemap_resolve_location (line_table, resolved_def_loc, + LRK_SPELLING_LOCATION, + &m); + if (l < RESERVED_LOCATION_COUNT + || LINEMAP_SYSP (m)) + continue; + } + /* Resolve the location of the expansion point of the macro which expansion gave the token represented by def_loc. This is the locus 2/ of the earlier comment. */ diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h index 4e30742..8496aa1 100644 --- a/libcpp/include/line-map.h +++ b/libcpp/include/line-map.h @@ -666,12 +666,30 @@ source_location linemap_resolve_location (struct line_maps *, location L of the point where M got expanded. If L is a spelling location inside a macro expansion M', then this function returns the point where M' was expanded. LOC_MAP is an output parameter. - When non-NULL, *LOC_MAP is set the the map of the returned + When non-NULL, *LOC_MAP is set to the map of the returned location. */ source_location linemap_unwind_toward_expansion (struct line_maps *, source_location loc, const struct line_map **loc_map); +/* If LOC is the virtual location of a token coming from the expansion + of a macro M and if its spelling location is reserved (e.g, a + location for a built-in token), then this function unwinds (using + linemap_unwind_toward_expansion) the location until a location that + is not reserved and is not in a system header is reached. In other + words, this unwinds the reserved location until a location that is + in real source code is reached. + + Otherwise, if the spelling location for LOC is not reserved or if + LOC doesn't come from the expansion of a macro, the function + returns LOC as is and *MAP is not touched. + + *MAP is set to the map of the returned location if the later is + different from LOC. */ +source_location linemap_unwind_to_first_non_reserved_loc (struct line_maps *, + source_location loc, + const struct line_map **map); + /* Expand source code location LOC and return a user readable source code location. LOC must be a spelling (non-virtual) location. If it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source diff --git a/libcpp/line-map.c b/libcpp/line-map.c index d7752bb..171aa0f 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -1111,6 +1111,55 @@ linemap_unwind_toward_expansion (struct line_maps *set, return resolved_location; } +/* If LOC is the virtual location of a token coming from the expansion + of a macro M and if its spelling location is reserved (e.g, a + location for a built-in token), then this function unwinds (using + linemap_unwind_toward_expansion) the location until a location that + is not reserved and is not in a sytem header is reached. In other + words, this unwinds the reserved location until a location that is + in real source code is reached. + + Otherwise, if the spelling location for LOC is not reserved or if + LOC doesn't come from the expansion of a macro, the function + returns LOC as is and *MAP is not touched. + + *MAP is set to the map of the returned location if the later is + different from LOC. */ +source_location +linemap_unwind_to_first_non_reserved_loc (struct line_maps *set, + source_location loc, + const struct line_map **map) +{ + source_location resolved_loc; + const struct line_map *map0 = NULL, *map1 = NULL; + + map0 = linemap_lookup (set, loc); + if (!linemap_macro_expansion_map_p (map0)) + return loc; + + resolved_loc = linemap_resolve_location (set, loc, + LRK_SPELLING_LOCATION, + &map1); + + if (resolved_loc >= RESERVED_LOCATION_COUNT + && !LINEMAP_SYSP (map1)) + return loc; + + while (linemap_macro_expansion_map_p (map0) + && (resolved_loc < RESERVED_LOCATION_COUNT + || LINEMAP_SYSP (map1))) + { + loc = linemap_unwind_toward_expansion (set, loc, &map0); + resolved_loc = linemap_resolve_location (set, loc, + LRK_SPELLING_LOCATION, + &map1); + } + + if (map != NULL) + *map = map0; + return loc; +} + /* Expand source code location LOC and return a user readable source code location. LOC must be a spelling (non-virtual) location. If it's a location < RESERVED_LOCATION_COUNT a zeroed expanded source