From patchwork Mon Oct 8 10:03:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paolo Carlini X-Patchwork-Id: 189969 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 82F7C2C0372 for ; Mon, 8 Oct 2012 21:03:58 +1100 (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=1350295438; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Message-ID:Date:From:User-Agent:MIME-Version: To:CC:Subject:Content-Type:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=gmgIrWJNrm4bSOzQfDSlg8+HKE0=; b=kDNNumawlXsmcu/ N810614Bogp+9s4HO6/u7frtuDxNIw+IY3TdpP4Abi/LP/g8rWvQw7y2+TaADsyq dpRRCO3RuiF9mpXPsFODx/xEA45tiADbiYxsRZdbu/LyXmspKHkE+yP/vI5Dvnr2 XcKiv4ZPbropdoSNeUJEccJxeqyg= 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:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=u7fvm/hpjB1Pq1hUmvTkRa/tZf9Hwf+cgeXQTfJV9E2mTEjeaaoxjXuZcf/Qwi +w92Cliq+sOf9Z1crA3LpjfNWyOscucHOE8mjMfrNJM7m4kXnVTH9yYEdhA3zuXr afPWEd78U+b5zvMtp9kvFS07LMCRiSELM8W+5TVTmDXNs=; Received: (qmail 6528 invoked by alias); 8 Oct 2012 10:03:50 -0000 Received: (qmail 6511 invoked by uid 22791); 8 Oct 2012 10:03:48 -0000 X-SWARE-Spam-Status: No, hits=-7.9 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_NO, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from acsinet15.oracle.com (HELO acsinet15.oracle.com) (141.146.126.227) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 08 Oct 2012 10:03:41 +0000 Received: from acsinet21.oracle.com (acsinet21.oracle.com [141.146.126.237]) by acsinet15.oracle.com (Sentrion-MTA-4.2.2/Sentrion-MTA-4.2.2) with ESMTP id q98A3cXW007758 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 8 Oct 2012 10:03:39 GMT Received: from acsmt357.oracle.com (acsmt357.oracle.com [141.146.40.157]) by acsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id q98A3caf029885 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 8 Oct 2012 10:03:38 GMT Received: from abhmt101.oracle.com (abhmt101.oracle.com [141.146.116.53]) by acsmt357.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id q98A3bnm014766; Mon, 8 Oct 2012 05:03:37 -0500 Received: from [192.168.1.4] (/79.43.235.228) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 08 Oct 2012 03:03:37 -0700 Message-ID: <5072A4F7.6020108@oracle.com> Date: Mon, 08 Oct 2012 12:03:35 +0200 From: Paolo Carlini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120825 Thunderbird/15.0 MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" CC: Jason Merrill Subject: [C++ Patch/RFC] PR 54194 X-IsSubscribed: yes 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 Hi, in this PR submitter points out that in the -Wparentheses warning, for, eg, char in[4]={0}, out[6]; out[1] = in[1] & 0x0F | ((in[3] & 0x3C) << 2); warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses] the caret points to end of the expression, ie the final closing parenthesis, which is rather misleading, because the problem is actually in the first operand of '|'. Ideally I guess one would like to somehow point to that first operand, but our infrastructure (shared with the C front-end, at the moment) isn't really ready to do that, and probably we would like to use a range (more than a caret) below the whole first operand (the problem isn't really with & per se). Considering also what we are already doing elsewhere, it seems to me that a straightforward and good improvement is obtained by passing to warn_about_parentheses the location of the outer operand (together with its code), as per the attached patchlet: then in the example the caret points to the actual '|' operator mentioned in the error message. Post 4.8.0 we can imagine further improvements... What do you think? Thanks, Paolo. /////////////////////////// Index: cp/typeck.c =================================================================== --- cp/typeck.c (revision 192130) +++ cp/typeck.c (working copy) @@ -3630,7 +3630,8 @@ build_x_binary_op (location_t loc, enum tree_code && !error_operand_p (arg2) && (code != LSHIFT_EXPR || !CLASS_TYPE_P (TREE_TYPE (arg1)))) - warn_about_parentheses (code, arg1_code, orig_arg1, arg2_code, orig_arg2); + warn_about_parentheses (loc, code, arg1_code, orig_arg1, + arg2_code, orig_arg2); if (processing_template_decl && expr != error_mark_node) return build_min_non_dep (code, expr, orig_arg1, orig_arg2); Index: c-family/c-common.c =================================================================== --- c-family/c-common.c (revision 192130) +++ c-family/c-common.c (working copy) @@ -10428,7 +10428,7 @@ warn_array_subscript_with_type_char (tree index) was enclosed in parentheses. */ void -warn_about_parentheses (enum tree_code code, +warn_about_parentheses (location_t loc, enum tree_code code, enum tree_code code_left, tree arg_left, enum tree_code code_right, tree arg_right) { @@ -10449,26 +10449,26 @@ void { case LSHIFT_EXPR: if (code_left == PLUS_EXPR || code_right == PLUS_EXPR) - warning (OPT_Wparentheses, - "suggest parentheses around %<+%> inside %<<<%>"); + warning_at (loc, OPT_Wparentheses, + "suggest parentheses around %<+%> inside %<<<%>"); else if (code_left == MINUS_EXPR || code_right == MINUS_EXPR) - warning (OPT_Wparentheses, - "suggest parentheses around %<-%> inside %<<<%>"); + warning_at (loc, OPT_Wparentheses, + "suggest parentheses around %<-%> inside %<<<%>"); return; case RSHIFT_EXPR: if (code_left == PLUS_EXPR || code_right == PLUS_EXPR) - warning (OPT_Wparentheses, - "suggest parentheses around %<+%> inside %<>>%>"); + warning_at (loc, OPT_Wparentheses, + "suggest parentheses around %<+%> inside %<>>%>"); else if (code_left == MINUS_EXPR || code_right == MINUS_EXPR) - warning (OPT_Wparentheses, - "suggest parentheses around %<-%> inside %<>>%>"); + warning_at (loc, OPT_Wparentheses, + "suggest parentheses around %<-%> inside %<>>%>"); return; case TRUTH_ORIF_EXPR: if (code_left == TRUTH_ANDIF_EXPR || code_right == TRUTH_ANDIF_EXPR) - warning (OPT_Wparentheses, - "suggest parentheses around %<&&%> within %<||%>"); + warning_at (loc, OPT_Wparentheses, + "suggest parentheses around %<&&%> within %<||%>"); return; case BIT_IOR_EXPR: @@ -10476,18 +10476,19 @@ void || code_left == PLUS_EXPR || code_left == MINUS_EXPR || code_right == BIT_AND_EXPR || code_right == BIT_XOR_EXPR || code_right == PLUS_EXPR || code_right == MINUS_EXPR) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around arithmetic in operand of %<|%>"); /* Check cases like x|y==z */ else if (TREE_CODE_CLASS (code_left) == tcc_comparison || TREE_CODE_CLASS (code_right) == tcc_comparison) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around comparison in operand of %<|%>"); /* Check cases like !x | y */ else if (code_left == TRUTH_NOT_EXPR && !APPEARS_TO_BE_BOOLEAN_EXPR_P (code_right, arg_right)) - warning (OPT_Wparentheses, "suggest parentheses around operand of " - "% or change %<|%> to %<||%> or % to %<~%>"); + warning_at (loc, OPT_Wparentheses, + "suggest parentheses around operand of " + "% or change %<|%> to %<||%> or % to %<~%>"); return; case BIT_XOR_EXPR: @@ -10495,44 +10496,45 @@ void || code_left == PLUS_EXPR || code_left == MINUS_EXPR || code_right == BIT_AND_EXPR || code_right == PLUS_EXPR || code_right == MINUS_EXPR) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around arithmetic in operand of %<^%>"); /* Check cases like x^y==z */ else if (TREE_CODE_CLASS (code_left) == tcc_comparison || TREE_CODE_CLASS (code_right) == tcc_comparison) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around comparison in operand of %<^%>"); return; case BIT_AND_EXPR: if (code_left == PLUS_EXPR || code_right == PLUS_EXPR) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around %<+%> in operand of %<&%>"); else if (code_left == MINUS_EXPR || code_right == MINUS_EXPR) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around %<-%> in operand of %<&%>"); /* Check cases like x&y==z */ else if (TREE_CODE_CLASS (code_left) == tcc_comparison || TREE_CODE_CLASS (code_right) == tcc_comparison) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around comparison in operand of %<&%>"); /* Check cases like !x & y */ else if (code_left == TRUTH_NOT_EXPR && !APPEARS_TO_BE_BOOLEAN_EXPR_P (code_right, arg_right)) - warning (OPT_Wparentheses, "suggest parentheses around operand of " - "% or change %<&%> to %<&&%> or % to %<~%>"); + warning_at (loc, OPT_Wparentheses, + "suggest parentheses around operand of " + "% or change %<&%> to %<&&%> or % to %<~%>"); return; case EQ_EXPR: if (TREE_CODE_CLASS (code_left) == tcc_comparison || TREE_CODE_CLASS (code_right) == tcc_comparison) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around comparison in operand of %<==%>"); return; case NE_EXPR: if (TREE_CODE_CLASS (code_left) == tcc_comparison || TREE_CODE_CLASS (code_right) == tcc_comparison) - warning (OPT_Wparentheses, + warning_at (loc, OPT_Wparentheses, "suggest parentheses around comparison in operand of %"); return; @@ -10544,8 +10546,9 @@ void || (TREE_CODE_CLASS (code_right) == tcc_comparison && code_right != NE_EXPR && code_right != EQ_EXPR && INTEGRAL_TYPE_P (TREE_TYPE (arg_right))))) - warning (OPT_Wparentheses, "comparisons like % do not " - "have their mathematical meaning"); + warning_at (loc, OPT_Wparentheses, + "comparisons like % do not " + "have their mathematical meaning"); return; } #undef NOT_A_BOOLEAN_EXPR_P Index: c-family/c-common.h =================================================================== --- c-family/c-common.h (revision 192130) +++ c-family/c-common.h (working copy) @@ -988,7 +988,8 @@ extern tree builtin_type_for_size (int, bool); extern void c_common_mark_addressable_vec (tree); extern void warn_array_subscript_with_type_char (tree); -extern void warn_about_parentheses (enum tree_code, +extern void warn_about_parentheses (location_t, + enum tree_code, enum tree_code, tree, enum tree_code, tree); extern void warn_for_unused_label (tree label); Index: c/c-typeck.c =================================================================== --- c/c-typeck.c (revision 192130) +++ c/c-typeck.c (working copy) @@ -3261,7 +3261,8 @@ parser_build_binary_op (location_t location, enum /* Check for cases such as x+y<