From patchwork Wed Apr 25 14:49:34 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 154962 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 74645B6F62 for ; Thu, 26 Apr 2012 00:50:18 +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=1335970218; 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=fOhpqXcY1bx2QztrNEJRDaFt0t4=; b=olELAQKX5MI+lBvG1MOEE5zMnwL+XxYFetBqkcoz/RijDaYLJGwREqyfFnPgEI jSuk9UHNl/ybxnWpr5YnNsQ0uruvE/g5TyNblDCR6+Fd7bqqh8f7zNsb4+0O4vis 8sBE7qNdWuhknoi3HbLt24pzmTayJfouJVytidkXgzor8= 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=DXIvT/GEKzMNB3AtOhOQqhGcVkGqof0b7JUQn4cH7hv0rXUTSCI8Uqte63tyY0 ZnKL1mt13J4ghq+w8gVye6MioFLYL6ByYoSjvxmkGbYIuktv60DKFr2ByaBncv+M kkyTInKAEoEp7iZZAYZPCSxJS0Z41X6HnkrIt3MYO31Zk=; Received: (qmail 10033 invoked by alias); 25 Apr 2012 14:50:10 -0000 Received: (qmail 9913 invoked by uid 22791); 25 Apr 2012 14:50:05 -0000 X-SWARE-Spam-Status: No, hits=-5.9 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_HELO_PASS, 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; Wed, 25 Apr 2012 14:49:38 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q3PEnbmG018705 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 25 Apr 2012 10:49:37 -0400 Received: from localhost (ovpn-116-18.ams2.redhat.com [10.36.116.18]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q3PEnZL1029489 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 25 Apr 2012 10:49:36 -0400 Received: by localhost (Postfix, from userid 500) id EA48C29C046; Wed, 25 Apr 2012 16:49:34 +0200 (CEST) From: Dodji Seketeli To: Gabriel Dos Reis Cc: GCC Patches , Tom Tromey , Jason Merrill Subject: Re: [PATCH 08/11] Make conversion warnings work on NULL with -ftrack-macro-expansion References: X-URL: http://www.redhat.com Date: Wed, 25 Apr 2012 16:49:34 +0200 In-Reply-To: (Gabriel Dos Reis's message of "Wed, 25 Apr 2012 09:06:59 -0500") 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 Gabriel Dos Reis writes: > Thanks. But a point of the suggestion was that we won't need the > same comment/explanation duplicated over at least 3 places. Just > one. All those three places deal exactly with one instance: null > pointer constant. That deserves a function in and of itself, which > is documented by the duplicated comments. > Please make that change. Everything else is OK. thanks. I am sorry for the round trips. Please find below a patch udpated accordingly. I am bootstrapping the whole patch set, but the impacted files of this patch have built fine so far. Thanks. gcc/ * input.h (expansion_point_location_if_in_system_header): Declare new function. * input.c (expansion_point_location_if_in_system_header): Define it. gcc/cp/ * call.c (conversion_null_warnings): Use the new expansion_point_location_if_in_system_header. * cvt.c (build_expr_type_conversion): Likewise. * typeck.c (cp_build_binary_op): Likewise. gcc/testsuite/ * g++.dg/warn/Wconversion-null-2.C: Add testing for __null, alongside the previous testing for NULL. --- gcc/cp/call.c | 7 ++++- gcc/cp/cvt.c | 9 +++++- gcc/cp/typeck.c | 9 ++++-- gcc/input.c | 20 +++++++++++++++ gcc/input.h | 1 + gcc/testsuite/g++.dg/warn/Wconversion-null-2.C | 31 +++++++++++++++++++++++- 6 files changed, 69 insertions(+), 8 deletions(-) diff --git a/gcc/cp/call.c b/gcc/cp/call.c index f9a7f08..85e45c2 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -5598,12 +5598,15 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum) if (expr == null_node && TREE_CODE (totype) != BOOLEAN_TYPE && ARITHMETIC_TYPE_P (totype)) { + source_location loc = + expansion_point_location_if_in_system_header (input_location); + if (fn) - warning_at (input_location, OPT_Wconversion_null, + warning_at (loc, OPT_Wconversion_null, "passing NULL to non-pointer argument %P of %qD", argnum, fn); else - warning_at (input_location, OPT_Wconversion_null, + warning_at (loc, OPT_Wconversion_null, "converting to non-pointer type %qT from NULL", totype); } diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index 3dab372..49ba38a 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -1472,8 +1472,13 @@ build_expr_type_conversion (int desires, tree expr, bool complain) if (expr == null_node && (desires & WANT_INT) && !(desires & WANT_NULL)) - warning_at (input_location, OPT_Wconversion_null, - "converting NULL to non-pointer type"); + { + source_location loc = + expansion_point_location_if_in_system_header (input_location); + + warning_at (loc, OPT_Wconversion_null, + "converting NULL to non-pointer type"); + } basetype = TREE_TYPE (expr); diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index fb2f1bc..52d264b 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3838,9 +3838,12 @@ cp_build_binary_op (location_t location, || (!null_ptr_cst_p (orig_op1) && !TYPE_PTR_P (type1) && !TYPE_PTR_TO_MEMBER_P (type1))) && (complain & tf_warning)) - /* Some sort of arithmetic operation involving NULL was - performed. */ - warning (OPT_Wpointer_arith, "NULL used in arithmetic"); + { + source_location loc = + expansion_point_location_if_in_system_header (input_location); + + warning_at (loc, OPT_Wpointer_arith, "NULL used in arithmetic"); + } switch (code) { diff --git a/gcc/input.c b/gcc/input.c index 260be7e..5f14489 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -162,6 +162,26 @@ expand_location_to_spelling_point (source_location loc) return expand_location_1 (loc, /*expansion_piont_p=*/false); } +/* If LOCATION is in a sytem header and if it's a virtual location for + a token coming from the expansion of a macro M, unwind it to the + location of the expansion point of M. Otherwise, just return + LOCATION. + + This is used for instance when we want to emit diagnostics about a + token that is located in a macro that is itself defined in a system + header -- e.g for the NULL macro. In that case, if LOCATION is + passed to diagnostics emitting functions like warning_at as is, no + diagnostic won't be emitted. */ + +source_location +expansion_point_location_if_in_system_header (source_location location) +{ + if (in_system_header_at (location)) + location = linemap_resolve_location (line_table, location, + LRK_MACRO_EXPANSION_POINT, + NULL); + return location; +} #define ONE_K 1024 #define ONE_M (ONE_K * ONE_K) diff --git a/gcc/input.h b/gcc/input.h index f755cdf..f588838 100644 --- a/gcc/input.h +++ b/gcc/input.h @@ -40,6 +40,7 @@ extern char builtins_location_check[(BUILTINS_LOCATION extern expanded_location expand_location (source_location); extern const char * location_get_source_line(expanded_location xloc); extern expanded_location expand_location_to_spelling_point (source_location); +extern source_location expansion_point_location_if_in_system_header (source_location); /* Historically GCC used location_t, while cpp used source_location. This could be removed but it hardly seems worth the effort. */ diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C b/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C index dd498c1..a71551f 100644 --- a/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C +++ b/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C @@ -25,7 +25,7 @@ void l(long) {} template <> void l(long long) {} -int main() +void warn_for_NULL() { int i = NULL; // { dg-warning "" } converting NULL to non-pointer type float z = NULL; // { dg-warning "" } converting NULL to non-pointer type @@ -47,3 +47,32 @@ int main() l(NULL); // No warning: NULL is used to implicitly instantiate the template NULL && NULL; // No warning: converting NULL to bool is OK } + +int warn_for___null() +{ + int i = __null; // { dg-warning "" } converting __null to non-pointer type + float z = __null; // { dg-warning "" } converting __null to non-pointer type + int a[2]; + + i != __null; // { dg-warning "" } __null used in arithmetic + __null != z; // { dg-warning "" } __null used in arithmetic + k != __null; // No warning: decay conversion + __null != a; // Likewise. + -__null; // { dg-warning "" } converting __null to non-pointer type + +__null; // { dg-warning "" } converting __null to non-pointer type + ~__null; // { dg-warning "" } converting __null to non-pointer type + a[__null] = 3; // { dg-warning "" } converting __null to non-pointer-type + i = __null; // { dg-warning "" } converting __null to non-pointer type + z = __null; // { dg-warning "" } converting __null to non-pointer type + k(__null); // { dg-warning "" } converting __null to int + g(__null); // { dg-warning "" } converting __null to int + h<__null>(); // No warning: __null bound to integer template parameter + l(__null); // No warning: __null is used to implicitly instantiate the template + __null && __null; // No warning: converting NULL to bool is OK +} + +int main() +{ + warn_for_NULL(); + warn_for___null(); +}