Hi again,
On 10/08/2012 11:44 PM, Jason Merrill wrote:
On 10/08/2012 05:31 PM, Paolo Carlini wrote:
So, there is a serious difficulty, I'm afraid: for the example at issue,
EXPR_LOCATION (arg_left) is 0 not any meaningful value. And of course
EXPR_LOC_OR_HERE would not be better in this case, would give
input_location. So, what do you think? Shall we just go with the loc for
now, or you think there is something relatively safe we can try?
Looks like we still need to SET_EXPR_LOCATION in cp_build_binary_op.
It would also be good to have a macro/function like EXPR_LOC_OR_HERE
that lets you provide the location to use if the expression doesn't
have one.
Thus I'm finishing testing the below (C++ is Ok)
It works pretty well, IMHO: for actual expressions we can have the
column number of the operand (which is now fixed up in
cp_parser_binary_expression), otherwise, for eg constants, we fall back
to the column number of the operator. As a bonus, in templates like
Wparentheses-26.C we even get the line number right.
Thanks!
Paolo.
////////////////////////////
2012-10-09 Paolo Carlini <paolo.carl...@oracle.com>
PR c++/54194
* tree.h: Add EXPR_LOC_OR_LOC.
c-family/
2012-10-09 Paolo Carlini <paolo.carl...@oracle.com>
PR c++/54194
* c-common.c (warn_about_parentheses): Add location_t parameter;
use EXPR_LOC_OR_LOC.
* c-common.h: Update declaration.
c/
2012-10-09 Paolo Carlini <paolo.carl...@oracle.com>
PR c++/54194
* c-typeck.c (parser_build_binary_op): Update warn_about_parentheses
call.
/cp
2012-10-09 Paolo Carlini <paolo.carl...@oracle.com>
PR c++/54194
* typeck.c (build_x_binary_op): Update warn_about_parentheses call.
* parser.c (cp_parser_binary_expression): Use SET_EXPR_LOCATION
on current.lhs.
/testsuite
2012-10-09 Paolo Carlini <paolo.carl...@oracle.com>
PR c++/54194
* g++.dg/warn/Wparentheses-26.C: Adjust.
* g++.dg/warn/Wparentheses-27.C: New.
Index: testsuite/g++.dg/warn/Wparentheses-26.C
===================================================================
--- testsuite/g++.dg/warn/Wparentheses-26.C (revision 192224)
+++ testsuite/g++.dg/warn/Wparentheses-26.C (working copy)
@@ -4,23 +4,23 @@
template<int i, int j = ((i + 7) >> 3)> class foo1 { };
typedef foo1<10> bar1;
-template<int i, int j = (i + 7 >> 3)> class foo2 { };
-typedef foo2<10> bar2; // { dg-warning "suggest parentheses around '\\+'" }
+template<int i, int j = (i + 7 >> 3)> class foo2 { }; // { dg-warning
"suggest parentheses around '\\+'" }
+typedef foo2<10> bar2;
template<int i, int j = (100 >> (i + 2))> class foo3 { };
typedef foo3<3> bar3;
-template<int i, int j = (100 >> i + 2)> class foo4 { };
-typedef foo4<3> bar4; // { dg-warning "suggest parentheses around '\\+'" }
+template<int i, int j = (100 >> i + 2)> class foo4 { }; // { dg-warning
"suggest parentheses around '\\+'" }
+typedef foo4<3> bar4;
template<int i, int j = (i + 7) | 3> class foo5 { };
typedef foo5<10> bar5;
-template<int i, int j = i + 7 | 3> class foo6 { };
-typedef foo6<10> bar6; // { dg-warning "suggest parentheses around
arithmetic" }
+template<int i, int j = i + 7 | 3> class foo6 { }; // { dg-warning "suggest
parentheses around arithmetic" }
+typedef foo6<10> bar6;
template<int i, int j = 3 | (i + 7)> class foo7 { };
typedef foo7<10> bar7;
-template<int i, int j = 3 | i + 7> class foo8 { };
-typedef foo8<10> bar8; // { dg-warning "suggest parentheses around
arithmetic" }
+template<int i, int j = 3 | i + 7> class foo8 { }; // { dg-warning "suggest
parentheses around arithmetic" }
+typedef foo8<10> bar8;
Index: testsuite/g++.dg/warn/Wparentheses-27.C
===================================================================
--- testsuite/g++.dg/warn/Wparentheses-27.C (revision 0)
+++ testsuite/g++.dg/warn/Wparentheses-27.C (working copy)
@@ -0,0 +1,8 @@
+// PR c++/54194
+// { dg-options "-Wparentheses" }
+
+int main()
+{
+ char in[4] = { 0 };
+ in[1] = in[1] & 0x0F | ((in[3] & 0x3C) << 2); // { dg-warning "17:suggest
parentheses" }
+}
Index: c/c-typeck.c
===================================================================
--- c/c-typeck.c (revision 192224)
+++ 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),
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c (revision 192224)
+++ c-family/c-common.c (working copy)
@@ -10527,7 +10527,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)
{
@@ -10547,104 +10547,147 @@ void
switch (code)
{
case LSHIFT_EXPR:
- if (code_left == PLUS_EXPR || code_right == PLUS_EXPR)
- warning (OPT_Wparentheses,
- "suggest parentheses around %<+%> inside %<<<%>");
- else if (code_left == MINUS_EXPR || code_right == MINUS_EXPR)
- warning (OPT_Wparentheses,
- "suggest parentheses around %<-%> inside %<<<%>");
+ if (code_left == PLUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
+ "suggest parentheses around %<+%> inside %<<<%>");
+ else if (code_right == PLUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, loc), OPT_Wparentheses,
+ "suggest parentheses around %<+%> inside %<<<%>");
+ else if (code_left == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
+ "suggest parentheses around %<-%> inside %<<<%>");
+ else if (code_right == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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 %<>>%>");
- else if (code_left == MINUS_EXPR || code_right == MINUS_EXPR)
- warning (OPT_Wparentheses,
- "suggest parentheses around %<-%> inside %<>>%>");
+ if (code_left == PLUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
+ "suggest parentheses around %<+%> inside %<>>%>");
+ else if (code_right == PLUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, loc), OPT_Wparentheses,
+ "suggest parentheses around %<+%> inside %<>>%>");
+ else if (code_left == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
+ "suggest parentheses around %<-%> inside %<>>%>");
+ else if (code_right == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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 %<||%>");
+ if (code_left == TRUTH_ANDIF_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
+ "suggest parentheses around %<&&%> within %<||%>");
+ else if (code_right == TRUTH_ANDIF_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, loc), OPT_Wparentheses,
+ "suggest parentheses around %<&&%> within %<||%>");
return;
case BIT_IOR_EXPR:
if (code_left == BIT_AND_EXPR || code_left == BIT_XOR_EXPR
- || 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,
+ || code_left == PLUS_EXPR || code_left == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around arithmetic in operand of %<|%>");
+ else if (code_right == BIT_AND_EXPR || code_right == BIT_XOR_EXPR
+ || code_right == PLUS_EXPR || code_right == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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,
+ else if (TREE_CODE_CLASS (code_left) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around comparison in operand of %<|%>");
+ else if (TREE_CODE_CLASS (code_right) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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 (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
+ "suggest parentheses around operand of "
+ "%<!%> or change %<|%> to %<||%> or %<!%> to %<~%>");
return;
case BIT_XOR_EXPR:
if (code_left == BIT_AND_EXPR
- || code_left == PLUS_EXPR || code_left == MINUS_EXPR
- || code_right == BIT_AND_EXPR
- || code_right == PLUS_EXPR || code_right == MINUS_EXPR)
- warning (OPT_Wparentheses,
+ || code_left == PLUS_EXPR || code_left == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around arithmetic in operand of %<^%>");
+ else if (code_right == BIT_AND_EXPR
+ || code_right == PLUS_EXPR || code_right == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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,
+ else if (TREE_CODE_CLASS (code_left) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around comparison in operand of %<^%>");
+ else if (TREE_CODE_CLASS (code_right) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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,
+ if (code_left == PLUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around %<+%> in operand of %<&%>");
- else if (code_left == MINUS_EXPR || code_right == MINUS_EXPR)
- warning (OPT_Wparentheses,
+ else if (code_right == PLUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, loc), OPT_Wparentheses,
+ "suggest parentheses around %<+%> in operand of %<&%>");
+ else if (code_left == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around %<-%> in operand of %<&%>");
+ else if (code_right == MINUS_EXPR)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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,
+ else if (TREE_CODE_CLASS (code_left) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around comparison in operand of %<&%>");
+ else if (TREE_CODE_CLASS (code_right) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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 (EXPR_LOC_OR_LOC (arg_left, 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,
+ if (TREE_CODE_CLASS (code_left) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around comparison in operand of %<==%>");
+ else if (TREE_CODE_CLASS (code_right) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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,
+ if (TREE_CODE_CLASS (code_left) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
"suggest parentheses around comparison in operand of %<!=%>");
+ else if (TREE_CODE_CLASS (code_right) == tcc_comparison)
+ warning_at (EXPR_LOC_OR_LOC (arg_right, loc), OPT_Wparentheses,
+ "suggest parentheses around comparison in operand of %<!=%>");
return;
default:
- if (TREE_CODE_CLASS (code) == tcc_comparison
- && ((TREE_CODE_CLASS (code_left) == tcc_comparison
+ if (TREE_CODE_CLASS (code) == tcc_comparison)
+ {
+ if (TREE_CODE_CLASS (code_left) == tcc_comparison
&& code_left != NE_EXPR && code_left != EQ_EXPR
&& INTEGRAL_TYPE_P (TREE_TYPE (arg_left)))
- || (TREE_CODE_CLASS (code_right) == tcc_comparison
+ warning_at (EXPR_LOC_OR_LOC (arg_left, loc), OPT_Wparentheses,
+ "comparisons like %<X<=Y<=Z%> do not "
+ "have their mathematical meaning");
+ else if (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");
+ && INTEGRAL_TYPE_P (TREE_TYPE (arg_right)))
+ warning_at (EXPR_LOC_OR_LOC (arg_right, 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 192224)
+++ c-family/c-common.h (working copy)
@@ -989,7 +989,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: cp/parser.c
===================================================================
--- cp/parser.c (revision 192224)
+++ cp/parser.c (working copy)
@@ -7472,6 +7472,8 @@ cp_parser_binary_expression (cp_parser* parser, bo
rhs, rhs_type, &overload,
tf_warning_or_error);
current.lhs_type = current.tree_type;
+ if (EXPR_P (current.lhs))
+ SET_EXPR_LOCATION (current.lhs, current.loc);
/* If the binary operator required the use of an overloaded operator,
then this expression cannot be an integral constant-expression.
Index: cp/typeck.c
===================================================================
--- cp/typeck.c (revision 192224)
+++ cp/typeck.c (working copy)
@@ -3680,7 +3680,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: tree.h
===================================================================
--- tree.h (revision 192224)
+++ tree.h (working copy)
@@ -1619,7 +1619,10 @@ struct GTY(()) tree_constructor {
!= UNKNOWN_LOCATION)
/* The location to be used in a diagnostic about this expression. Do not
use this macro if the location will be assigned to other expressions. */
-#define EXPR_LOC_OR_HERE(NODE) (EXPR_HAS_LOCATION (NODE) ? (NODE)->exp.locus :
input_location)
+#define EXPR_LOC_OR_HERE(NODE) (EXPR_HAS_LOCATION (NODE) \
+ ? (NODE)->exp.locus : input_location)
+#define EXPR_LOC_OR_LOC(NODE, LOCUS) (EXPR_HAS_LOCATION (NODE) \
+ ? (NODE)->exp.locus : (LOCUS))
#define EXPR_FILENAME(NODE) LOCATION_FILE (EXPR_CHECK ((NODE))->exp.locus)
#define EXPR_LINENO(NODE) LOCATION_LINE (EXPR_CHECK (NODE)->exp.locus)
