[C++ Patch/RFC] PR 54194

Paolo Carlini paolo.carlini@oracle.com
Mon Oct 8 10:03:00 GMT 2012


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.

///////////////////////////

-------------- next part --------------
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 %<X<=Y<=Z%> do not "
-		 "have their mathematical meaning");
+	warning_at (loc, OPT_Wparentheses,
+		    "comparisons like %<X<=Y<=Z%> 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<<z which users are likely
      to misinterpret.  */
   if (warn_parentheses)
-    warn_about_parentheses (code, code1, arg1.value, code2, arg2.value);
+    warn_about_parentheses (input_location, code,
+			    code1, arg1.value, code2, arg2.value);
 
   if (warn_logical_op)
     warn_logical_operator (input_location, code, TREE_TYPE (result.value),


More information about the Gcc-patches mailing list