From patchwork Mon Oct 8 10:03:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [C++,Patch/RFC] PR 54194 Date: Mon, 08 Oct 2012 00:03:35 -0000 From: Paolo Carlini X-Patchwork-Id: 189969 Message-Id: <5072A4F7.6020108@oracle.com> To: "gcc-patches@gcc.gnu.org" Cc: Jason Merrill 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<