* C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
@ 2017-07-31 14:15 Marek Polacek
2017-07-31 15:31 ` David Malcolm
2017-07-31 15:54 ` Martin Sebor
0 siblings, 2 replies; 14+ messages in thread
From: Marek Polacek @ 2017-07-31 14:15 UTC (permalink / raw)
To: GCC Patches, Joseph Myers, David Malcolm
This patch improves the diagnostic of -Wsign-compare for ?: by also printing
the types, similarly to my recent patch. But we can do even better here if we
actually point to the operand in question, so I passed the locations of the
operands from the parser. So instead of
x.c:8:16: warning: signed and unsigned type in conditional expression [-Wsign-compare]
return x ? y : -1;
^
you'll now see:
x.c:8:18: warning: operand of conditional expression changes signedness: 'int' to 'unsigned int' [-Wsign-compare]
return x ? y : -1;
^
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-07-31 Marek Polacek <polacek@redhat.com>
PR c/81417
* c-array-notation.c (fix_builtin_array_notation_fn): Update calls to
build_conditional_expr.
* c-parser.c (c_parser_conditional_expression): Pass the locations of
OP1 and OP2 down to build_conditional_expr.
* c-tree.h (build_conditional_expr): Update declaration.
* c-typeck.c (build_conditional_expr): Add location_t parameters.
For -Wsign-compare, also print the types.
* objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Update
a call to build_conditional_expr.
* Wsign-compare-1.c: New test.
* gcc.dg/compare1.c: Adjust dg-bogus.
* gcc.dg/compare2.c: Likewise.
* gcc.dg/compare3.c: Likewise.
* gcc.dg/compare7.c: Likewise.
* gcc.dg/compare8.c: Likewise.
* gcc.dg/compare9.c: Likewise.
* gcc.dg/pr11492.c: Likewise.
diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
index e430f5c681b..40f1cfdabb8 100644
--- gcc/c/c-array-notation.c
+++ gcc/c/c-array-notation.c
@@ -355,8 +355,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
new_var_init = build_modify_expr
@@ -375,8 +376,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
new_var_init = build_modify_expr
@@ -394,8 +396,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
new_var_init = build_modify_expr
@@ -413,8 +416,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
if (TYPE_MIN_VALUE (new_var_type))
@@ -434,7 +438,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_expr = build_conditional_expr
(location,
build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
- new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+ new_yes_expr, TREE_TYPE (*new_var), location,
+ new_no_expr, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
if (TYPE_MAX_VALUE (new_var_type))
@@ -454,7 +459,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_expr = build_conditional_expr
(location,
build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
- new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+ new_yes_expr, TREE_TYPE (*new_var), location,
+ new_no_expr, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
new_var_init = build_modify_expr
@@ -504,7 +510,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
build2 (LE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
func_parm),
false,
- new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+ new_yes_list, TREE_TYPE (*new_var), location,
+ new_no_list, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
new_var_init = build_modify_expr
@@ -554,7 +561,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
build2 (GE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
func_parm),
false,
- new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+ new_yes_list, TREE_TYPE (*new_var), location,
+ new_no_list, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE:
new_var_init = build_modify_expr
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 16cd3579972..1876f89d321 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -6508,7 +6508,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
tree omp_atomic_lhs)
{
struct c_expr cond, exp1, exp2, ret;
- location_t start, cond_loc, colon_loc, middle_loc;
+ location_t start, cond_loc, colon_loc, exp1_loc;
gcc_assert (!after || c_dialect_objc ());
@@ -6527,7 +6527,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
{
tree eptype = NULL_TREE;
- middle_loc = c_parser_peek_token (parser)->location;
+ location_t middle_loc = c_parser_peek_token (parser)->location;
pedwarn (middle_loc, OPT_Wpedantic,
"ISO C forbids omitting the middle term of a ?: expression");
if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
@@ -6544,6 +6544,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
if (eptype)
exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype, exp1.value);
exp1.original_type = NULL;
+ exp1_loc = start;
cond.value = c_objc_common_truthvalue_conversion (cond_loc, exp1.value);
c_inhibit_evaluation_warnings += cond.value == truthvalue_true_node;
}
@@ -6553,6 +6554,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
= c_objc_common_truthvalue_conversion
(cond_loc, default_conversion (cond.value));
c_inhibit_evaluation_warnings += cond.value == truthvalue_false_node;
+ exp1_loc = c_parser_peek_token (parser)->location;
exp1 = c_parser_expression_conv (parser);
mark_exp_read (exp1.value);
c_inhibit_evaluation_warnings +=
@@ -6569,16 +6571,14 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
ret.original_type = NULL;
return ret;
}
- {
- location_t exp2_loc = c_parser_peek_token (parser)->location;
- exp2 = c_parser_conditional_expression (parser, NULL, NULL_TREE);
- exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
- }
+ location_t exp2_loc = c_parser_peek_token (parser)->location;
+ exp2 = c_parser_conditional_expression (parser, NULL, NULL_TREE);
+ exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
c_inhibit_evaluation_warnings -= cond.value == truthvalue_true_node;
ret.value = build_conditional_expr (colon_loc, cond.value,
cond.original_code == C_MAYBE_CONST_EXPR,
- exp1.value, exp1.original_type,
- exp2.value, exp2.original_type);
+ exp1.value, exp1.original_type, exp1_loc,
+ exp2.value, exp2.original_type, exp2_loc);
ret.original_code = ERROR_MARK;
if (exp1.value == error_mark_node || exp2.value == error_mark_node)
ret.original_type = NULL;
diff --git gcc/c/c-tree.h gcc/c/c-tree.h
index a8197eb768d..be2f272d2dd 100644
--- gcc/c/c-tree.h
+++ gcc/c/c-tree.h
@@ -644,7 +644,7 @@ extern struct c_expr parser_build_binary_op (location_t,
enum tree_code, struct c_expr,
struct c_expr);
extern tree build_conditional_expr (location_t, tree, bool, tree, tree,
- tree, tree);
+ location_t, tree, tree, location_t);
extern tree build_compound_expr (location_t, tree, tree);
extern tree c_cast_expr (location_t, struct c_type_name *, tree);
extern tree build_c_cast (location_t, tree, tree);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 7451f3249fd..8b0e020d4c9 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4863,8 +4863,8 @@ ep_convert_and_check (location_t loc, tree type, tree expr,
tree
build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
- tree op1, tree op1_original_type, tree op2,
- tree op2_original_type)
+ tree op1, tree op1_original_type, location_t op1_loc,
+ tree op2, tree op2_original_type, location_t op2_loc)
{
tree type1;
tree type2;
@@ -5029,10 +5029,18 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
|| (unsigned_op1
&& tree_expr_nonnegative_warnv_p (op2, &ovf)))
/* OK */;
+ else if (unsigned_op2)
+ warning_at (op1_loc, OPT_Wsign_compare,
+ "operand of conditional expression "
+ "changes signedness: %qT to %qT",
+ TREE_TYPE (orig_op1),
+ TREE_TYPE (orig_op2));
else
- warning_at (colon_loc, OPT_Wsign_compare,
- ("signed and unsigned type in "
- "conditional expression"));
+ warning_at (op2_loc, OPT_Wsign_compare,
+ "operand of conditional expression "
+ "changes signedness: %qT to %qT",
+ TREE_TYPE (orig_op2),
+ TREE_TYPE (orig_op1));
}
if (!op1_maybe_const || TREE_CODE (op1) != INTEGER_CST)
op1 = c_wrap_maybe_const (op1, !op1_maybe_const);
diff --git gcc/objc/objc-next-runtime-abi-02.c gcc/objc/objc-next-runtime-abi-02.c
index 97314860e01..a9c2f5d3ba5 100644
--- gcc/objc/objc-next-runtime-abi-02.c
+++ gcc/objc/objc-next-runtime-abi-02.c
@@ -1645,8 +1645,8 @@ build_v2_build_objc_method_call (int super_flag, tree method_prototype,
/* ??? CHECKME. */
ret_val = build_conditional_expr (input_location,
ifexp, 1,
- ret_val, NULL_TREE,
- ftree, NULL_TREE);
+ ret_val, NULL_TREE, input_location,
+ ftree, NULL_TREE, input_location);
#endif
}
return ret_val;
diff --git gcc/testsuite/gcc.dg/Wsign-compare-1.c gcc/testsuite/gcc.dg/Wsign-compare-1.c
index e69de29bb2d..cd7fa1310b7 100644
--- gcc/testsuite/gcc.dg/Wsign-compare-1.c
+++ gcc/testsuite/gcc.dg/Wsign-compare-1.c
@@ -0,0 +1,45 @@
+/* PR c/81417 */
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare" } */
+
+unsigned int
+f1 (int x, unsigned int y)
+{
+ return x ? y : -1; /* { dg-warning "18:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
+}
+
+unsigned int
+f2 (int x, unsigned int y)
+{
+ return x ? -1 : y; /* { dg-warning "14:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
+}
+
+unsigned int
+f3 (unsigned int y)
+{
+ return y ?: -1; /* { dg-warning "15:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
+}
+
+unsigned int
+f4 (int x, unsigned y, short u)
+{
+ return x ? y : u; /* { dg-warning "18:operand of conditional expression changes signedness: 'short int' to 'unsigned int'" } */
+}
+
+unsigned int
+f5 (int x, unsigned y, short u)
+{
+ return x ? u : y; /* { dg-warning "14:operand of conditional expression changes signedness: 'short int' to 'unsigned int'" } */
+}
+
+unsigned int
+f6 (int x, unsigned y, signed char u)
+{
+ return x ? y : u; /* { dg-warning "18:operand of conditional expression changes signedness: 'signed char' to 'unsigned int'" } */
+}
+
+unsigned int
+f7 (int x, unsigned y, signed char u)
+{
+ return x ? u : y; /* { dg-warning "14:operand of conditional expression changes signedness: 'signed char' to 'unsigned int'" } */
+}
diff --git gcc/testsuite/gcc.dg/compare1.c gcc/testsuite/gcc.dg/compare1.c
index 7becfbdb17f..ebab8c2cbf7 100644
--- gcc/testsuite/gcc.dg/compare1.c
+++ gcc/testsuite/gcc.dg/compare1.c
@@ -22,17 +22,17 @@ enum mm2
int f(enum mm1 x)
{
- return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
+ return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
}
int g(enum mm1 x)
{
- return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
+ return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
}
int h(enum mm2 x)
{
- return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
+ return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
}
int i(enum mm2 x)
diff --git gcc/testsuite/gcc.dg/compare2.c gcc/testsuite/gcc.dg/compare2.c
index c309f1d00eb..cfadaccb8af 100644
--- gcc/testsuite/gcc.dg/compare2.c
+++ gcc/testsuite/gcc.dg/compare2.c
@@ -9,35 +9,35 @@ int tf = 1;
void f(int x, unsigned int y)
{
/* ?: branches are constants. */
- x > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 1" } */
- y > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 2" } */
+ x > (tf?64:128); /* { dg-bogus "changes signedness" "case 1" } */
+ y > (tf?64:128); /* { dg-bogus "changes signedness" "case 2" } */
/* ?: branches are (recursively) constants. */
- x > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 3" } */
- y > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 4" } */
+ x > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 3" } */
+ y > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 4" } */
/* ?: branches are signed constants. */
- x > (tf?64:-1); /* { dg-bogus "signed and unsigned" "case 5" } */
+ x > (tf?64:-1); /* { dg-bogus "changes signedness" "case 5" } */
y > (tf?64:-1); /* { dg-warning "different signedness" "case 6" } */
/* ?: branches are (recursively) signed constants. */
- x > (tf?64:(tf?128:-1)); /* { dg-bogus "signed and unsigned" "case 7" } */
+ x > (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case 7" } */
y > (tf?64:(tf?128:-1)); /* { dg-warning "different signedness" "case 8" } */
/* Statement expression. */
- x > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 9" } */
- y > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 10" } */
+ x > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 9" } */
+ y > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 10" } */
/* Statement expression with recursive ?: . */
- x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 11" } */
- y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 12" } */
+ x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 11" } */
+ y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 12" } */
/* Statement expression with signed ?:. */
- x > ({tf; tf?64:-1;}); /* { dg-bogus "signed and unsigned" "case 13" } */
+ x > ({tf; tf?64:-1;}); /* { dg-bogus "changes signedness" "case 13" } */
y > ({tf; tf?64:-1;}); /* { dg-warning "different signedness" "case 14" } */
/* Statement expression with recursive signed ?:. */
- x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "signed and unsigned" "case 15" } */
+ x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "changes signedness" "case 15" } */
y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different signedness" "case 16" } */
/* ?: branches are constants. */
diff --git gcc/testsuite/gcc.dg/compare3.c gcc/testsuite/gcc.dg/compare3.c
index eda3faf2754..836231fb870 100644
--- gcc/testsuite/gcc.dg/compare3.c
+++ gcc/testsuite/gcc.dg/compare3.c
@@ -11,49 +11,49 @@ void f(int x, unsigned int y)
/* Test comparing conditional expressions containing truth values.
This can occur explicitly, or e.g. when (foo?2:(bar?1:0)) is
optimized into (foo?2:(bar!=0)). */
- x > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 1" } */
- y > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 2" } */
- x > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 3" } */
- y > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 4" } */
-
- x > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 5" } */
- y > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 6" } */
- x > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 7" } */
- y > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 8" } */
-
- x > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 9" } */
- y > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 10" } */
- x > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 11" } */
- y > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 12" } */
-
- x < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 13" } */
- y < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 14" } */
- x < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 15" } */
- y < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 16" } */
-
- x > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 17" } */
- y > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 18" } */
- x > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 19" } */
- y > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 20" } */
-
- x > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 21" } */
- y > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 22" } */
- x > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 23" } */
- y > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 24" } */
-
- x > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 25" } */
- y > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 26" } */
- x > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 27" } */
- y > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 28" } */
-
- x > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 29" } */
- y > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 30" } */
- x > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 31" } */
- y > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 32" } */
-
- x > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 33" } */
- y > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 34" } */
- x > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 35" } */
- y > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 36" } */
+ x > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 1" } */
+ y > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 2" } */
+ x > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 3" } */
+ y > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 4" } */
+
+ x > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 5" } */
+ y > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 6" } */
+ x > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 7" } */
+ y > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 8" } */
+
+ x > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 9" } */
+ y > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 10" } */
+ x > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 11" } */
+ y > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 12" } */
+
+ x < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 13" } */
+ y < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 14" } */
+ x < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 15" } */
+ y < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 16" } */
+
+ x > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 17" } */
+ y > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 18" } */
+ x > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 19" } */
+ y > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 20" } */
+
+ x > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 21" } */
+ y > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 22" } */
+ x > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 23" } */
+ y > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 24" } */
+
+ x > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 25" } */
+ y > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 26" } */
+ x > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 27" } */
+ y > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 28" } */
+
+ x > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 29" } */
+ y > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 30" } */
+ x > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 31" } */
+ y > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 32" } */
+
+ x > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 33" } */
+ y > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 34" } */
+ x > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 35" } */
+ y > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 36" } */
}
diff --git gcc/testsuite/gcc.dg/compare7.c gcc/testsuite/gcc.dg/compare7.c
index e2fbc04bfc2..b6fe6e78334 100644
--- gcc/testsuite/gcc.dg/compare7.c
+++ gcc/testsuite/gcc.dg/compare7.c
@@ -6,5 +6,5 @@
int f(unsigned a, int b)
{
- return a < b; /* { dg-bogus "signed and unsigned" } */
+ return a < b; /* { dg-bogus "changes signedness" } */
}
diff --git gcc/testsuite/gcc.dg/compare8.c gcc/testsuite/gcc.dg/compare8.c
index d723c45a095..d09b69c53a2 100644
--- gcc/testsuite/gcc.dg/compare8.c
+++ gcc/testsuite/gcc.dg/compare8.c
@@ -4,18 +4,18 @@
int
f(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
int
g(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
int
h(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
diff --git gcc/testsuite/gcc.dg/compare9.c gcc/testsuite/gcc.dg/compare9.c
index 02150cb1fb6..fba61e42a48 100644
--- gcc/testsuite/gcc.dg/compare9.c
+++ gcc/testsuite/gcc.dg/compare9.c
@@ -22,20 +22,20 @@ enum mm2
int f(enum mm1 x)
{
- return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
+ return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
}
int g(enum mm1 x)
{
- return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
+ return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
}
int h(enum mm2 x)
{
- return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
+ return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
}
int i(enum mm2 x)
{
- return x == (tf?DI2:-1); /* { dg-bogus "signed and unsigned" "case 4" } */
+ return x == (tf?DI2:-1); /* { dg-bogus "changes signedness" "case 4" } */
}
diff --git gcc/testsuite/gcc.dg/pr11492.c gcc/testsuite/gcc.dg/pr11492.c
index cf17712dde1..86435a83e79 100644
--- gcc/testsuite/gcc.dg/pr11492.c
+++ gcc/testsuite/gcc.dg/pr11492.c
@@ -5,7 +5,7 @@ int main( void )
{
unsigned int a;
unsigned char b;
- for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison between signed and unsigned integer" } */
+ for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison of integer expressions of different signedness" } */
{ ; }
return 0;
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-07-31 14:15 C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417) Marek Polacek
@ 2017-07-31 15:31 ` David Malcolm
2017-08-01 14:16 ` Marek Polacek
2017-07-31 15:54 ` Martin Sebor
1 sibling, 1 reply; 14+ messages in thread
From: David Malcolm @ 2017-07-31 15:31 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Joseph Myers
On Mon, 2017-07-31 at 16:14 +0200, Marek Polacek wrote:
> This patch improves the diagnostic of -Wsign-compare for ?: by also
> printing
> the types, similarly to my recent patch. But we can do even better
> here if we
> actually point to the operand in question, so I passed the locations
> of the
> operands from the parser.
Thanks for updating the patch.
> So instead of
>
> x.c:8:16: warning: signed and unsigned type in conditional expression
> [-Wsign-compare]
> return x ? y : -1;
> ^
> you'll now see:
>
> x.c:8:18: warning: operand of conditional expression changes
> signedness: 'int' to 'unsigned int' [-Wsign-compare]
> return x ? y : -1;
> ^
That's an improvement, but I would have expected it to underline the
whole of the pertinent subexpression e.g.:
return x ? y : -1;
^~
rather than just:
return x ? y : -1;
^
From my reading of the patch, it's capturing just the location of the
first token within the subexpression (hence e.g. just the minus token
in the example above, which happens to make some sense for this case,
but wouldn't in general).
Hopefully you can get at the location_t for the whole of the
subexpression using c_expr, rather than peeking at the first token.
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
The patch doesn't have a testcase for the location information; please
add one, using -fdiagnostics-show-caret and dg-begin-multiline-output/
dg-end-multiline-output. Please ensure that the pertinent expressions
are more than one character wide, so that the test properly verifies
the underlining.
Thanks
Dave
> 2017-07-31 Marek Polacek <polacek@redhat.com>
>
> PR c/81417
> * c-array-notation.c (fix_builtin_array_notation_fn): Update
> calls to
> build_conditional_expr.
> * c-parser.c (c_parser_conditional_expression): Pass the
> locations of
> OP1 and OP2 down to build_conditional_expr.
> * c-tree.h (build_conditional_expr): Update declaration.
> * c-typeck.c (build_conditional_expr): Add location_t
> parameters.
> For -Wsign-compare, also print the types.
>
> * objc-next-runtime-abi-02.c (build_v2_build_objc_method_call):
> Update
> a call to build_conditional_expr.
>
> * Wsign-compare-1.c: New test.
> * gcc.dg/compare1.c: Adjust dg-bogus.
> * gcc.dg/compare2.c: Likewise.
> * gcc.dg/compare3.c: Likewise.
> * gcc.dg/compare7.c: Likewise.
> * gcc.dg/compare8.c: Likewise.
> * gcc.dg/compare9.c: Likewise.
> * gcc.dg/pr11492.c: Likewise.
>
> diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
> index e430f5c681b..40f1cfdabb8 100644
> --- gcc/c/c-array-notation.c
> +++ gcc/c/c-array-notation.c
> @@ -355,8 +355,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
> new_var_init = build_modify_expr
> @@ -375,8 +376,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
> new_var_init = build_modify_expr
> @@ -394,8 +396,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
> new_var_init = build_modify_expr
> @@ -413,8 +416,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
> if (TYPE_MIN_VALUE (new_var_type))
> @@ -434,7 +438,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var,
> func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE
> (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
> if (TYPE_MAX_VALUE (new_var_type))
> @@ -454,7 +459,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var,
> func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE
> (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
> new_var_init = build_modify_expr
> @@ -504,7 +510,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> build2 (LE_EXPR, TREE_TYPE (array_ind_value),
> array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE
> (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
> new_var_init = build_modify_expr
> @@ -554,7 +561,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> build2 (GE_EXPR, TREE_TYPE (array_ind_value),
> array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE
> (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE:
> new_var_init = build_modify_expr
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index 16cd3579972..1876f89d321 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -6508,7 +6508,7 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> tree omp_atomic_lhs)
> {
> struct c_expr cond, exp1, exp2, ret;
> - location_t start, cond_loc, colon_loc, middle_loc;
> + location_t start, cond_loc, colon_loc, exp1_loc;
>
> gcc_assert (!after || c_dialect_objc ());
>
> @@ -6527,7 +6527,7 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> {
> tree eptype = NULL_TREE;
>
> - middle_loc = c_parser_peek_token (parser)->location;
> + location_t middle_loc = c_parser_peek_token (parser)-
> >location;
> pedwarn (middle_loc, OPT_Wpedantic,
> "ISO C forbids omitting the middle term of a ?:
> expression");
> if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
> @@ -6544,6 +6544,7 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> if (eptype)
> exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype,
> exp1.value);
> exp1.original_type = NULL;
> + exp1_loc = start;
> cond.value = c_objc_common_truthvalue_conversion (cond_loc,
> exp1.value);
> c_inhibit_evaluation_warnings += cond.value ==
> truthvalue_true_node;
> }
> @@ -6553,6 +6554,7 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> = c_objc_common_truthvalue_conversion
> (cond_loc, default_conversion (cond.value));
> c_inhibit_evaluation_warnings += cond.value ==
> truthvalue_false_node;
> + exp1_loc = c_parser_peek_token (parser)->location;
> exp1 = c_parser_expression_conv (parser);
> mark_exp_read (exp1.value);
> c_inhibit_evaluation_warnings +=
> @@ -6569,16 +6571,14 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> ret.original_type = NULL;
> return ret;
> }
> - {
> - location_t exp2_loc = c_parser_peek_token (parser)->location;
> - exp2 = c_parser_conditional_expression (parser, NULL,
> NULL_TREE);
> - exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
> - }
> + location_t exp2_loc = c_parser_peek_token (parser)->location;
> + exp2 = c_parser_conditional_expression (parser, NULL, NULL_TREE);
> + exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
> c_inhibit_evaluation_warnings -= cond.value ==
> truthvalue_true_node;
> ret.value = build_conditional_expr (colon_loc, cond.value,
> cond.original_code ==
> C_MAYBE_CONST_EXPR,
> - exp1.value,
> exp1.original_type,
> - exp2.value,
> exp2.original_type);
> + exp1.value,
> exp1.original_type, exp1_loc,
> + exp2.value,
> exp2.original_type, exp2_loc);
> ret.original_code = ERROR_MARK;
> if (exp1.value == error_mark_node || exp2.value ==
> error_mark_node)
> ret.original_type = NULL;
> diff --git gcc/c/c-tree.h gcc/c/c-tree.h
> index a8197eb768d..be2f272d2dd 100644
> --- gcc/c/c-tree.h
> +++ gcc/c/c-tree.h
> @@ -644,7 +644,7 @@ extern struct c_expr parser_build_binary_op
> (location_t,
> enum tree_code,
> struct c_expr,
> struct c_expr);
> extern tree build_conditional_expr (location_t, tree, bool, tree,
> tree,
> - tree, tree);
> + location_t, tree, tree,
> location_t);
> extern tree build_compound_expr (location_t, tree, tree);
> extern tree c_cast_expr (location_t, struct c_type_name *, tree);
> extern tree build_c_cast (location_t, tree, tree);
> diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> index 7451f3249fd..8b0e020d4c9 100644
> --- gcc/c/c-typeck.c
> +++ gcc/c/c-typeck.c
> @@ -4863,8 +4863,8 @@ ep_convert_and_check (location_t loc, tree
> type, tree expr,
>
> tree
> build_conditional_expr (location_t colon_loc, tree ifexp, bool
> ifexp_bcp,
> - tree op1, tree op1_original_type, tree op2,
> - tree op2_original_type)
> + tree op1, tree op1_original_type, location_t
> op1_loc,
> + tree op2, tree op2_original_type, location_t
> op2_loc)
> {
> tree type1;
> tree type2;
> @@ -5029,10 +5029,18 @@ build_conditional_expr (location_t colon_loc,
> tree ifexp, bool ifexp_bcp,
> || (unsigned_op1
> && tree_expr_nonnegative_warnv_p (op2,
> &ovf)))
> /* OK */;
> + else if (unsigned_op2)
> + warning_at (op1_loc, OPT_Wsign_compare,
> + "operand of conditional
> expression "
> + "changes signedness: %qT to
> %qT",
> + TREE_TYPE (orig_op1),
> + TREE_TYPE (orig_op2));
> else
> - warning_at (colon_loc, OPT_Wsign_compare,
> - ("signed and unsigned type in "
> - "conditional expression"));
> + warning_at (op2_loc, OPT_Wsign_compare,
> + "operand of conditional
> expression "
> + "changes signedness: %qT to
> %qT",
> + TREE_TYPE (orig_op2),
> + TREE_TYPE (orig_op1));
> }
> if (!op1_maybe_const || TREE_CODE (op1) !=
> INTEGER_CST)
> op1 = c_wrap_maybe_const (op1,
> !op1_maybe_const);
> diff --git gcc/objc/objc-next-runtime-abi-02.c gcc/objc/objc-next-
> runtime-abi-02.c
> index 97314860e01..a9c2f5d3ba5 100644
> --- gcc/objc/objc-next-runtime-abi-02.c
> +++ gcc/objc/objc-next-runtime-abi-02.c
> @@ -1645,8 +1645,8 @@ build_v2_build_objc_method_call (int
> super_flag, tree method_prototype,
> /* ??? CHECKME. */
> ret_val = build_conditional_expr (input_location,
> ifexp, 1,
> - ret_val, NULL_TREE,
> - ftree, NULL_TREE);
> + ret_val, NULL_TREE,
> input_location,
> + ftree, NULL_TREE,
> input_location);
> #endif
> }
> return ret_val;
> diff --git gcc/testsuite/gcc.dg/Wsign-compare-1.c
> gcc/testsuite/gcc.dg/Wsign-compare-1.c
> index e69de29bb2d..cd7fa1310b7 100644
> --- gcc/testsuite/gcc.dg/Wsign-compare-1.c
> +++ gcc/testsuite/gcc.dg/Wsign-compare-1.c
> @@ -0,0 +1,45 @@
> +/* PR c/81417 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wsign-compare" } */
> +
> +unsigned int
> +f1 (int x, unsigned int y)
> +{
> + return x ? y : -1; /* { dg-warning "18:operand of conditional
> expression changes signedness: 'int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f2 (int x, unsigned int y)
> +{
> + return x ? -1 : y; /* { dg-warning "14:operand of conditional
> expression changes signedness: 'int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f3 (unsigned int y)
> +{
> + return y ?: -1; /* { dg-warning "15:operand of conditional
> expression changes signedness: 'int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f4 (int x, unsigned y, short u)
> +{
> + return x ? y : u; /* { dg-warning "18:operand of conditional
> expression changes signedness: 'short int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f5 (int x, unsigned y, short u)
> +{
> + return x ? u : y; /* { dg-warning "14:operand of conditional
> expression changes signedness: 'short int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f6 (int x, unsigned y, signed char u)
> +{
> + return x ? y : u; /* { dg-warning "18:operand of conditional
> expression changes signedness: 'signed char' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f7 (int x, unsigned y, signed char u)
> +{
> + return x ? u : y; /* { dg-warning "14:operand of conditional
> expression changes signedness: 'signed char' to 'unsigned int'" } */
> +}
> diff --git gcc/testsuite/gcc.dg/compare1.c
> gcc/testsuite/gcc.dg/compare1.c
> index 7becfbdb17f..ebab8c2cbf7 100644
> --- gcc/testsuite/gcc.dg/compare1.c
> +++ gcc/testsuite/gcc.dg/compare1.c
> @@ -22,17 +22,17 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case
> 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case
> 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case
> 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case
> 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned"
> "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case
> 3" } */
> }
>
> int i(enum mm2 x)
> diff --git gcc/testsuite/gcc.dg/compare2.c
> gcc/testsuite/gcc.dg/compare2.c
> index c309f1d00eb..cfadaccb8af 100644
> --- gcc/testsuite/gcc.dg/compare2.c
> +++ gcc/testsuite/gcc.dg/compare2.c
> @@ -9,35 +9,35 @@ int tf = 1;
> void f(int x, unsigned int y)
> {
> /* ?: branches are constants. */
> - x > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 1" } */
> - y > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 2" } */
> + x > (tf?64:128); /* { dg-bogus "changes signedness" "case 1" } */
> + y > (tf?64:128); /* { dg-bogus "changes signedness" "case 2" } */
>
> /* ?: branches are (recursively) constants. */
> - x > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned"
> "case 3" } */
> - y > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned"
> "case 4" } */
> + x > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case
> 3" } */
> + y > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case
> 4" } */
>
> /* ?: branches are signed constants. */
> - x > (tf?64:-1); /* { dg-bogus "signed and unsigned" "case 5" } */
> + x > (tf?64:-1); /* { dg-bogus "changes signedness" "case 5" } */
> y > (tf?64:-1); /* { dg-warning "different signedness" "case 6" }
> */
>
> /* ?: branches are (recursively) signed constants. */
> - x > (tf?64:(tf?128:-1)); /* { dg-bogus "signed and unsigned" "case
> 7" } */
> + x > (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case
> 7" } */
> y > (tf?64:(tf?128:-1)); /* { dg-warning "different signedness"
> "case 8" } */
>
> /* Statement expression. */
> - x > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 9" } */
> - y > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 10" }
> */
> + x > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 9" } */
> + y > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 10" } */
>
> /* Statement expression with recursive ?: . */
> - x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and
> unsigned" "case 11" } */
> - y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and
> unsigned" "case 12" } */
> + x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes
> signedness" "case 11" } */
> + y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes
> signedness" "case 12" } */
>
> /* Statement expression with signed ?:. */
> - x > ({tf; tf?64:-1;}); /* { dg-bogus "signed and unsigned" "case
> 13" } */
> + x > ({tf; tf?64:-1;}); /* { dg-bogus "changes signedness" "case
> 13" } */
> y > ({tf; tf?64:-1;}); /* { dg-warning "different signedness"
> "case 14" } */
>
> /* Statement expression with recursive signed ?:. */
> - x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "signed and
> unsigned" "case 15" } */
> + x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "changes signedness"
> "case 15" } */
> y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different
> signedness" "case 16" } */
>
> /* ?: branches are constants. */
> diff --git gcc/testsuite/gcc.dg/compare3.c
> gcc/testsuite/gcc.dg/compare3.c
> index eda3faf2754..836231fb870 100644
> --- gcc/testsuite/gcc.dg/compare3.c
> +++ gcc/testsuite/gcc.dg/compare3.c
> @@ -11,49 +11,49 @@ void f(int x, unsigned int y)
> /* Test comparing conditional expressions containing truth values.
> This can occur explicitly, or e.g. when (foo?2:(bar?1:0)) is
> optimized into (foo?2:(bar!=0)). */
> - x > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 1"
> } */
> - y > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 2"
> } */
> - x > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 3"
> } */
> - y > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 4"
> } */
> -
> - x > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 5"
> } */
> - y > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 6"
> } */
> - x > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 7"
> } */
> - y > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 8"
> } */
> -
> - x > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 9" }
> */
> - y > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 10"
> } */
> - x > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 11"
> } */
> - y > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 12"
> } */
> -
> - x < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 13"
> } */
> - y < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 14"
> } */
> - x < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 15"
> } */
> - y < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 16"
> } */
> -
> - x > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 17"
> } */
> - y > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 18"
> } */
> - x > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 19"
> } */
> - y > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 20"
> } */
> -
> - x > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 21"
> } */
> - y > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 22"
> } */
> - x > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 23"
> } */
> - y > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 24"
> } */
> -
> - x > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 25"
> } */
> - y > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 26"
> } */
> - x > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 27"
> } */
> - y > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 28"
> } */
> -
> - x > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 29"
> } */
> - y > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 30"
> } */
> - x > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 31"
> } */
> - y > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 32"
> } */
> -
> - x > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 33" }
> */
> - y > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 34" }
> */
> - x > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 35" }
> */
> - y > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 36" }
> */
> + x > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 1" }
> */
> + y > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 2" }
> */
> + x > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 3" }
> */
> + y > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 4" }
> */
> +
> + x > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 5" }
> */
> + y > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 6" }
> */
> + x > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 7" }
> */
> + y > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 8" }
> */
> +
> + x > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 9" }
> */
> + y > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 10" }
> */
> + x > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 11" }
> */
> + y > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 12" }
> */
> +
> + x < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 13" }
> */
> + y < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 14" }
> */
> + x < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 15" }
> */
> + y < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 16" }
> */
> +
> + x > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 17"
> } */
> + y > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 18"
> } */
> + x > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 19"
> } */
> + y > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 20"
> } */
> +
> + x > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 21"
> } */
> + y > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 22"
> } */
> + x > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 23"
> } */
> + y > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 24"
> } */
> +
> + x > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 25"
> } */
> + y > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 26"
> } */
> + x > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 27"
> } */
> + y > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 28"
> } */
> +
> + x > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 29"
> } */
> + y > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 30"
> } */
> + x > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 31"
> } */
> + y > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 32"
> } */
> +
> + x > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 33" }
> */
> + y > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 34" }
> */
> + x > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 35" }
> */
> + y > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 36" }
> */
>
> }
> diff --git gcc/testsuite/gcc.dg/compare7.c
> gcc/testsuite/gcc.dg/compare7.c
> index e2fbc04bfc2..b6fe6e78334 100644
> --- gcc/testsuite/gcc.dg/compare7.c
> +++ gcc/testsuite/gcc.dg/compare7.c
> @@ -6,5 +6,5 @@
>
> int f(unsigned a, int b)
> {
> - return a < b; /* { dg-bogus "signed and unsigned" } */
> + return a < b; /* { dg-bogus "changes signedness" } */
> }
> diff --git gcc/testsuite/gcc.dg/compare8.c
> gcc/testsuite/gcc.dg/compare8.c
> index d723c45a095..d09b69c53a2 100644
> --- gcc/testsuite/gcc.dg/compare8.c
> +++ gcc/testsuite/gcc.dg/compare8.c
> @@ -4,18 +4,18 @@
> int
> f(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "signed and
> unsigned" } */
> + return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "changes
> signedness" } */
> }
>
> int
> g(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "signed and
> unsigned" } */
> + return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "changes
> signedness" } */
> }
>
> int
> h(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "signed and
> unsigned" } */
> + return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "changes
> signedness" } */
> }
>
> diff --git gcc/testsuite/gcc.dg/compare9.c
> gcc/testsuite/gcc.dg/compare9.c
> index 02150cb1fb6..fba61e42a48 100644
> --- gcc/testsuite/gcc.dg/compare9.c
> +++ gcc/testsuite/gcc.dg/compare9.c
> @@ -22,20 +22,20 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case
> 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case
> 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case
> 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case
> 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned"
> "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case
> 3" } */
> }
>
> int i(enum mm2 x)
> {
> - return x == (tf?DI2:-1); /* { dg-bogus "signed and unsigned" "case
> 4" } */
> + return x == (tf?DI2:-1); /* { dg-bogus "changes signedness" "case
> 4" } */
> }
> diff --git gcc/testsuite/gcc.dg/pr11492.c
> gcc/testsuite/gcc.dg/pr11492.c
> index cf17712dde1..86435a83e79 100644
> --- gcc/testsuite/gcc.dg/pr11492.c
> +++ gcc/testsuite/gcc.dg/pr11492.c
> @@ -5,7 +5,7 @@ int main( void )
> {
> unsigned int a;
> unsigned char b;
> - for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison
> between signed and unsigned integer" } */
> + for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison
> of integer expressions of different signedness" } */
> { ; }
>
> return 0;
>
> Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-07-31 14:15 C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417) Marek Polacek
2017-07-31 15:31 ` David Malcolm
@ 2017-07-31 15:54 ` Martin Sebor
2017-07-31 16:05 ` Marek Polacek
1 sibling, 1 reply; 14+ messages in thread
From: Martin Sebor @ 2017-07-31 15:54 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Joseph Myers, David Malcolm
On 07/31/2017 08:14 AM, Marek Polacek wrote:
> This patch improves the diagnostic of -Wsign-compare for ?: by also printing
> the types, similarly to my recent patch. But we can do even better here if we
> actually point to the operand in question, so I passed the locations of the
> operands from the parser. So instead of
>
> x.c:8:16: warning: signed and unsigned type in conditional expression [-Wsign-compare]
> return x ? y : -1;
> ^
> you'll now see:
>
> x.c:8:18: warning: operand of conditional expression changes signedness: 'int' to 'unsigned int' [-Wsign-compare]
> return x ? y : -1;
> ^
I like that this is more informative than the last warning you
committed for this bug: it says what type the operand is converted
to. The last one only shows what the types of the operands are but
leaves users guessing as to what that might mean (integer promotion
rules are often poorly understood). Where the last warning prints
comparison of integer expressions of different signedness: ‘int’ and
‘unsigned int’
it would be nice to go back and add this detail to it as well, and
have it print something like this instead:
comparison of integer expressions of different signedness changes
type of the second operand from ‘int’ to ‘unsigned int’
Where constant expressions are involved it would also be helpful
to show the result of the conversion. For instance:
comparison between ‘int’ and ‘unsigned int’ changes the value of the
second operand from ‘-1’ to ‘4294967296’
Martin
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-07-31 Marek Polacek <polacek@redhat.com>
>
> PR c/81417
> * c-array-notation.c (fix_builtin_array_notation_fn): Update calls to
> build_conditional_expr.
> * c-parser.c (c_parser_conditional_expression): Pass the locations of
> OP1 and OP2 down to build_conditional_expr.
> * c-tree.h (build_conditional_expr): Update declaration.
> * c-typeck.c (build_conditional_expr): Add location_t parameters.
> For -Wsign-compare, also print the types.
>
> * objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Update
> a call to build_conditional_expr.
>
> * Wsign-compare-1.c: New test.
> * gcc.dg/compare1.c: Adjust dg-bogus.
> * gcc.dg/compare2.c: Likewise.
> * gcc.dg/compare3.c: Likewise.
> * gcc.dg/compare7.c: Likewise.
> * gcc.dg/compare8.c: Likewise.
> * gcc.dg/compare9.c: Likewise.
> * gcc.dg/pr11492.c: Likewise.
>
> diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
> index e430f5c681b..40f1cfdabb8 100644
> --- gcc/c/c-array-notation.c
> +++ gcc/c/c-array-notation.c
> @@ -355,8 +355,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
> new_var_init = build_modify_expr
> @@ -375,8 +376,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
> new_var_init = build_modify_expr
> @@ -394,8 +396,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
> new_var_init = build_modify_expr
> @@ -413,8 +416,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
> if (TYPE_MIN_VALUE (new_var_type))
> @@ -434,7 +438,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
> if (TYPE_MAX_VALUE (new_var_type))
> @@ -454,7 +459,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
> new_var_init = build_modify_expr
> @@ -504,7 +510,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> build2 (LE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
> new_var_init = build_modify_expr
> @@ -554,7 +561,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> build2 (GE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE:
> new_var_init = build_modify_expr
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index 16cd3579972..1876f89d321 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -6508,7 +6508,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> tree omp_atomic_lhs)
> {
> struct c_expr cond, exp1, exp2, ret;
> - location_t start, cond_loc, colon_loc, middle_loc;
> + location_t start, cond_loc, colon_loc, exp1_loc;
>
> gcc_assert (!after || c_dialect_objc ());
>
> @@ -6527,7 +6527,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> {
> tree eptype = NULL_TREE;
>
> - middle_loc = c_parser_peek_token (parser)->location;
> + location_t middle_loc = c_parser_peek_token (parser)->location;
> pedwarn (middle_loc, OPT_Wpedantic,
> "ISO C forbids omitting the middle term of a ?: expression");
> if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
> @@ -6544,6 +6544,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> if (eptype)
> exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype, exp1.value);
> exp1.original_type = NULL;
> + exp1_loc = start;
> cond.value = c_objc_common_truthvalue_conversion (cond_loc, exp1.value);
> c_inhibit_evaluation_warnings += cond.value == truthvalue_true_node;
> }
> @@ -6553,6 +6554,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> = c_objc_common_truthvalue_conversion
> (cond_loc, default_conversion (cond.value));
> c_inhibit_evaluation_warnings += cond.value == truthvalue_false_node;
> + exp1_loc = c_parser_peek_token (parser)->location;
> exp1 = c_parser_expression_conv (parser);
> mark_exp_read (exp1.value);
> c_inhibit_evaluation_warnings +=
> @@ -6569,16 +6571,14 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> ret.original_type = NULL;
> return ret;
> }
> - {
> - location_t exp2_loc = c_parser_peek_token (parser)->location;
> - exp2 = c_parser_conditional_expression (parser, NULL, NULL_TREE);
> - exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
> - }
> + location_t exp2_loc = c_parser_peek_token (parser)->location;
> + exp2 = c_parser_conditional_expression (parser, NULL, NULL_TREE);
> + exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
> c_inhibit_evaluation_warnings -= cond.value == truthvalue_true_node;
> ret.value = build_conditional_expr (colon_loc, cond.value,
> cond.original_code == C_MAYBE_CONST_EXPR,
> - exp1.value, exp1.original_type,
> - exp2.value, exp2.original_type);
> + exp1.value, exp1.original_type, exp1_loc,
> + exp2.value, exp2.original_type, exp2_loc);
> ret.original_code = ERROR_MARK;
> if (exp1.value == error_mark_node || exp2.value == error_mark_node)
> ret.original_type = NULL;
> diff --git gcc/c/c-tree.h gcc/c/c-tree.h
> index a8197eb768d..be2f272d2dd 100644
> --- gcc/c/c-tree.h
> +++ gcc/c/c-tree.h
> @@ -644,7 +644,7 @@ extern struct c_expr parser_build_binary_op (location_t,
> enum tree_code, struct c_expr,
> struct c_expr);
> extern tree build_conditional_expr (location_t, tree, bool, tree, tree,
> - tree, tree);
> + location_t, tree, tree, location_t);
> extern tree build_compound_expr (location_t, tree, tree);
> extern tree c_cast_expr (location_t, struct c_type_name *, tree);
> extern tree build_c_cast (location_t, tree, tree);
> diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> index 7451f3249fd..8b0e020d4c9 100644
> --- gcc/c/c-typeck.c
> +++ gcc/c/c-typeck.c
> @@ -4863,8 +4863,8 @@ ep_convert_and_check (location_t loc, tree type, tree expr,
>
> tree
> build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
> - tree op1, tree op1_original_type, tree op2,
> - tree op2_original_type)
> + tree op1, tree op1_original_type, location_t op1_loc,
> + tree op2, tree op2_original_type, location_t op2_loc)
> {
> tree type1;
> tree type2;
> @@ -5029,10 +5029,18 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
> || (unsigned_op1
> && tree_expr_nonnegative_warnv_p (op2, &ovf)))
> /* OK */;
> + else if (unsigned_op2)
> + warning_at (op1_loc, OPT_Wsign_compare,
> + "operand of conditional expression "
> + "changes signedness: %qT to %qT",
> + TREE_TYPE (orig_op1),
> + TREE_TYPE (orig_op2));
> else
> - warning_at (colon_loc, OPT_Wsign_compare,
> - ("signed and unsigned type in "
> - "conditional expression"));
> + warning_at (op2_loc, OPT_Wsign_compare,
> + "operand of conditional expression "
> + "changes signedness: %qT to %qT",
> + TREE_TYPE (orig_op2),
> + TREE_TYPE (orig_op1));
> }
> if (!op1_maybe_const || TREE_CODE (op1) != INTEGER_CST)
> op1 = c_wrap_maybe_const (op1, !op1_maybe_const);
> diff --git gcc/objc/objc-next-runtime-abi-02.c gcc/objc/objc-next-runtime-abi-02.c
> index 97314860e01..a9c2f5d3ba5 100644
> --- gcc/objc/objc-next-runtime-abi-02.c
> +++ gcc/objc/objc-next-runtime-abi-02.c
> @@ -1645,8 +1645,8 @@ build_v2_build_objc_method_call (int super_flag, tree method_prototype,
> /* ??? CHECKME. */
> ret_val = build_conditional_expr (input_location,
> ifexp, 1,
> - ret_val, NULL_TREE,
> - ftree, NULL_TREE);
> + ret_val, NULL_TREE, input_location,
> + ftree, NULL_TREE, input_location);
> #endif
> }
> return ret_val;
> diff --git gcc/testsuite/gcc.dg/Wsign-compare-1.c gcc/testsuite/gcc.dg/Wsign-compare-1.c
> index e69de29bb2d..cd7fa1310b7 100644
> --- gcc/testsuite/gcc.dg/Wsign-compare-1.c
> +++ gcc/testsuite/gcc.dg/Wsign-compare-1.c
> @@ -0,0 +1,45 @@
> +/* PR c/81417 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wsign-compare" } */
> +
> +unsigned int
> +f1 (int x, unsigned int y)
> +{
> + return x ? y : -1; /* { dg-warning "18:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f2 (int x, unsigned int y)
> +{
> + return x ? -1 : y; /* { dg-warning "14:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f3 (unsigned int y)
> +{
> + return y ?: -1; /* { dg-warning "15:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f4 (int x, unsigned y, short u)
> +{
> + return x ? y : u; /* { dg-warning "18:operand of conditional expression changes signedness: 'short int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f5 (int x, unsigned y, short u)
> +{
> + return x ? u : y; /* { dg-warning "14:operand of conditional expression changes signedness: 'short int' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f6 (int x, unsigned y, signed char u)
> +{
> + return x ? y : u; /* { dg-warning "18:operand of conditional expression changes signedness: 'signed char' to 'unsigned int'" } */
> +}
> +
> +unsigned int
> +f7 (int x, unsigned y, signed char u)
> +{
> + return x ? u : y; /* { dg-warning "14:operand of conditional expression changes signedness: 'signed char' to 'unsigned int'" } */
> +}
> diff --git gcc/testsuite/gcc.dg/compare1.c gcc/testsuite/gcc.dg/compare1.c
> index 7becfbdb17f..ebab8c2cbf7 100644
> --- gcc/testsuite/gcc.dg/compare1.c
> +++ gcc/testsuite/gcc.dg/compare1.c
> @@ -22,17 +22,17 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
> }
>
> int i(enum mm2 x)
> diff --git gcc/testsuite/gcc.dg/compare2.c gcc/testsuite/gcc.dg/compare2.c
> index c309f1d00eb..cfadaccb8af 100644
> --- gcc/testsuite/gcc.dg/compare2.c
> +++ gcc/testsuite/gcc.dg/compare2.c
> @@ -9,35 +9,35 @@ int tf = 1;
> void f(int x, unsigned int y)
> {
> /* ?: branches are constants. */
> - x > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 1" } */
> - y > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 2" } */
> + x > (tf?64:128); /* { dg-bogus "changes signedness" "case 1" } */
> + y > (tf?64:128); /* { dg-bogus "changes signedness" "case 2" } */
>
> /* ?: branches are (recursively) constants. */
> - x > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 3" } */
> - y > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 4" } */
> + x > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 3" } */
> + y > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 4" } */
>
> /* ?: branches are signed constants. */
> - x > (tf?64:-1); /* { dg-bogus "signed and unsigned" "case 5" } */
> + x > (tf?64:-1); /* { dg-bogus "changes signedness" "case 5" } */
> y > (tf?64:-1); /* { dg-warning "different signedness" "case 6" } */
>
> /* ?: branches are (recursively) signed constants. */
> - x > (tf?64:(tf?128:-1)); /* { dg-bogus "signed and unsigned" "case 7" } */
> + x > (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case 7" } */
> y > (tf?64:(tf?128:-1)); /* { dg-warning "different signedness" "case 8" } */
>
> /* Statement expression. */
> - x > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 9" } */
> - y > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 10" } */
> + x > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 9" } */
> + y > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 10" } */
>
> /* Statement expression with recursive ?: . */
> - x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 11" } */
> - y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 12" } */
> + x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 11" } */
> + y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 12" } */
>
> /* Statement expression with signed ?:. */
> - x > ({tf; tf?64:-1;}); /* { dg-bogus "signed and unsigned" "case 13" } */
> + x > ({tf; tf?64:-1;}); /* { dg-bogus "changes signedness" "case 13" } */
> y > ({tf; tf?64:-1;}); /* { dg-warning "different signedness" "case 14" } */
>
> /* Statement expression with recursive signed ?:. */
> - x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "signed and unsigned" "case 15" } */
> + x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "changes signedness" "case 15" } */
> y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different signedness" "case 16" } */
>
> /* ?: branches are constants. */
> diff --git gcc/testsuite/gcc.dg/compare3.c gcc/testsuite/gcc.dg/compare3.c
> index eda3faf2754..836231fb870 100644
> --- gcc/testsuite/gcc.dg/compare3.c
> +++ gcc/testsuite/gcc.dg/compare3.c
> @@ -11,49 +11,49 @@ void f(int x, unsigned int y)
> /* Test comparing conditional expressions containing truth values.
> This can occur explicitly, or e.g. when (foo?2:(bar?1:0)) is
> optimized into (foo?2:(bar!=0)). */
> - x > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 1" } */
> - y > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 2" } */
> - x > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 3" } */
> - y > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 4" } */
> -
> - x > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 5" } */
> - y > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 6" } */
> - x > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 7" } */
> - y > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 8" } */
> -
> - x > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 9" } */
> - y > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 10" } */
> - x > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 11" } */
> - y > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 12" } */
> -
> - x < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 13" } */
> - y < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 14" } */
> - x < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 15" } */
> - y < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 16" } */
> -
> - x > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 17" } */
> - y > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 18" } */
> - x > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 19" } */
> - y > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 20" } */
> -
> - x > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 21" } */
> - y > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 22" } */
> - x > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 23" } */
> - y > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 24" } */
> -
> - x > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 25" } */
> - y > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 26" } */
> - x > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 27" } */
> - y > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 28" } */
> -
> - x > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 29" } */
> - y > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 30" } */
> - x > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 31" } */
> - y > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 32" } */
> -
> - x > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 33" } */
> - y > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 34" } */
> - x > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 35" } */
> - y > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 36" } */
> + x > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 1" } */
> + y > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 2" } */
> + x > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 3" } */
> + y > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 4" } */
> +
> + x > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 5" } */
> + y > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 6" } */
> + x > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 7" } */
> + y > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 8" } */
> +
> + x > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 9" } */
> + y > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 10" } */
> + x > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 11" } */
> + y > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 12" } */
> +
> + x < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 13" } */
> + y < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 14" } */
> + x < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 15" } */
> + y < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 16" } */
> +
> + x > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 17" } */
> + y > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 18" } */
> + x > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 19" } */
> + y > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 20" } */
> +
> + x > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 21" } */
> + y > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 22" } */
> + x > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 23" } */
> + y > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 24" } */
> +
> + x > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 25" } */
> + y > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 26" } */
> + x > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 27" } */
> + y > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 28" } */
> +
> + x > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 29" } */
> + y > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 30" } */
> + x > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 31" } */
> + y > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 32" } */
> +
> + x > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 33" } */
> + y > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 34" } */
> + x > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 35" } */
> + y > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 36" } */
>
> }
> diff --git gcc/testsuite/gcc.dg/compare7.c gcc/testsuite/gcc.dg/compare7.c
> index e2fbc04bfc2..b6fe6e78334 100644
> --- gcc/testsuite/gcc.dg/compare7.c
> +++ gcc/testsuite/gcc.dg/compare7.c
> @@ -6,5 +6,5 @@
>
> int f(unsigned a, int b)
> {
> - return a < b; /* { dg-bogus "signed and unsigned" } */
> + return a < b; /* { dg-bogus "changes signedness" } */
> }
> diff --git gcc/testsuite/gcc.dg/compare8.c gcc/testsuite/gcc.dg/compare8.c
> index d723c45a095..d09b69c53a2 100644
> --- gcc/testsuite/gcc.dg/compare8.c
> +++ gcc/testsuite/gcc.dg/compare8.c
> @@ -4,18 +4,18 @@
> int
> f(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
> + return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
> }
>
> int
> g(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
> + return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
> }
>
> int
> h(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
> + return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
> }
>
> diff --git gcc/testsuite/gcc.dg/compare9.c gcc/testsuite/gcc.dg/compare9.c
> index 02150cb1fb6..fba61e42a48 100644
> --- gcc/testsuite/gcc.dg/compare9.c
> +++ gcc/testsuite/gcc.dg/compare9.c
> @@ -22,20 +22,20 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
> }
>
> int i(enum mm2 x)
> {
> - return x == (tf?DI2:-1); /* { dg-bogus "signed and unsigned" "case 4" } */
> + return x == (tf?DI2:-1); /* { dg-bogus "changes signedness" "case 4" } */
> }
> diff --git gcc/testsuite/gcc.dg/pr11492.c gcc/testsuite/gcc.dg/pr11492.c
> index cf17712dde1..86435a83e79 100644
> --- gcc/testsuite/gcc.dg/pr11492.c
> +++ gcc/testsuite/gcc.dg/pr11492.c
> @@ -5,7 +5,7 @@ int main( void )
> {
> unsigned int a;
> unsigned char b;
> - for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison between signed and unsigned integer" } */
> + for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison of integer expressions of different signedness" } */
> { ; }
>
> return 0;
>
> Marek
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-07-31 15:54 ` Martin Sebor
@ 2017-07-31 16:05 ` Marek Polacek
2017-07-31 17:07 ` Martin Sebor
2017-08-01 20:18 ` David Malcolm
0 siblings, 2 replies; 14+ messages in thread
From: Marek Polacek @ 2017-07-31 16:05 UTC (permalink / raw)
To: Martin Sebor; +Cc: GCC Patches, Joseph Myers, David Malcolm
On Mon, Jul 31, 2017 at 09:54:03AM -0600, Martin Sebor wrote:
> On 07/31/2017 08:14 AM, Marek Polacek wrote:
> > This patch improves the diagnostic of -Wsign-compare for ?: by also printing
> > the types, similarly to my recent patch. But we can do even better here if we
> > actually point to the operand in question, so I passed the locations of the
> > operands from the parser. So instead of
> >
> > x.c:8:16: warning: signed and unsigned type in conditional expression [-Wsign-compare]
> > return x ? y : -1;
> > ^
> > you'll now see:
> >
> > x.c:8:18: warning: operand of conditional expression changes signedness: 'int' to 'unsigned int' [-Wsign-compare]
> > return x ? y : -1;
> > ^
>
> I like that this is more informative than the last warning you
> committed for this bug: it says what type the operand is converted
> to. The last one only shows what the types of the operands are but
> leaves users guessing as to what that might mean (integer promotion
> rules are often poorly understood). Where the last warning prints
>
> comparison of integer expressions of different signedness: ‘int’ and
> ‘unsigned int’
>
> it would be nice to go back and add this detail to it as well, and
> have it print something like this instead:
>
> comparison of integer expressions of different signedness changes type of
> the second operand from ‘int’ to ‘unsigned int’
>
> Where constant expressions are involved it would also be helpful
> to show the result of the conversion. For instance:
>
> comparison between ‘int’ and ‘unsigned int’ changes the value of the
> second operand from ‘-1’ to ‘4294967296’
Hmm, interesting. I could do that. How do other people feel about this?
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-07-31 16:05 ` Marek Polacek
@ 2017-07-31 17:07 ` Martin Sebor
2017-08-01 20:18 ` David Malcolm
1 sibling, 0 replies; 14+ messages in thread
From: Martin Sebor @ 2017-07-31 17:07 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers, David Malcolm
On 07/31/2017 10:05 AM, Marek Polacek wrote:
> On Mon, Jul 31, 2017 at 09:54:03AM -0600, Martin Sebor wrote:
>> On 07/31/2017 08:14 AM, Marek Polacek wrote:
>>> This patch improves the diagnostic of -Wsign-compare for ?: by also printing
>>> the types, similarly to my recent patch. But we can do even better here if we
>>> actually point to the operand in question, so I passed the locations of the
>>> operands from the parser. So instead of
>>>
>>> x.c:8:16: warning: signed and unsigned type in conditional expression [-Wsign-compare]
>>> return x ? y : -1;
>>> ^
>>> you'll now see:
>>>
>>> x.c:8:18: warning: operand of conditional expression changes signedness: 'int' to 'unsigned int' [-Wsign-compare]
>>> return x ? y : -1;
>>> ^
>>
>> I like that this is more informative than the last warning you
>> committed for this bug: it says what type the operand is converted
>> to. The last one only shows what the types of the operands are but
>> leaves users guessing as to what that might mean (integer promotion
>> rules are often poorly understood). Where the last warning prints
>>
>> comparison of integer expressions of different signedness: ‘int’ and
>> ‘unsigned int’
>>
>> it would be nice to go back and add this detail to it as well, and
>> have it print something like this instead:
>>
>> comparison of integer expressions of different signedness changes type of
>> the second operand from ‘int’ to ‘unsigned int’
>>
>> Where constant expressions are involved it would also be helpful
>> to show the result of the conversion. For instance:
>>
>> comparison between ‘int’ and ‘unsigned int’ changes the value of the
>> second operand from ‘-1’ to ‘4294967296’
>
> Hmm, interesting. I could do that. How do other people feel about this?
Since you ask for input from others, let me mention that I also
would find it helpful if we could come up with some sort of high
level direction on what information to include in diagnostics and
how to present it (e.g., a section on the GCC Diagnostic Guidelines
page on the Wiki). Not only would it help guide us in implementing
new diagnostics but it would also result in a more consistent and
uniform user experience.
For what it's worth, my own preference is to err on the side of
providing more detail rather than less so the changes I made in
r248431 to -Wconversion reflect that. So for example there, GCC
might now print:
unsigned conversion from ‘int‘ to ‘unsigned char‘ changes value from
‘-128‘ to ‘128‘"
My suggestions above were based on the style I chose here.
Martin
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-07-31 15:31 ` David Malcolm
@ 2017-08-01 14:16 ` Marek Polacek
2017-08-01 20:49 ` David Malcolm
0 siblings, 1 reply; 14+ messages in thread
From: Marek Polacek @ 2017-08-01 14:16 UTC (permalink / raw)
To: David Malcolm; +Cc: GCC Patches, Joseph Myers
On Mon, Jul 31, 2017 at 11:31:44AM -0400, David Malcolm wrote:
> On Mon, 2017-07-31 at 16:14 +0200, Marek Polacek wrote:
> > This patch improves the diagnostic of -Wsign-compare for ?: by also
> > printing
> > the types, similarly to my recent patch. But we can do even better
> > here if we
> > actually point to the operand in question, so I passed the locations
> > of the
> > operands from the parser.
>
> Thanks for updating the patch.
>
> > So instead of
> >
> > x.c:8:16: warning: signed and unsigned type in conditional expression
> > [-Wsign-compare]
> > return x ? y : -1;
> > ^
> > you'll now see:
> >
> > x.c:8:18: warning: operand of conditional expression changes
> > signedness: 'int' to 'unsigned int' [-Wsign-compare]
> > return x ? y : -1;
> > ^
>
> That's an improvement, but I would have expected it to underline the
> whole of the pertinent subexpression e.g.:
>
> return x ? y : -1;
> ^~
>
> rather than just:
>
> return x ? y : -1;
> ^
>
> From my reading of the patch, it's capturing just the location of the
> first token within the subexpression (hence e.g. just the minus token
> in the example above, which happens to make some sense for this case,
> but wouldn't in general).
Right.
> Hopefully you can get at the location_t for the whole of the
> subexpression using c_expr, rather than peeking at the first token.
It didn't seem obvious to me how to do that, but I've spent more time
on this now. I believe we need to use make_location (I've introduced
a new overload), as in the patch below. Or did you in mind anything
else?
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> The patch doesn't have a testcase for the location information; please
> add one, using -fdiagnostics-show-caret and dg-begin-multiline-output/
> dg-end-multiline-output. Please ensure that the pertinent expressions
> are more than one character wide, so that the test properly verifies
> the underlining.
Done.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-08-01 Marek Polacek <polacek@redhat.com>
PR c/81417
* c-array-notation.c (fix_builtin_array_notation_fn): Update calls to
build_conditional_expr.
* c-parser.c (c_parser_conditional_expression): Create locations for
EXP1 and EXP2 from their source ranges. Pass the locations down to
build_conditional_expr.
* c-tree.h (build_conditional_expr): Update declaration.
* c-typeck.c (build_conditional_expr): Add location_t parameters.
For -Wsign-compare, also print the types.
* input.c (make_location): New overload.
* input.h (make_location): Declare.
* objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Update
a call to build_conditional_expr.
* Wsign-compare-1.c: New test.
* gcc.dg/compare1.c: Adjust dg-bogus.
* gcc.dg/compare2.c: Likewise.
* gcc.dg/compare3.c: Likewise.
* gcc.dg/compare7.c: Likewise.
* gcc.dg/compare8.c: Likewise.
* gcc.dg/compare9.c: Likewise.
* gcc.dg/pr11492.c: Likewise.
diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
index e430f5c681b..40f1cfdabb8 100644
--- gcc/c/c-array-notation.c
+++ gcc/c/c-array-notation.c
@@ -355,8 +355,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
new_var_init = build_modify_expr
@@ -375,8 +376,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
new_var_init = build_modify_expr
@@ -394,8 +396,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
new_var_init = build_modify_expr
@@ -413,8 +416,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
if (TYPE_MIN_VALUE (new_var_type))
@@ -434,7 +438,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_expr = build_conditional_expr
(location,
build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
- new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+ new_yes_expr, TREE_TYPE (*new_var), location,
+ new_no_expr, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
if (TYPE_MAX_VALUE (new_var_type))
@@ -454,7 +459,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_expr = build_conditional_expr
(location,
build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
- new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+ new_yes_expr, TREE_TYPE (*new_var), location,
+ new_no_expr, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
new_var_init = build_modify_expr
@@ -504,7 +510,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
build2 (LE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
func_parm),
false,
- new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+ new_yes_list, TREE_TYPE (*new_var), location,
+ new_no_list, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
new_var_init = build_modify_expr
@@ -554,7 +561,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
build2 (GE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
func_parm),
false,
- new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+ new_yes_list, TREE_TYPE (*new_var), location,
+ new_no_list, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE:
new_var_init = build_modify_expr
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 16cd3579972..b64864f0d34 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -6508,7 +6508,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
tree omp_atomic_lhs)
{
struct c_expr cond, exp1, exp2, ret;
- location_t start, cond_loc, colon_loc, middle_loc;
+ location_t start, cond_loc, colon_loc;
gcc_assert (!after || c_dialect_objc ());
@@ -6527,7 +6527,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
{
tree eptype = NULL_TREE;
- middle_loc = c_parser_peek_token (parser)->location;
+ location_t middle_loc = c_parser_peek_token (parser)->location;
pedwarn (middle_loc, OPT_Wpedantic,
"ISO C forbids omitting the middle term of a ?: expression");
if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
@@ -6544,6 +6544,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
if (eptype)
exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype, exp1.value);
exp1.original_type = NULL;
+ exp1.src_range = cond.src_range;
cond.value = c_objc_common_truthvalue_conversion (cond_loc, exp1.value);
c_inhibit_evaluation_warnings += cond.value == truthvalue_true_node;
}
@@ -6575,10 +6576,12 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
}
c_inhibit_evaluation_warnings -= cond.value == truthvalue_true_node;
+ location_t loc1 = make_location (exp1.get_start (), exp1.src_range);
+ location_t loc2 = make_location (exp2.get_start (), exp2.src_range);
ret.value = build_conditional_expr (colon_loc, cond.value,
cond.original_code == C_MAYBE_CONST_EXPR,
- exp1.value, exp1.original_type,
- exp2.value, exp2.original_type);
+ exp1.value, exp1.original_type, loc1,
+ exp2.value, exp2.original_type, loc2);
ret.original_code = ERROR_MARK;
if (exp1.value == error_mark_node || exp2.value == error_mark_node)
ret.original_type = NULL;
diff --git gcc/c/c-tree.h gcc/c/c-tree.h
index a8197eb768d..be2f272d2dd 100644
--- gcc/c/c-tree.h
+++ gcc/c/c-tree.h
@@ -644,7 +644,7 @@ extern struct c_expr parser_build_binary_op (location_t,
enum tree_code, struct c_expr,
struct c_expr);
extern tree build_conditional_expr (location_t, tree, bool, tree, tree,
- tree, tree);
+ location_t, tree, tree, location_t);
extern tree build_compound_expr (location_t, tree, tree);
extern tree c_cast_expr (location_t, struct c_type_name *, tree);
extern tree build_c_cast (location_t, tree, tree);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 71d01350186..3f797377679 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4863,8 +4863,8 @@ ep_convert_and_check (location_t loc, tree type, tree expr,
tree
build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
- tree op1, tree op1_original_type, tree op2,
- tree op2_original_type)
+ tree op1, tree op1_original_type, location_t op1_loc,
+ tree op2, tree op2_original_type, location_t op2_loc)
{
tree type1;
tree type2;
@@ -5029,10 +5029,18 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
|| (unsigned_op1
&& tree_expr_nonnegative_warnv_p (op2, &ovf)))
/* OK */;
+ else if (unsigned_op2)
+ warning_at (op1_loc, OPT_Wsign_compare,
+ "operand of conditional expression "
+ "changes signedness: %qT to %qT",
+ TREE_TYPE (orig_op1),
+ TREE_TYPE (orig_op2));
else
- warning_at (colon_loc, OPT_Wsign_compare,
- ("signed and unsigned type in "
- "conditional expression"));
+ warning_at (op2_loc, OPT_Wsign_compare,
+ "operand of conditional expression "
+ "changes signedness: %qT to %qT",
+ TREE_TYPE (orig_op2),
+ TREE_TYPE (orig_op1));
}
if (!op1_maybe_const || TREE_CODE (op1) != INTEGER_CST)
op1 = c_wrap_maybe_const (op1, !op1_maybe_const);
diff --git gcc/input.c gcc/input.c
index 0480eb24ec0..a01c504fe57 100644
--- gcc/input.c
+++ gcc/input.c
@@ -898,6 +898,15 @@ make_location (location_t caret, location_t start, location_t finish)
return combined_loc;
}
+/* Same as above, but taking a source range rather than two locations. */
+
+location_t
+make_location (location_t caret, source_range src_range)
+{
+ location_t pure_loc = get_pure_location (caret);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+}
+
#define ONE_K 1024
#define ONE_M (ONE_K * ONE_K)
diff --git gcc/input.h gcc/input.h
index 7251eef664e..f58d2488342 100644
--- gcc/input.h
+++ gcc/input.h
@@ -109,6 +109,7 @@ get_finish (location_t loc)
extern location_t make_location (location_t caret,
location_t start, location_t finish);
+extern location_t make_location (location_t caret, source_range src_range);
void dump_line_table_statistics (void);
diff --git gcc/objc/objc-next-runtime-abi-02.c gcc/objc/objc-next-runtime-abi-02.c
index 97314860e01..a9c2f5d3ba5 100644
--- gcc/objc/objc-next-runtime-abi-02.c
+++ gcc/objc/objc-next-runtime-abi-02.c
@@ -1645,8 +1645,8 @@ build_v2_build_objc_method_call (int super_flag, tree method_prototype,
/* ??? CHECKME. */
ret_val = build_conditional_expr (input_location,
ifexp, 1,
- ret_val, NULL_TREE,
- ftree, NULL_TREE);
+ ret_val, NULL_TREE, input_location,
+ ftree, NULL_TREE, input_location);
#endif
}
return ret_val;
diff --git gcc/testsuite/gcc.dg/Wsign-compare-1.c gcc/testsuite/gcc.dg/Wsign-compare-1.c
index e69de29bb2d..d6fde20b68d 100644
--- gcc/testsuite/gcc.dg/Wsign-compare-1.c
+++ gcc/testsuite/gcc.dg/Wsign-compare-1.c
@@ -0,0 +1,83 @@
+/* PR c/81417 */
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare -fdiagnostics-show-caret" } */
+
+unsigned int
+f0 (int x, unsigned int y)
+{
+ return x ? y : -1; /* { dg-warning "18:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return x ? y : -1;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f1 (int xxx, unsigned int yyy)
+{
+ return xxx ? yyy : -1; /* { dg-warning "22:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? yyy : -1;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f2 (int xxx, unsigned int yyy)
+{
+ return xxx ? -1 : yyy; /* { dg-warning "16:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? -1 : yyy;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f3 (unsigned int yyy)
+{
+ return yyy ?: -1; /* { dg-warning "17:operand of conditional expression changes signedness: 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return yyy ?: -1;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f4 (int xxx, unsigned yyy, short uuu)
+{
+ return xxx ? yyy : uuu; /* { dg-warning "22:operand of conditional expression changes signedness: 'short int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? yyy : uuu;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f5 (int xxx, unsigned yyy, short uuu)
+{
+ return xxx ? uuu : yyy; /* { dg-warning "16:operand of conditional expression changes signedness: 'short int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? uuu : yyy;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f6 (int xxx, unsigned yyy, signed char uuu)
+{
+ return xxx ? yyy : uuu; /* { dg-warning "22:operand of conditional expression changes signedness: 'signed char' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? yyy : uuu;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f7 (int xxx, unsigned yyy, signed char uuu)
+{
+ return xxx ? uuu : yyy; /* { dg-warning "16:operand of conditional expression changes signedness: 'signed char' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? uuu : yyy;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
diff --git gcc/testsuite/gcc.dg/compare1.c gcc/testsuite/gcc.dg/compare1.c
index 7becfbdb17f..ebab8c2cbf7 100644
--- gcc/testsuite/gcc.dg/compare1.c
+++ gcc/testsuite/gcc.dg/compare1.c
@@ -22,17 +22,17 @@ enum mm2
int f(enum mm1 x)
{
- return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
+ return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
}
int g(enum mm1 x)
{
- return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
+ return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
}
int h(enum mm2 x)
{
- return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
+ return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
}
int i(enum mm2 x)
diff --git gcc/testsuite/gcc.dg/compare2.c gcc/testsuite/gcc.dg/compare2.c
index c309f1d00eb..cfadaccb8af 100644
--- gcc/testsuite/gcc.dg/compare2.c
+++ gcc/testsuite/gcc.dg/compare2.c
@@ -9,35 +9,35 @@ int tf = 1;
void f(int x, unsigned int y)
{
/* ?: branches are constants. */
- x > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 1" } */
- y > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 2" } */
+ x > (tf?64:128); /* { dg-bogus "changes signedness" "case 1" } */
+ y > (tf?64:128); /* { dg-bogus "changes signedness" "case 2" } */
/* ?: branches are (recursively) constants. */
- x > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 3" } */
- y > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 4" } */
+ x > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 3" } */
+ y > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 4" } */
/* ?: branches are signed constants. */
- x > (tf?64:-1); /* { dg-bogus "signed and unsigned" "case 5" } */
+ x > (tf?64:-1); /* { dg-bogus "changes signedness" "case 5" } */
y > (tf?64:-1); /* { dg-warning "different signedness" "case 6" } */
/* ?: branches are (recursively) signed constants. */
- x > (tf?64:(tf?128:-1)); /* { dg-bogus "signed and unsigned" "case 7" } */
+ x > (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case 7" } */
y > (tf?64:(tf?128:-1)); /* { dg-warning "different signedness" "case 8" } */
/* Statement expression. */
- x > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 9" } */
- y > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 10" } */
+ x > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 9" } */
+ y > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 10" } */
/* Statement expression with recursive ?: . */
- x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 11" } */
- y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 12" } */
+ x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 11" } */
+ y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 12" } */
/* Statement expression with signed ?:. */
- x > ({tf; tf?64:-1;}); /* { dg-bogus "signed and unsigned" "case 13" } */
+ x > ({tf; tf?64:-1;}); /* { dg-bogus "changes signedness" "case 13" } */
y > ({tf; tf?64:-1;}); /* { dg-warning "different signedness" "case 14" } */
/* Statement expression with recursive signed ?:. */
- x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "signed and unsigned" "case 15" } */
+ x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "changes signedness" "case 15" } */
y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different signedness" "case 16" } */
/* ?: branches are constants. */
diff --git gcc/testsuite/gcc.dg/compare3.c gcc/testsuite/gcc.dg/compare3.c
index eda3faf2754..836231fb870 100644
--- gcc/testsuite/gcc.dg/compare3.c
+++ gcc/testsuite/gcc.dg/compare3.c
@@ -11,49 +11,49 @@ void f(int x, unsigned int y)
/* Test comparing conditional expressions containing truth values.
This can occur explicitly, or e.g. when (foo?2:(bar?1:0)) is
optimized into (foo?2:(bar!=0)). */
- x > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 1" } */
- y > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 2" } */
- x > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 3" } */
- y > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 4" } */
-
- x > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 5" } */
- y > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 6" } */
- x > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 7" } */
- y > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 8" } */
-
- x > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 9" } */
- y > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 10" } */
- x > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 11" } */
- y > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 12" } */
-
- x < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 13" } */
- y < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 14" } */
- x < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 15" } */
- y < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 16" } */
-
- x > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 17" } */
- y > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 18" } */
- x > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 19" } */
- y > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 20" } */
-
- x > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 21" } */
- y > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 22" } */
- x > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 23" } */
- y > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 24" } */
-
- x > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 25" } */
- y > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 26" } */
- x > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 27" } */
- y > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 28" } */
-
- x > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 29" } */
- y > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 30" } */
- x > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 31" } */
- y > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 32" } */
-
- x > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 33" } */
- y > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 34" } */
- x > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 35" } */
- y > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 36" } */
+ x > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 1" } */
+ y > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 2" } */
+ x > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 3" } */
+ y > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 4" } */
+
+ x > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 5" } */
+ y > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 6" } */
+ x > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 7" } */
+ y > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 8" } */
+
+ x > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 9" } */
+ y > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 10" } */
+ x > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 11" } */
+ y > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 12" } */
+
+ x < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 13" } */
+ y < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 14" } */
+ x < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 15" } */
+ y < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 16" } */
+
+ x > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 17" } */
+ y > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 18" } */
+ x > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 19" } */
+ y > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 20" } */
+
+ x > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 21" } */
+ y > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 22" } */
+ x > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 23" } */
+ y > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 24" } */
+
+ x > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 25" } */
+ y > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 26" } */
+ x > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 27" } */
+ y > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 28" } */
+
+ x > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 29" } */
+ y > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 30" } */
+ x > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 31" } */
+ y > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 32" } */
+
+ x > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 33" } */
+ y > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 34" } */
+ x > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 35" } */
+ y > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 36" } */
}
diff --git gcc/testsuite/gcc.dg/compare7.c gcc/testsuite/gcc.dg/compare7.c
index e2fbc04bfc2..b6fe6e78334 100644
--- gcc/testsuite/gcc.dg/compare7.c
+++ gcc/testsuite/gcc.dg/compare7.c
@@ -6,5 +6,5 @@
int f(unsigned a, int b)
{
- return a < b; /* { dg-bogus "signed and unsigned" } */
+ return a < b; /* { dg-bogus "changes signedness" } */
}
diff --git gcc/testsuite/gcc.dg/compare8.c gcc/testsuite/gcc.dg/compare8.c
index d723c45a095..d09b69c53a2 100644
--- gcc/testsuite/gcc.dg/compare8.c
+++ gcc/testsuite/gcc.dg/compare8.c
@@ -4,18 +4,18 @@
int
f(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
int
g(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
int
h(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
diff --git gcc/testsuite/gcc.dg/compare9.c gcc/testsuite/gcc.dg/compare9.c
index 02150cb1fb6..fba61e42a48 100644
--- gcc/testsuite/gcc.dg/compare9.c
+++ gcc/testsuite/gcc.dg/compare9.c
@@ -22,20 +22,20 @@ enum mm2
int f(enum mm1 x)
{
- return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
+ return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
}
int g(enum mm1 x)
{
- return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
+ return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
}
int h(enum mm2 x)
{
- return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
+ return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
}
int i(enum mm2 x)
{
- return x == (tf?DI2:-1); /* { dg-bogus "signed and unsigned" "case 4" } */
+ return x == (tf?DI2:-1); /* { dg-bogus "changes signedness" "case 4" } */
}
diff --git gcc/testsuite/gcc.dg/pr11492.c gcc/testsuite/gcc.dg/pr11492.c
index cf17712dde1..86435a83e79 100644
--- gcc/testsuite/gcc.dg/pr11492.c
+++ gcc/testsuite/gcc.dg/pr11492.c
@@ -5,7 +5,7 @@ int main( void )
{
unsigned int a;
unsigned char b;
- for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison between signed and unsigned integer" } */
+ for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison of integer expressions of different signedness" } */
{ ; }
return 0;
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-07-31 16:05 ` Marek Polacek
2017-07-31 17:07 ` Martin Sebor
@ 2017-08-01 20:18 ` David Malcolm
1 sibling, 0 replies; 14+ messages in thread
From: David Malcolm @ 2017-08-01 20:18 UTC (permalink / raw)
To: Marek Polacek, Martin Sebor; +Cc: GCC Patches, Joseph Myers
On Mon, 2017-07-31 at 18:05 +0200, Marek Polacek wrote:
> On Mon, Jul 31, 2017 at 09:54:03AM -0600, Martin Sebor wrote:
> > On 07/31/2017 08:14 AM, Marek Polacek wrote:
> > > This patch improves the diagnostic of -Wsign-compare for ?: by
> > > also printing
> > > the types, similarly to my recent patch. But we can do even
> > > better here if we
> > > actually point to the operand in question, so I passed the
> > > locations of the
> > > operands from the parser. So instead of
> > >
> > > x.c:8:16: warning: signed and unsigned type in conditional
> > > expression [-Wsign-compare]
> > > return x ? y : -1;
> > > ^
> > > you'll now see:
> > >
> > > x.c:8:18: warning: operand of conditional expression changes
> > > signedness: 'int' to 'unsigned int' [-Wsign-compare]
> > > return x ? y : -1;
> > > ^
> >
> > I like that this is more informative than the last warning you
> > committed for this bug: it says what type the operand is converted
> > to. The last one only shows what the types of the operands are but
> > leaves users guessing as to what that might mean (integer promotion
> > rules are often poorly understood). Where the last warning prints
> >
> > comparison of integer expressions of different signedness: ‘int’
> > and
> > ‘unsigned int’
> >
> > it would be nice to go back and add this detail to it as well, and
> > have it print something like this instead:
> >
> > comparison of integer expressions of different signedness changes
> > type of
> > the second operand from ‘int’ to ‘unsigned int’
> >
> > Where constant expressions are involved it would also be helpful
> > to show the result of the conversion. For instance:
> >
> > comparison between ‘int’ and ‘unsigned int’ changes the value of
> > the
> > second operand from ‘-1’ to ‘4294967296’
>
> Hmm, interesting. I could do that. How do other people feel about
> this?
>
> Marek
I think that printing values is very helpful for convincing the user
that they need to listen to the compiler.
That said, we could do a better job of printing values (and boundary
values), especially for large values near a power of two; Martin filed
that as PR 80437; I've added some thoughts on that to that PR.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-08-01 14:16 ` Marek Polacek
@ 2017-08-01 20:49 ` David Malcolm
2017-08-02 2:36 ` Martin Sebor
2017-08-02 12:43 ` Marek Polacek
0 siblings, 2 replies; 14+ messages in thread
From: David Malcolm @ 2017-08-01 20:49 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers
On Tue, 2017-08-01 at 16:15 +0200, Marek Polacek wrote:
> On Mon, Jul 31, 2017 at 11:31:44AM -0400, David Malcolm wrote:
> > On Mon, 2017-07-31 at 16:14 +0200, Marek Polacek wrote:
> > > This patch improves the diagnostic of -Wsign-compare for ?: by
> > > also
> > > printing
> > > the types, similarly to my recent patch. But we can do even
> > > better
> > > here if we
> > > actually point to the operand in question, so I passed the
> > > locations
> > > of the
> > > operands from the parser.
> >
> > Thanks for updating the patch.
> >
> > > So instead of
> > >
> > > x.c:8:16: warning: signed and unsigned type in conditional
> > > expression
> > > [-Wsign-compare]
> > > return x ? y : -1;
> > > ^
> > > you'll now see:
> > >
> > > x.c:8:18: warning: operand of conditional expression changes
> > > signedness: 'int' to 'unsigned int' [-Wsign-compare]
> > > return x ? y : -1;
> > > ^
> >
> > That's an improvement, but I would have expected it to underline
> > the
> > whole of the pertinent subexpression e.g.:
> >
> > return x ? y : -1;
> > ^~
> >
> > rather than just:
> >
> > return x ? y : -1;
> > ^
> >
> > From my reading of the patch, it's capturing just the location of
> > the
> > first token within the subexpression (hence e.g. just the minus
> > token
> > in the example above, which happens to make some sense for this
> > case,
> > but wouldn't in general).
>
> Right.
>
> > Hopefully you can get at the location_t for the whole of the
> > subexpression using c_expr, rather than peeking at the first token.
>
> It didn't seem obvious to me how to do that, but I've spent more time
> on this now. I believe we need to use make_location (I've introduced
> a new overload), as in the patch below. Or did you in mind anything
> else?
I think I was confused with the C++ frontend, where cp_expr has a
location_t representing all three of (caret, start, finish), whereas in
the C frontend c_expr has a source_range, representing just (start,
finish).
New overload looks reasonable.
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >
> > The patch doesn't have a testcase for the location information;
> > please
> > add one, using -fdiagnostics-show-caret and dg-begin-multiline-
> > output/
> > dg-end-multiline-output. Please ensure that the pertinent
> > expressions
> > are more than one character wide, so that the test properly
> > verifies
> > the underlining.
>
> Done.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
Thanks for updating the patch.
I'm wondering if the messages could use a slight rewording, to give a
clue to the user about the reason *why* the expression has changed
signedness. The old message "signed and unsigned type in conditional
expression" gave the clue (but failed to underline the subexpression
changing sign, and tell what the old/new types were).
A horribly verbose way to put it would be something like:
"operand of conditional expression with mixed signedness changes
signedness from %qT to %qT due to promotion to unsigned to match
unsignedness of other operand" (ugh)
(assuming I'm understanding the logic correctly)
or something like:
"operand of conditional expression changes signedness from %qT to %qT
due to unsignedness of other operand"
or somesuch (am not 100% happy with that either).
Other than than, LGTM.
> 2017-08-01 Marek Polacek <polacek@redhat.com>
>
> PR c/81417
> * c-array-notation.c (fix_builtin_array_notation_fn): Update
> calls to
> build_conditional_expr.
> * c-parser.c (c_parser_conditional_expression): Create
> locations for
> EXP1 and EXP2 from their source ranges. Pass the locations
> down to
> build_conditional_expr.
> * c-tree.h (build_conditional_expr): Update declaration.
> * c-typeck.c (build_conditional_expr): Add location_t
> parameters.
> For -Wsign-compare, also print the types.
>
> * input.c (make_location): New overload.
> * input.h (make_location): Declare.
>
> * objc-next-runtime-abi-02.c (build_v2_build_objc_method_call):
> Update
> a call to build_conditional_expr.
>
> * Wsign-compare-1.c: New test.
> * gcc.dg/compare1.c: Adjust dg-bogus.
> * gcc.dg/compare2.c: Likewise.
> * gcc.dg/compare3.c: Likewise.
> * gcc.dg/compare7.c: Likewise.
> * gcc.dg/compare8.c: Likewise.
> * gcc.dg/compare9.c: Likewise.
> * gcc.dg/pr11492.c: Likewise.
>
> diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
> index e430f5c681b..40f1cfdabb8 100644
> --- gcc/c/c-array-notation.c
> +++ gcc/c/c-array-notation.c
> @@ -355,8 +355,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
> new_var_init = build_modify_expr
> @@ -375,8 +376,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
> new_var_init = build_modify_expr
> @@ -394,8 +396,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
> new_var_init = build_modify_expr
> @@ -413,8 +416,9 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm),
> func_parm,
> build_zero_cst (TREE_TYPE
> (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE
> (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
> if (TYPE_MIN_VALUE (new_var_type))
> @@ -434,7 +438,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var,
> func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE
> (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
> if (TYPE_MAX_VALUE (new_var_type))
> @@ -454,7 +459,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var,
> func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE
> (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
> new_var_init = build_modify_expr
> @@ -504,7 +510,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> build2 (LE_EXPR, TREE_TYPE (array_ind_value),
> array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE
> (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
> new_var_init = build_modify_expr
> @@ -554,7 +561,8 @@ fix_builtin_array_notation_fn (tree
> an_builtin_fn, tree *new_var)
> build2 (GE_EXPR, TREE_TYPE (array_ind_value),
> array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE
> (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE:
> new_var_init = build_modify_expr
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index 16cd3579972..b64864f0d34 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -6508,7 +6508,7 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> tree omp_atomic_lhs)
> {
> struct c_expr cond, exp1, exp2, ret;
> - location_t start, cond_loc, colon_loc, middle_loc;
> + location_t start, cond_loc, colon_loc;
>
> gcc_assert (!after || c_dialect_objc ());
>
> @@ -6527,7 +6527,7 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> {
> tree eptype = NULL_TREE;
>
> - middle_loc = c_parser_peek_token (parser)->location;
> + location_t middle_loc = c_parser_peek_token (parser)-
> >location;
> pedwarn (middle_loc, OPT_Wpedantic,
> "ISO C forbids omitting the middle term of a ?:
> expression");
> if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
> @@ -6544,6 +6544,7 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> if (eptype)
> exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype,
> exp1.value);
> exp1.original_type = NULL;
> + exp1.src_range = cond.src_range;
> cond.value = c_objc_common_truthvalue_conversion (cond_loc,
> exp1.value);
> c_inhibit_evaluation_warnings += cond.value ==
> truthvalue_true_node;
> }
> @@ -6575,10 +6576,12 @@ c_parser_conditional_expression (c_parser
> *parser, struct c_expr *after,
> exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
> }
> c_inhibit_evaluation_warnings -= cond.value ==
> truthvalue_true_node;
> + location_t loc1 = make_location (exp1.get_start (),
> exp1.src_range);
> + location_t loc2 = make_location (exp2.get_start (),
> exp2.src_range);
> ret.value = build_conditional_expr (colon_loc, cond.value,
> cond.original_code ==
> C_MAYBE_CONST_EXPR,
> - exp1.value,
> exp1.original_type,
> - exp2.value,
> exp2.original_type);
> + exp1.value,
> exp1.original_type, loc1,
> + exp2.value,
> exp2.original_type, loc2);
> ret.original_code = ERROR_MARK;
> if (exp1.value == error_mark_node || exp2.value ==
> error_mark_node)
> ret.original_type = NULL;
> diff --git gcc/c/c-tree.h gcc/c/c-tree.h
> index a8197eb768d..be2f272d2dd 100644
> --- gcc/c/c-tree.h
> +++ gcc/c/c-tree.h
> @@ -644,7 +644,7 @@ extern struct c_expr parser_build_binary_op
> (location_t,
> enum tree_code,
> struct c_expr,
> struct c_expr);
> extern tree build_conditional_expr (location_t, tree, bool, tree,
> tree,
> - tree, tree);
> + location_t, tree, tree,
> location_t);
> extern tree build_compound_expr (location_t, tree, tree);
> extern tree c_cast_expr (location_t, struct c_type_name *, tree);
> extern tree build_c_cast (location_t, tree, tree);
> diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> index 71d01350186..3f797377679 100644
> --- gcc/c/c-typeck.c
> +++ gcc/c/c-typeck.c
> @@ -4863,8 +4863,8 @@ ep_convert_and_check (location_t loc, tree
> type, tree expr,
>
> tree
> build_conditional_expr (location_t colon_loc, tree ifexp, bool
> ifexp_bcp,
> - tree op1, tree op1_original_type, tree op2,
> - tree op2_original_type)
> + tree op1, tree op1_original_type, location_t
> op1_loc,
> + tree op2, tree op2_original_type, location_t
> op2_loc)
> {
> tree type1;
> tree type2;
> @@ -5029,10 +5029,18 @@ build_conditional_expr (location_t colon_loc,
> tree ifexp, bool ifexp_bcp,
> || (unsigned_op1
> && tree_expr_nonnegative_warnv_p (op2,
> &ovf)))
> /* OK */;
> + else if (unsigned_op2)
> + warning_at (op1_loc, OPT_Wsign_compare,
> + "operand of conditional
> expression "
> + "changes signedness: %qT to
> %qT",
> + TREE_TYPE (orig_op1),
> + TREE_TYPE (orig_op2));
> else
> - warning_at (colon_loc, OPT_Wsign_compare,
> - ("signed and unsigned type in "
> - "conditional expression"));
> + warning_at (op2_loc, OPT_Wsign_compare,
> + "operand of conditional
> expression "
> + "changes signedness: %qT to
> %qT",
> + TREE_TYPE (orig_op2),
> + TREE_TYPE (orig_op1));
> }
> if (!op1_maybe_const || TREE_CODE (op1) !=
> INTEGER_CST)
> op1 = c_wrap_maybe_const (op1,
> !op1_maybe_const);
> diff --git gcc/input.c gcc/input.c
> index 0480eb24ec0..a01c504fe57 100644
> --- gcc/input.c
> +++ gcc/input.c
> @@ -898,6 +898,15 @@ make_location (location_t caret, location_t
> start, location_t finish)
> return combined_loc;
> }
>
> +/* Same as above, but taking a source range rather than two
> locations. */
> +
> +location_t
> +make_location (location_t caret, source_range src_range)
> +{
> + location_t pure_loc = get_pure_location (caret);
> + return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range,
> NULL);
> +}
> +
> #define ONE_K 1024
> #define ONE_M (ONE_K * ONE_K)
>
> diff --git gcc/input.h gcc/input.h
> index 7251eef664e..f58d2488342 100644
> --- gcc/input.h
> +++ gcc/input.h
> @@ -109,6 +109,7 @@ get_finish (location_t loc)
>
> extern location_t make_location (location_t caret,
> location_t start, location_t
> finish);
> +extern location_t make_location (location_t caret, source_range
> src_range);
>
> void dump_line_table_statistics (void);
>
> diff --git gcc/objc/objc-next-runtime-abi-02.c gcc/objc/objc-next-
> runtime-abi-02.c
> index 97314860e01..a9c2f5d3ba5 100644
> --- gcc/objc/objc-next-runtime-abi-02.c
> +++ gcc/objc/objc-next-runtime-abi-02.c
> @@ -1645,8 +1645,8 @@ build_v2_build_objc_method_call (int
> super_flag, tree method_prototype,
> /* ??? CHECKME. */
> ret_val = build_conditional_expr (input_location,
> ifexp, 1,
> - ret_val, NULL_TREE,
> - ftree, NULL_TREE);
> + ret_val, NULL_TREE,
> input_location,
> + ftree, NULL_TREE,
> input_location);
> #endif
> }
> return ret_val;
> diff --git gcc/testsuite/gcc.dg/Wsign-compare-1.c
> gcc/testsuite/gcc.dg/Wsign-compare-1.c
> index e69de29bb2d..d6fde20b68d 100644
> --- gcc/testsuite/gcc.dg/Wsign-compare-1.c
> +++ gcc/testsuite/gcc.dg/Wsign-compare-1.c
> @@ -0,0 +1,83 @@
> +/* PR c/81417 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wsign-compare -fdiagnostics-show-caret" } */
> +
> +unsigned int
> +f0 (int x, unsigned int y)
> +{
> + return x ? y : -1; /* { dg-warning "18:operand of conditional
> expression changes signedness: 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return x ? y : -1;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f1 (int xxx, unsigned int yyy)
> +{
> + return xxx ? yyy : -1; /* { dg-warning "22:operand of conditional
> expression changes signedness: 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? yyy : -1;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f2 (int xxx, unsigned int yyy)
> +{
> + return xxx ? -1 : yyy; /* { dg-warning "16:operand of conditional
> expression changes signedness: 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? -1 : yyy;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f3 (unsigned int yyy)
> +{
> + return yyy ?: -1; /* { dg-warning "17:operand of conditional
> expression changes signedness: 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return yyy ?: -1;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f4 (int xxx, unsigned yyy, short uuu)
> +{
> + return xxx ? yyy : uuu; /* { dg-warning "22:operand of conditional
> expression changes signedness: 'short int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? yyy : uuu;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f5 (int xxx, unsigned yyy, short uuu)
> +{
> + return xxx ? uuu : yyy; /* { dg-warning "16:operand of conditional
> expression changes signedness: 'short int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? uuu : yyy;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f6 (int xxx, unsigned yyy, signed char uuu)
> +{
> + return xxx ? yyy : uuu; /* { dg-warning "22:operand of conditional
> expression changes signedness: 'signed char' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? yyy : uuu;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f7 (int xxx, unsigned yyy, signed char uuu)
> +{
> + return xxx ? uuu : yyy; /* { dg-warning "16:operand of conditional
> expression changes signedness: 'signed char' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? uuu : yyy;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> diff --git gcc/testsuite/gcc.dg/compare1.c
> gcc/testsuite/gcc.dg/compare1.c
> index 7becfbdb17f..ebab8c2cbf7 100644
> --- gcc/testsuite/gcc.dg/compare1.c
> +++ gcc/testsuite/gcc.dg/compare1.c
> @@ -22,17 +22,17 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case
> 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case
> 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case
> 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case
> 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned"
> "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case
> 3" } */
> }
>
> int i(enum mm2 x)
> diff --git gcc/testsuite/gcc.dg/compare2.c
> gcc/testsuite/gcc.dg/compare2.c
> index c309f1d00eb..cfadaccb8af 100644
> --- gcc/testsuite/gcc.dg/compare2.c
> +++ gcc/testsuite/gcc.dg/compare2.c
> @@ -9,35 +9,35 @@ int tf = 1;
> void f(int x, unsigned int y)
> {
> /* ?: branches are constants. */
> - x > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 1" } */
> - y > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 2" } */
> + x > (tf?64:128); /* { dg-bogus "changes signedness" "case 1" } */
> + y > (tf?64:128); /* { dg-bogus "changes signedness" "case 2" } */
>
> /* ?: branches are (recursively) constants. */
> - x > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned"
> "case 3" } */
> - y > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned"
> "case 4" } */
> + x > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case
> 3" } */
> + y > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case
> 4" } */
>
> /* ?: branches are signed constants. */
> - x > (tf?64:-1); /* { dg-bogus "signed and unsigned" "case 5" } */
> + x > (tf?64:-1); /* { dg-bogus "changes signedness" "case 5" } */
> y > (tf?64:-1); /* { dg-warning "different signedness" "case 6" }
> */
>
> /* ?: branches are (recursively) signed constants. */
> - x > (tf?64:(tf?128:-1)); /* { dg-bogus "signed and unsigned" "case
> 7" } */
> + x > (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case
> 7" } */
> y > (tf?64:(tf?128:-1)); /* { dg-warning "different signedness"
> "case 8" } */
>
> /* Statement expression. */
> - x > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 9" } */
> - y > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 10" }
> */
> + x > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 9" } */
> + y > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 10" } */
>
> /* Statement expression with recursive ?: . */
> - x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and
> unsigned" "case 11" } */
> - y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and
> unsigned" "case 12" } */
> + x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes
> signedness" "case 11" } */
> + y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes
> signedness" "case 12" } */
>
> /* Statement expression with signed ?:. */
> - x > ({tf; tf?64:-1;}); /* { dg-bogus "signed and unsigned" "case
> 13" } */
> + x > ({tf; tf?64:-1;}); /* { dg-bogus "changes signedness" "case
> 13" } */
> y > ({tf; tf?64:-1;}); /* { dg-warning "different signedness"
> "case 14" } */
>
> /* Statement expression with recursive signed ?:. */
> - x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "signed and
> unsigned" "case 15" } */
> + x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "changes signedness"
> "case 15" } */
> y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different
> signedness" "case 16" } */
>
> /* ?: branches are constants. */
> diff --git gcc/testsuite/gcc.dg/compare3.c
> gcc/testsuite/gcc.dg/compare3.c
> index eda3faf2754..836231fb870 100644
> --- gcc/testsuite/gcc.dg/compare3.c
> +++ gcc/testsuite/gcc.dg/compare3.c
> @@ -11,49 +11,49 @@ void f(int x, unsigned int y)
> /* Test comparing conditional expressions containing truth values.
> This can occur explicitly, or e.g. when (foo?2:(bar?1:0)) is
> optimized into (foo?2:(bar!=0)). */
> - x > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 1"
> } */
> - y > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 2"
> } */
> - x > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 3"
> } */
> - y > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 4"
> } */
> -
> - x > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 5"
> } */
> - y > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 6"
> } */
> - x > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 7"
> } */
> - y > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 8"
> } */
> -
> - x > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 9" }
> */
> - y > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 10"
> } */
> - x > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 11"
> } */
> - y > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 12"
> } */
> -
> - x < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 13"
> } */
> - y < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 14"
> } */
> - x < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 15"
> } */
> - y < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 16"
> } */
> -
> - x > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 17"
> } */
> - y > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 18"
> } */
> - x > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 19"
> } */
> - y > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 20"
> } */
> -
> - x > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 21"
> } */
> - y > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 22"
> } */
> - x > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 23"
> } */
> - y > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 24"
> } */
> -
> - x > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 25"
> } */
> - y > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 26"
> } */
> - x > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 27"
> } */
> - y > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 28"
> } */
> -
> - x > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 29"
> } */
> - y > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 30"
> } */
> - x > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 31"
> } */
> - y > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 32"
> } */
> -
> - x > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 33" }
> */
> - y > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 34" }
> */
> - x > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 35" }
> */
> - y > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 36" }
> */
> + x > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 1" }
> */
> + y > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 2" }
> */
> + x > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 3" }
> */
> + y > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 4" }
> */
> +
> + x > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 5" }
> */
> + y > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 6" }
> */
> + x > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 7" }
> */
> + y > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 8" }
> */
> +
> + x > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 9" }
> */
> + y > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 10" }
> */
> + x > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 11" }
> */
> + y > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 12" }
> */
> +
> + x < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 13" }
> */
> + y < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 14" }
> */
> + x < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 15" }
> */
> + y < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 16" }
> */
> +
> + x > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 17"
> } */
> + y > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 18"
> } */
> + x > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 19"
> } */
> + y > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 20"
> } */
> +
> + x > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 21"
> } */
> + y > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 22"
> } */
> + x > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 23"
> } */
> + y > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 24"
> } */
> +
> + x > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 25"
> } */
> + y > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 26"
> } */
> + x > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 27"
> } */
> + y > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 28"
> } */
> +
> + x > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 29"
> } */
> + y > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 30"
> } */
> + x > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 31"
> } */
> + y > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 32"
> } */
> +
> + x > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 33" }
> */
> + y > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 34" }
> */
> + x > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 35" }
> */
> + y > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 36" }
> */
>
> }
> diff --git gcc/testsuite/gcc.dg/compare7.c
> gcc/testsuite/gcc.dg/compare7.c
> index e2fbc04bfc2..b6fe6e78334 100644
> --- gcc/testsuite/gcc.dg/compare7.c
> +++ gcc/testsuite/gcc.dg/compare7.c
> @@ -6,5 +6,5 @@
>
> int f(unsigned a, int b)
> {
> - return a < b; /* { dg-bogus "signed and unsigned" } */
> + return a < b; /* { dg-bogus "changes signedness" } */
> }
> diff --git gcc/testsuite/gcc.dg/compare8.c
> gcc/testsuite/gcc.dg/compare8.c
> index d723c45a095..d09b69c53a2 100644
> --- gcc/testsuite/gcc.dg/compare8.c
> +++ gcc/testsuite/gcc.dg/compare8.c
> @@ -4,18 +4,18 @@
> int
> f(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "signed and
> unsigned" } */
> + return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "changes
> signedness" } */
> }
>
> int
> g(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "signed and
> unsigned" } */
> + return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "changes
> signedness" } */
> }
>
> int
> h(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "signed and
> unsigned" } */
> + return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "changes
> signedness" } */
> }
>
> diff --git gcc/testsuite/gcc.dg/compare9.c
> gcc/testsuite/gcc.dg/compare9.c
> index 02150cb1fb6..fba61e42a48 100644
> --- gcc/testsuite/gcc.dg/compare9.c
> +++ gcc/testsuite/gcc.dg/compare9.c
> @@ -22,20 +22,20 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case
> 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case
> 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case
> 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case
> 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned"
> "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case
> 3" } */
> }
>
> int i(enum mm2 x)
> {
> - return x == (tf?DI2:-1); /* { dg-bogus "signed and unsigned" "case
> 4" } */
> + return x == (tf?DI2:-1); /* { dg-bogus "changes signedness" "case
> 4" } */
> }
> diff --git gcc/testsuite/gcc.dg/pr11492.c
> gcc/testsuite/gcc.dg/pr11492.c
> index cf17712dde1..86435a83e79 100644
> --- gcc/testsuite/gcc.dg/pr11492.c
> +++ gcc/testsuite/gcc.dg/pr11492.c
> @@ -5,7 +5,7 @@ int main( void )
> {
> unsigned int a;
> unsigned char b;
> - for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison
> between signed and unsigned integer" } */
> + for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison
> of integer expressions of different signedness" } */
> { ; }
>
> return 0;
> Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-08-01 20:49 ` David Malcolm
@ 2017-08-02 2:36 ` Martin Sebor
2017-08-02 12:43 ` Marek Polacek
1 sibling, 0 replies; 14+ messages in thread
From: Martin Sebor @ 2017-08-02 2:36 UTC (permalink / raw)
To: David Malcolm, Marek Polacek; +Cc: GCC Patches, Joseph Myers
>
> I'm wondering if the messages could use a slight rewording, to give a
> clue to the user about the reason *why* the expression has changed
> signedness. The old message "signed and unsigned type in conditional
> expression" gave the clue (but failed to underline the subexpression
> changing sign, and tell what the old/new types were).
>
> A horribly verbose way to put it would be something like:
>
> "operand of conditional expression with mixed signedness changes
> signedness from %qT to %qT due to promotion to unsigned to match
> unsignedness of other operand" (ugh)
>
> (assuming I'm understanding the logic correctly)
>
> or something like:
>
> "operand of conditional expression changes signedness from %qT to %qT
> due to unsignedness of other operand"
>
> or somesuch (am not 100% happy with that either).
If I can make an observation I'd say (since you're not happy with
the wordiness) that mentioning signedness in addition to the types
of the operands is superfluous: it should be sufficiently clear
from the "from 'T' to 'unsigned T'" part.
That being said, what might be helpful is mentioning which operand's
type changes: the second or third. That could be viewed as redundant
as well thanks to the underlining but only as long as the underlining
is correct and as long as it's not disabled by some option.
With that I'd suggest to consider the simple:
second operand of conditional expression changes type from %qT to %qT
Martin
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-08-01 20:49 ` David Malcolm
2017-08-02 2:36 ` Martin Sebor
@ 2017-08-02 12:43 ` Marek Polacek
2017-08-08 11:28 ` Marek Polacek
` (2 more replies)
1 sibling, 3 replies; 14+ messages in thread
From: Marek Polacek @ 2017-08-02 12:43 UTC (permalink / raw)
To: David Malcolm; +Cc: GCC Patches, Joseph Myers
On Tue, Aug 01, 2017 at 04:48:01PM -0400, David Malcolm wrote:
> I'm wondering if the messages could use a slight rewording, to give a
> clue to the user about the reason *why* the expression has changed
> signedness. The old message "signed and unsigned type in conditional
> expression" gave the clue (but failed to underline the subexpression
> changing sign, and tell what the old/new types were).
>
> A horribly verbose way to put it would be something like:
>
> "operand of conditional expression with mixed signedness changes
> signedness from %qT to %qT due to promotion to unsigned to match
> unsignedness of other operand" (ugh)
>
> (assuming I'm understanding the logic correctly)
>
> or something like:
>
> "operand of conditional expression changes signedness from %qT to %qT
> due to unsignedness of other operand"
>
> or somesuch (am not 100% happy with that either).
Hmm, how about this, then?
"operand of ?: changes signedness from %qT to %qT due to unsignedness of other operand"
I couldn't come up with anything more brief yet conveying all the information.
I don't like adding "second"/"third"/... very much; we should offer a good
location already.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-08-02 Marek Polacek <polacek@redhat.com>
PR c/81417
* c-array-notation.c (fix_builtin_array_notation_fn): Update calls to
build_conditional_expr.
* c-parser.c (c_parser_conditional_expression): Create locations for
EXP1 and EXP2 from their source ranges. Pass the locations down to
build_conditional_expr.
* c-tree.h (build_conditional_expr): Update declaration.
* c-typeck.c (build_conditional_expr): Add location_t parameters.
For -Wsign-compare, also print the types.
* input.c (make_location): New overload.
* input.h (make_location): Declare.
* objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Update
a call to build_conditional_expr.
* Wsign-compare-1.c: New test.
* gcc.dg/compare1.c: Adjust dg-bogus.
* gcc.dg/compare2.c: Likewise.
* gcc.dg/compare3.c: Likewise.
* gcc.dg/compare7.c: Likewise.
* gcc.dg/compare8.c: Likewise.
* gcc.dg/compare9.c: Likewise.
* gcc.dg/pr11492.c: Likewise.
diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
index e430f5c681b..40f1cfdabb8 100644
--- gcc/c/c-array-notation.c
+++ gcc/c/c-array-notation.c
@@ -355,8 +355,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
new_var_init = build_modify_expr
@@ -375,8 +376,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
new_var_init = build_modify_expr
@@ -394,8 +396,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
new_var_init = build_modify_expr
@@ -413,8 +416,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
build_zero_cst (TREE_TYPE (func_parm)));
new_expr = build_conditional_expr
- (location, new_cond_expr, false, new_yes_expr,
- TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
+ (location, new_cond_expr, false,
+ new_yes_expr, TREE_TYPE (new_yes_expr), location,
+ new_no_expr, TREE_TYPE (new_no_expr), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
if (TYPE_MIN_VALUE (new_var_type))
@@ -434,7 +438,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_expr = build_conditional_expr
(location,
build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
- new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+ new_yes_expr, TREE_TYPE (*new_var), location,
+ new_no_expr, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
if (TYPE_MAX_VALUE (new_var_type))
@@ -454,7 +459,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
new_expr = build_conditional_expr
(location,
build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
- new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+ new_yes_expr, TREE_TYPE (*new_var), location,
+ new_no_expr, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
new_var_init = build_modify_expr
@@ -504,7 +510,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
build2 (LE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
func_parm),
false,
- new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+ new_yes_list, TREE_TYPE (*new_var), location,
+ new_no_list, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
new_var_init = build_modify_expr
@@ -554,7 +561,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
build2 (GE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
func_parm),
false,
- new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+ new_yes_list, TREE_TYPE (*new_var), location,
+ new_no_list, TREE_TYPE (*new_var), location);
break;
case BUILT_IN_CILKPLUS_SEC_REDUCE:
new_var_init = build_modify_expr
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 16cd3579972..b64864f0d34 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -6508,7 +6508,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
tree omp_atomic_lhs)
{
struct c_expr cond, exp1, exp2, ret;
- location_t start, cond_loc, colon_loc, middle_loc;
+ location_t start, cond_loc, colon_loc;
gcc_assert (!after || c_dialect_objc ());
@@ -6527,7 +6527,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
{
tree eptype = NULL_TREE;
- middle_loc = c_parser_peek_token (parser)->location;
+ location_t middle_loc = c_parser_peek_token (parser)->location;
pedwarn (middle_loc, OPT_Wpedantic,
"ISO C forbids omitting the middle term of a ?: expression");
if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
@@ -6544,6 +6544,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
if (eptype)
exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype, exp1.value);
exp1.original_type = NULL;
+ exp1.src_range = cond.src_range;
cond.value = c_objc_common_truthvalue_conversion (cond_loc, exp1.value);
c_inhibit_evaluation_warnings += cond.value == truthvalue_true_node;
}
@@ -6575,10 +6576,12 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
}
c_inhibit_evaluation_warnings -= cond.value == truthvalue_true_node;
+ location_t loc1 = make_location (exp1.get_start (), exp1.src_range);
+ location_t loc2 = make_location (exp2.get_start (), exp2.src_range);
ret.value = build_conditional_expr (colon_loc, cond.value,
cond.original_code == C_MAYBE_CONST_EXPR,
- exp1.value, exp1.original_type,
- exp2.value, exp2.original_type);
+ exp1.value, exp1.original_type, loc1,
+ exp2.value, exp2.original_type, loc2);
ret.original_code = ERROR_MARK;
if (exp1.value == error_mark_node || exp2.value == error_mark_node)
ret.original_type = NULL;
diff --git gcc/c/c-tree.h gcc/c/c-tree.h
index a8197eb768d..be2f272d2dd 100644
--- gcc/c/c-tree.h
+++ gcc/c/c-tree.h
@@ -644,7 +644,7 @@ extern struct c_expr parser_build_binary_op (location_t,
enum tree_code, struct c_expr,
struct c_expr);
extern tree build_conditional_expr (location_t, tree, bool, tree, tree,
- tree, tree);
+ location_t, tree, tree, location_t);
extern tree build_compound_expr (location_t, tree, tree);
extern tree c_cast_expr (location_t, struct c_type_name *, tree);
extern tree build_c_cast (location_t, tree, tree);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 71d01350186..7c02ad1cdbd 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4863,8 +4863,8 @@ ep_convert_and_check (location_t loc, tree type, tree expr,
tree
build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
- tree op1, tree op1_original_type, tree op2,
- tree op2_original_type)
+ tree op1, tree op1_original_type, location_t op1_loc,
+ tree op2, tree op2_original_type, location_t op2_loc)
{
tree type1;
tree type2;
@@ -5029,10 +5029,18 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
|| (unsigned_op1
&& tree_expr_nonnegative_warnv_p (op2, &ovf)))
/* OK */;
+ else if (unsigned_op2)
+ warning_at (op1_loc, OPT_Wsign_compare,
+ "operand of ?: changes signedness from "
+ "%qT to %qT due to unsignedness of other "
+ "operand", TREE_TYPE (orig_op1),
+ TREE_TYPE (orig_op2));
else
- warning_at (colon_loc, OPT_Wsign_compare,
- ("signed and unsigned type in "
- "conditional expression"));
+ warning_at (op2_loc, OPT_Wsign_compare,
+ "operand of ?: changes signedness from "
+ "%qT to %qT due to unsignedness of other "
+ "operand", TREE_TYPE (orig_op2),
+ TREE_TYPE (orig_op1));
}
if (!op1_maybe_const || TREE_CODE (op1) != INTEGER_CST)
op1 = c_wrap_maybe_const (op1, !op1_maybe_const);
diff --git gcc/input.c gcc/input.c
index 0480eb24ec0..a01c504fe57 100644
--- gcc/input.c
+++ gcc/input.c
@@ -898,6 +898,15 @@ make_location (location_t caret, location_t start, location_t finish)
return combined_loc;
}
+/* Same as above, but taking a source range rather than two locations. */
+
+location_t
+make_location (location_t caret, source_range src_range)
+{
+ location_t pure_loc = get_pure_location (caret);
+ return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
+}
+
#define ONE_K 1024
#define ONE_M (ONE_K * ONE_K)
diff --git gcc/input.h gcc/input.h
index 7251eef664e..f58d2488342 100644
--- gcc/input.h
+++ gcc/input.h
@@ -109,6 +109,7 @@ get_finish (location_t loc)
extern location_t make_location (location_t caret,
location_t start, location_t finish);
+extern location_t make_location (location_t caret, source_range src_range);
void dump_line_table_statistics (void);
diff --git gcc/objc/objc-next-runtime-abi-02.c gcc/objc/objc-next-runtime-abi-02.c
index 97314860e01..a9c2f5d3ba5 100644
--- gcc/objc/objc-next-runtime-abi-02.c
+++ gcc/objc/objc-next-runtime-abi-02.c
@@ -1645,8 +1645,8 @@ build_v2_build_objc_method_call (int super_flag, tree method_prototype,
/* ??? CHECKME. */
ret_val = build_conditional_expr (input_location,
ifexp, 1,
- ret_val, NULL_TREE,
- ftree, NULL_TREE);
+ ret_val, NULL_TREE, input_location,
+ ftree, NULL_TREE, input_location);
#endif
}
return ret_val;
diff --git gcc/testsuite/gcc.dg/Wsign-compare-1.c gcc/testsuite/gcc.dg/Wsign-compare-1.c
index e69de29bb2d..be3bd2fcbd8 100644
--- gcc/testsuite/gcc.dg/Wsign-compare-1.c
+++ gcc/testsuite/gcc.dg/Wsign-compare-1.c
@@ -0,0 +1,83 @@
+/* PR c/81417 */
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare -fdiagnostics-show-caret" } */
+
+unsigned int
+f0 (int x, unsigned int y)
+{
+ return x ? y : -1; /* { dg-warning "18:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return x ? y : -1;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f1 (int xxx, unsigned int yyy)
+{
+ return xxx ? yyy : -1; /* { dg-warning "22:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? yyy : -1;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f2 (int xxx, unsigned int yyy)
+{
+ return xxx ? -1 : yyy; /* { dg-warning "16:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? -1 : yyy;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f3 (unsigned int yyy)
+{
+ return yyy ?: -1; /* { dg-warning "17:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return yyy ?: -1;
+ ^~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f4 (int xxx, unsigned yyy, short uuu)
+{
+ return xxx ? yyy : uuu; /* { dg-warning "22:operand of \\?: changes signedness from 'short int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? yyy : uuu;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f5 (int xxx, unsigned yyy, short uuu)
+{
+ return xxx ? uuu : yyy; /* { dg-warning "16:operand of \\?: changes signedness from 'short int' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? uuu : yyy;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f6 (int xxx, unsigned yyy, signed char uuu)
+{
+ return xxx ? yyy : uuu; /* { dg-warning "22:operand of \\?: changes signedness from 'signed char' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? yyy : uuu;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
+
+unsigned int
+f7 (int xxx, unsigned yyy, signed char uuu)
+{
+ return xxx ? uuu : yyy; /* { dg-warning "16:operand of \\?: changes signedness from 'signed char' to 'unsigned int'" } */
+/* { dg-begin-multiline-output "" }
+ return xxx ? uuu : yyy;
+ ^~~
+ { dg-end-multiline-output "" } */
+}
diff --git gcc/testsuite/gcc.dg/compare1.c gcc/testsuite/gcc.dg/compare1.c
index 7becfbdb17f..ebab8c2cbf7 100644
--- gcc/testsuite/gcc.dg/compare1.c
+++ gcc/testsuite/gcc.dg/compare1.c
@@ -22,17 +22,17 @@ enum mm2
int f(enum mm1 x)
{
- return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
+ return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
}
int g(enum mm1 x)
{
- return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
+ return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
}
int h(enum mm2 x)
{
- return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
+ return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
}
int i(enum mm2 x)
diff --git gcc/testsuite/gcc.dg/compare2.c gcc/testsuite/gcc.dg/compare2.c
index c309f1d00eb..cfadaccb8af 100644
--- gcc/testsuite/gcc.dg/compare2.c
+++ gcc/testsuite/gcc.dg/compare2.c
@@ -9,35 +9,35 @@ int tf = 1;
void f(int x, unsigned int y)
{
/* ?: branches are constants. */
- x > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 1" } */
- y > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 2" } */
+ x > (tf?64:128); /* { dg-bogus "changes signedness" "case 1" } */
+ y > (tf?64:128); /* { dg-bogus "changes signedness" "case 2" } */
/* ?: branches are (recursively) constants. */
- x > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 3" } */
- y > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 4" } */
+ x > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 3" } */
+ y > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 4" } */
/* ?: branches are signed constants. */
- x > (tf?64:-1); /* { dg-bogus "signed and unsigned" "case 5" } */
+ x > (tf?64:-1); /* { dg-bogus "changes signedness" "case 5" } */
y > (tf?64:-1); /* { dg-warning "different signedness" "case 6" } */
/* ?: branches are (recursively) signed constants. */
- x > (tf?64:(tf?128:-1)); /* { dg-bogus "signed and unsigned" "case 7" } */
+ x > (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case 7" } */
y > (tf?64:(tf?128:-1)); /* { dg-warning "different signedness" "case 8" } */
/* Statement expression. */
- x > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 9" } */
- y > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 10" } */
+ x > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 9" } */
+ y > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 10" } */
/* Statement expression with recursive ?: . */
- x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 11" } */
- y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 12" } */
+ x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 11" } */
+ y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 12" } */
/* Statement expression with signed ?:. */
- x > ({tf; tf?64:-1;}); /* { dg-bogus "signed and unsigned" "case 13" } */
+ x > ({tf; tf?64:-1;}); /* { dg-bogus "changes signedness" "case 13" } */
y > ({tf; tf?64:-1;}); /* { dg-warning "different signedness" "case 14" } */
/* Statement expression with recursive signed ?:. */
- x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "signed and unsigned" "case 15" } */
+ x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "changes signedness" "case 15" } */
y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different signedness" "case 16" } */
/* ?: branches are constants. */
diff --git gcc/testsuite/gcc.dg/compare3.c gcc/testsuite/gcc.dg/compare3.c
index eda3faf2754..836231fb870 100644
--- gcc/testsuite/gcc.dg/compare3.c
+++ gcc/testsuite/gcc.dg/compare3.c
@@ -11,49 +11,49 @@ void f(int x, unsigned int y)
/* Test comparing conditional expressions containing truth values.
This can occur explicitly, or e.g. when (foo?2:(bar?1:0)) is
optimized into (foo?2:(bar!=0)). */
- x > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 1" } */
- y > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 2" } */
- x > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 3" } */
- y > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 4" } */
-
- x > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 5" } */
- y > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 6" } */
- x > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 7" } */
- y > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 8" } */
-
- x > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 9" } */
- y > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 10" } */
- x > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 11" } */
- y > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 12" } */
-
- x < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 13" } */
- y < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 14" } */
- x < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 15" } */
- y < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 16" } */
-
- x > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 17" } */
- y > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 18" } */
- x > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 19" } */
- y > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 20" } */
-
- x > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 21" } */
- y > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 22" } */
- x > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 23" } */
- y > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 24" } */
-
- x > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 25" } */
- y > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 26" } */
- x > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 27" } */
- y > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 28" } */
-
- x > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 29" } */
- y > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 30" } */
- x > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 31" } */
- y > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 32" } */
-
- x > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 33" } */
- y > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 34" } */
- x > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 35" } */
- y > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 36" } */
+ x > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 1" } */
+ y > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 2" } */
+ x > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 3" } */
+ y > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 4" } */
+
+ x > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 5" } */
+ y > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 6" } */
+ x > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 7" } */
+ y > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 8" } */
+
+ x > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 9" } */
+ y > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 10" } */
+ x > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 11" } */
+ y > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 12" } */
+
+ x < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 13" } */
+ y < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 14" } */
+ x < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 15" } */
+ y < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 16" } */
+
+ x > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 17" } */
+ y > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 18" } */
+ x > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 19" } */
+ y > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 20" } */
+
+ x > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 21" } */
+ y > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 22" } */
+ x > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 23" } */
+ y > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 24" } */
+
+ x > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 25" } */
+ y > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 26" } */
+ x > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 27" } */
+ y > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 28" } */
+
+ x > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 29" } */
+ y > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 30" } */
+ x > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 31" } */
+ y > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 32" } */
+
+ x > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 33" } */
+ y > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 34" } */
+ x > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 35" } */
+ y > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 36" } */
}
diff --git gcc/testsuite/gcc.dg/compare7.c gcc/testsuite/gcc.dg/compare7.c
index e2fbc04bfc2..b6fe6e78334 100644
--- gcc/testsuite/gcc.dg/compare7.c
+++ gcc/testsuite/gcc.dg/compare7.c
@@ -6,5 +6,5 @@
int f(unsigned a, int b)
{
- return a < b; /* { dg-bogus "signed and unsigned" } */
+ return a < b; /* { dg-bogus "changes signedness" } */
}
diff --git gcc/testsuite/gcc.dg/compare8.c gcc/testsuite/gcc.dg/compare8.c
index d723c45a095..d09b69c53a2 100644
--- gcc/testsuite/gcc.dg/compare8.c
+++ gcc/testsuite/gcc.dg/compare8.c
@@ -4,18 +4,18 @@
int
f(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
int
g(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
int
h(unsigned short a1, unsigned short a2, unsigned int b)
{
- return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
+ return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
}
diff --git gcc/testsuite/gcc.dg/compare9.c gcc/testsuite/gcc.dg/compare9.c
index 02150cb1fb6..fba61e42a48 100644
--- gcc/testsuite/gcc.dg/compare9.c
+++ gcc/testsuite/gcc.dg/compare9.c
@@ -22,20 +22,20 @@ enum mm2
int f(enum mm1 x)
{
- return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
+ return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
}
int g(enum mm1 x)
{
- return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
+ return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
}
int h(enum mm2 x)
{
- return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
+ return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
}
int i(enum mm2 x)
{
- return x == (tf?DI2:-1); /* { dg-bogus "signed and unsigned" "case 4" } */
+ return x == (tf?DI2:-1); /* { dg-bogus "changes signedness" "case 4" } */
}
diff --git gcc/testsuite/gcc.dg/pr11492.c gcc/testsuite/gcc.dg/pr11492.c
index cf17712dde1..86435a83e79 100644
--- gcc/testsuite/gcc.dg/pr11492.c
+++ gcc/testsuite/gcc.dg/pr11492.c
@@ -5,7 +5,7 @@ int main( void )
{
unsigned int a;
unsigned char b;
- for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison between signed and unsigned integer" } */
+ for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison of integer expressions of different signedness" } */
{ ; }
return 0;
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-08-02 12:43 ` Marek Polacek
@ 2017-08-08 11:28 ` Marek Polacek
2017-08-08 21:48 ` Joseph Myers
2017-08-10 7:50 ` Andreas Schwab
2 siblings, 0 replies; 14+ messages in thread
From: Marek Polacek @ 2017-08-08 11:28 UTC (permalink / raw)
To: David Malcolm; +Cc: GCC Patches, Joseph Myers
Ping.
The make_location change should allow us to do really cool things, if we
pass c_exprs to build_binary_loc, btw. Like all the ranges for diagnostic
and similar.
On Wed, Aug 02, 2017 at 02:43:39PM +0200, Marek Polacek wrote:
> On Tue, Aug 01, 2017 at 04:48:01PM -0400, David Malcolm wrote:
> > I'm wondering if the messages could use a slight rewording, to give a
> > clue to the user about the reason *why* the expression has changed
> > signedness. The old message "signed and unsigned type in conditional
> > expression" gave the clue (but failed to underline the subexpression
> > changing sign, and tell what the old/new types were).
> >
> > A horribly verbose way to put it would be something like:
> >
> > "operand of conditional expression with mixed signedness changes
> > signedness from %qT to %qT due to promotion to unsigned to match
> > unsignedness of other operand" (ugh)
> >
> > (assuming I'm understanding the logic correctly)
> >
> > or something like:
> >
> > "operand of conditional expression changes signedness from %qT to %qT
> > due to unsignedness of other operand"
> >
> > or somesuch (am not 100% happy with that either).
>
> Hmm, how about this, then?
>
> "operand of ?: changes signedness from %qT to %qT due to unsignedness of other operand"
>
> I couldn't come up with anything more brief yet conveying all the information.
> I don't like adding "second"/"third"/... very much; we should offer a good
> location already.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-08-02 Marek Polacek <polacek@redhat.com>
>
> PR c/81417
> * c-array-notation.c (fix_builtin_array_notation_fn): Update calls to
> build_conditional_expr.
> * c-parser.c (c_parser_conditional_expression): Create locations for
> EXP1 and EXP2 from their source ranges. Pass the locations down to
> build_conditional_expr.
> * c-tree.h (build_conditional_expr): Update declaration.
> * c-typeck.c (build_conditional_expr): Add location_t parameters.
> For -Wsign-compare, also print the types.
>
> * input.c (make_location): New overload.
> * input.h (make_location): Declare.
>
> * objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Update
> a call to build_conditional_expr.
>
> * Wsign-compare-1.c: New test.
> * gcc.dg/compare1.c: Adjust dg-bogus.
> * gcc.dg/compare2.c: Likewise.
> * gcc.dg/compare3.c: Likewise.
> * gcc.dg/compare7.c: Likewise.
> * gcc.dg/compare8.c: Likewise.
> * gcc.dg/compare9.c: Likewise.
> * gcc.dg/pr11492.c: Likewise.
>
> diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
> index e430f5c681b..40f1cfdabb8 100644
> --- gcc/c/c-array-notation.c
> +++ gcc/c/c-array-notation.c
> @@ -355,8 +355,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ALL_NONZERO:
> new_var_init = build_modify_expr
> @@ -375,8 +376,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_ZERO:
> new_var_init = build_modify_expr
> @@ -394,8 +396,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (EQ_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_ANY_NONZERO:
> new_var_init = build_modify_expr
> @@ -413,8 +416,9 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_cond_expr = build2 (NE_EXPR, TREE_TYPE (func_parm), func_parm,
> build_zero_cst (TREE_TYPE (func_parm)));
> new_expr = build_conditional_expr
> - (location, new_cond_expr, false, new_yes_expr,
> - TREE_TYPE (new_yes_expr), new_no_expr, TREE_TYPE (new_no_expr));
> + (location, new_cond_expr, false,
> + new_yes_expr, TREE_TYPE (new_yes_expr), location,
> + new_no_expr, TREE_TYPE (new_no_expr), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
> if (TYPE_MIN_VALUE (new_var_type))
> @@ -434,7 +438,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
> if (TYPE_MAX_VALUE (new_var_type))
> @@ -454,7 +459,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> new_expr = build_conditional_expr
> (location,
> build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
> - new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
> + new_yes_expr, TREE_TYPE (*new_var), location,
> + new_no_expr, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
> new_var_init = build_modify_expr
> @@ -504,7 +510,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> build2 (LE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
> new_var_init = build_modify_expr
> @@ -554,7 +561,8 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
> build2 (GE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
> func_parm),
> false,
> - new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
> + new_yes_list, TREE_TYPE (*new_var), location,
> + new_no_list, TREE_TYPE (*new_var), location);
> break;
> case BUILT_IN_CILKPLUS_SEC_REDUCE:
> new_var_init = build_modify_expr
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index 16cd3579972..b64864f0d34 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -6508,7 +6508,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> tree omp_atomic_lhs)
> {
> struct c_expr cond, exp1, exp2, ret;
> - location_t start, cond_loc, colon_loc, middle_loc;
> + location_t start, cond_loc, colon_loc;
>
> gcc_assert (!after || c_dialect_objc ());
>
> @@ -6527,7 +6527,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> {
> tree eptype = NULL_TREE;
>
> - middle_loc = c_parser_peek_token (parser)->location;
> + location_t middle_loc = c_parser_peek_token (parser)->location;
> pedwarn (middle_loc, OPT_Wpedantic,
> "ISO C forbids omitting the middle term of a ?: expression");
> if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
> @@ -6544,6 +6544,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> if (eptype)
> exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype, exp1.value);
> exp1.original_type = NULL;
> + exp1.src_range = cond.src_range;
> cond.value = c_objc_common_truthvalue_conversion (cond_loc, exp1.value);
> c_inhibit_evaluation_warnings += cond.value == truthvalue_true_node;
> }
> @@ -6575,10 +6576,12 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
> exp2 = convert_lvalue_to_rvalue (exp2_loc, exp2, true, true);
> }
> c_inhibit_evaluation_warnings -= cond.value == truthvalue_true_node;
> + location_t loc1 = make_location (exp1.get_start (), exp1.src_range);
> + location_t loc2 = make_location (exp2.get_start (), exp2.src_range);
> ret.value = build_conditional_expr (colon_loc, cond.value,
> cond.original_code == C_MAYBE_CONST_EXPR,
> - exp1.value, exp1.original_type,
> - exp2.value, exp2.original_type);
> + exp1.value, exp1.original_type, loc1,
> + exp2.value, exp2.original_type, loc2);
> ret.original_code = ERROR_MARK;
> if (exp1.value == error_mark_node || exp2.value == error_mark_node)
> ret.original_type = NULL;
> diff --git gcc/c/c-tree.h gcc/c/c-tree.h
> index a8197eb768d..be2f272d2dd 100644
> --- gcc/c/c-tree.h
> +++ gcc/c/c-tree.h
> @@ -644,7 +644,7 @@ extern struct c_expr parser_build_binary_op (location_t,
> enum tree_code, struct c_expr,
> struct c_expr);
> extern tree build_conditional_expr (location_t, tree, bool, tree, tree,
> - tree, tree);
> + location_t, tree, tree, location_t);
> extern tree build_compound_expr (location_t, tree, tree);
> extern tree c_cast_expr (location_t, struct c_type_name *, tree);
> extern tree build_c_cast (location_t, tree, tree);
> diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> index 71d01350186..7c02ad1cdbd 100644
> --- gcc/c/c-typeck.c
> +++ gcc/c/c-typeck.c
> @@ -4863,8 +4863,8 @@ ep_convert_and_check (location_t loc, tree type, tree expr,
>
> tree
> build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
> - tree op1, tree op1_original_type, tree op2,
> - tree op2_original_type)
> + tree op1, tree op1_original_type, location_t op1_loc,
> + tree op2, tree op2_original_type, location_t op2_loc)
> {
> tree type1;
> tree type2;
> @@ -5029,10 +5029,18 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
> || (unsigned_op1
> && tree_expr_nonnegative_warnv_p (op2, &ovf)))
> /* OK */;
> + else if (unsigned_op2)
> + warning_at (op1_loc, OPT_Wsign_compare,
> + "operand of ?: changes signedness from "
> + "%qT to %qT due to unsignedness of other "
> + "operand", TREE_TYPE (orig_op1),
> + TREE_TYPE (orig_op2));
> else
> - warning_at (colon_loc, OPT_Wsign_compare,
> - ("signed and unsigned type in "
> - "conditional expression"));
> + warning_at (op2_loc, OPT_Wsign_compare,
> + "operand of ?: changes signedness from "
> + "%qT to %qT due to unsignedness of other "
> + "operand", TREE_TYPE (orig_op2),
> + TREE_TYPE (orig_op1));
> }
> if (!op1_maybe_const || TREE_CODE (op1) != INTEGER_CST)
> op1 = c_wrap_maybe_const (op1, !op1_maybe_const);
> diff --git gcc/input.c gcc/input.c
> index 0480eb24ec0..a01c504fe57 100644
> --- gcc/input.c
> +++ gcc/input.c
> @@ -898,6 +898,15 @@ make_location (location_t caret, location_t start, location_t finish)
> return combined_loc;
> }
>
> +/* Same as above, but taking a source range rather than two locations. */
> +
> +location_t
> +make_location (location_t caret, source_range src_range)
> +{
> + location_t pure_loc = get_pure_location (caret);
> + return COMBINE_LOCATION_DATA (line_table, pure_loc, src_range, NULL);
> +}
> +
> #define ONE_K 1024
> #define ONE_M (ONE_K * ONE_K)
>
> diff --git gcc/input.h gcc/input.h
> index 7251eef664e..f58d2488342 100644
> --- gcc/input.h
> +++ gcc/input.h
> @@ -109,6 +109,7 @@ get_finish (location_t loc)
>
> extern location_t make_location (location_t caret,
> location_t start, location_t finish);
> +extern location_t make_location (location_t caret, source_range src_range);
>
> void dump_line_table_statistics (void);
>
> diff --git gcc/objc/objc-next-runtime-abi-02.c gcc/objc/objc-next-runtime-abi-02.c
> index 97314860e01..a9c2f5d3ba5 100644
> --- gcc/objc/objc-next-runtime-abi-02.c
> +++ gcc/objc/objc-next-runtime-abi-02.c
> @@ -1645,8 +1645,8 @@ build_v2_build_objc_method_call (int super_flag, tree method_prototype,
> /* ??? CHECKME. */
> ret_val = build_conditional_expr (input_location,
> ifexp, 1,
> - ret_val, NULL_TREE,
> - ftree, NULL_TREE);
> + ret_val, NULL_TREE, input_location,
> + ftree, NULL_TREE, input_location);
> #endif
> }
> return ret_val;
> diff --git gcc/testsuite/gcc.dg/Wsign-compare-1.c gcc/testsuite/gcc.dg/Wsign-compare-1.c
> index e69de29bb2d..be3bd2fcbd8 100644
> --- gcc/testsuite/gcc.dg/Wsign-compare-1.c
> +++ gcc/testsuite/gcc.dg/Wsign-compare-1.c
> @@ -0,0 +1,83 @@
> +/* PR c/81417 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wsign-compare -fdiagnostics-show-caret" } */
> +
> +unsigned int
> +f0 (int x, unsigned int y)
> +{
> + return x ? y : -1; /* { dg-warning "18:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return x ? y : -1;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f1 (int xxx, unsigned int yyy)
> +{
> + return xxx ? yyy : -1; /* { dg-warning "22:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? yyy : -1;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f2 (int xxx, unsigned int yyy)
> +{
> + return xxx ? -1 : yyy; /* { dg-warning "16:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? -1 : yyy;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f3 (unsigned int yyy)
> +{
> + return yyy ?: -1; /* { dg-warning "17:operand of \\?: changes signedness from 'int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return yyy ?: -1;
> + ^~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f4 (int xxx, unsigned yyy, short uuu)
> +{
> + return xxx ? yyy : uuu; /* { dg-warning "22:operand of \\?: changes signedness from 'short int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? yyy : uuu;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f5 (int xxx, unsigned yyy, short uuu)
> +{
> + return xxx ? uuu : yyy; /* { dg-warning "16:operand of \\?: changes signedness from 'short int' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? uuu : yyy;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f6 (int xxx, unsigned yyy, signed char uuu)
> +{
> + return xxx ? yyy : uuu; /* { dg-warning "22:operand of \\?: changes signedness from 'signed char' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? yyy : uuu;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +unsigned int
> +f7 (int xxx, unsigned yyy, signed char uuu)
> +{
> + return xxx ? uuu : yyy; /* { dg-warning "16:operand of \\?: changes signedness from 'signed char' to 'unsigned int'" } */
> +/* { dg-begin-multiline-output "" }
> + return xxx ? uuu : yyy;
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> diff --git gcc/testsuite/gcc.dg/compare1.c gcc/testsuite/gcc.dg/compare1.c
> index 7becfbdb17f..ebab8c2cbf7 100644
> --- gcc/testsuite/gcc.dg/compare1.c
> +++ gcc/testsuite/gcc.dg/compare1.c
> @@ -22,17 +22,17 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
> }
>
> int i(enum mm2 x)
> diff --git gcc/testsuite/gcc.dg/compare2.c gcc/testsuite/gcc.dg/compare2.c
> index c309f1d00eb..cfadaccb8af 100644
> --- gcc/testsuite/gcc.dg/compare2.c
> +++ gcc/testsuite/gcc.dg/compare2.c
> @@ -9,35 +9,35 @@ int tf = 1;
> void f(int x, unsigned int y)
> {
> /* ?: branches are constants. */
> - x > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 1" } */
> - y > (tf?64:128); /* { dg-bogus "signed and unsigned" "case 2" } */
> + x > (tf?64:128); /* { dg-bogus "changes signedness" "case 1" } */
> + y > (tf?64:128); /* { dg-bogus "changes signedness" "case 2" } */
>
> /* ?: branches are (recursively) constants. */
> - x > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 3" } */
> - y > (tf?64:(tf?128:256)); /* { dg-bogus "signed and unsigned" "case 4" } */
> + x > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 3" } */
> + y > (tf?64:(tf?128:256)); /* { dg-bogus "changes signedness" "case 4" } */
>
> /* ?: branches are signed constants. */
> - x > (tf?64:-1); /* { dg-bogus "signed and unsigned" "case 5" } */
> + x > (tf?64:-1); /* { dg-bogus "changes signedness" "case 5" } */
> y > (tf?64:-1); /* { dg-warning "different signedness" "case 6" } */
>
> /* ?: branches are (recursively) signed constants. */
> - x > (tf?64:(tf?128:-1)); /* { dg-bogus "signed and unsigned" "case 7" } */
> + x > (tf?64:(tf?128:-1)); /* { dg-bogus "changes signedness" "case 7" } */
> y > (tf?64:(tf?128:-1)); /* { dg-warning "different signedness" "case 8" } */
>
> /* Statement expression. */
> - x > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 9" } */
> - y > ({tf; 64;}); /* { dg-bogus "signed and unsigned" "case 10" } */
> + x > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 9" } */
> + y > ({tf; 64;}); /* { dg-bogus "changes signedness" "case 10" } */
>
> /* Statement expression with recursive ?: . */
> - x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 11" } */
> - y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "signed and unsigned" "case 12" } */
> + x > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 11" } */
> + y > ({tf; tf?64:(tf?128:256);}); /* { dg-bogus "changes signedness" "case 12" } */
>
> /* Statement expression with signed ?:. */
> - x > ({tf; tf?64:-1;}); /* { dg-bogus "signed and unsigned" "case 13" } */
> + x > ({tf; tf?64:-1;}); /* { dg-bogus "changes signedness" "case 13" } */
> y > ({tf; tf?64:-1;}); /* { dg-warning "different signedness" "case 14" } */
>
> /* Statement expression with recursive signed ?:. */
> - x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "signed and unsigned" "case 15" } */
> + x > ({tf; tf?64:(tf?128:-1);}); /* { dg-bogus "changes signedness" "case 15" } */
> y > ({tf; tf?64:(tf?128:-1);}); /* { dg-warning "different signedness" "case 16" } */
>
> /* ?: branches are constants. */
> diff --git gcc/testsuite/gcc.dg/compare3.c gcc/testsuite/gcc.dg/compare3.c
> index eda3faf2754..836231fb870 100644
> --- gcc/testsuite/gcc.dg/compare3.c
> +++ gcc/testsuite/gcc.dg/compare3.c
> @@ -11,49 +11,49 @@ void f(int x, unsigned int y)
> /* Test comparing conditional expressions containing truth values.
> This can occur explicitly, or e.g. when (foo?2:(bar?1:0)) is
> optimized into (foo?2:(bar!=0)). */
> - x > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 1" } */
> - y > (tf?64:(tf!=x)); /* { dg-bogus "signed and unsigned" "case 2" } */
> - x > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 3" } */
> - y > (tf?(tf!=x):64); /* { dg-bogus "signed and unsigned" "case 4" } */
> -
> - x > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 5" } */
> - y > (tf?64:(tf==x)); /* { dg-bogus "signed and unsigned" "case 6" } */
> - x > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 7" } */
> - y > (tf?(tf==x):64); /* { dg-bogus "signed and unsigned" "case 8" } */
> -
> - x > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 9" } */
> - y > (tf?64:(tf>x)); /* { dg-bogus "signed and unsigned" "case 10" } */
> - x > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 11" } */
> - y > (tf?(tf>x):64); /* { dg-bogus "signed and unsigned" "case 12" } */
> -
> - x < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 13" } */
> - y < (tf?64:(tf<x)); /* { dg-bogus "signed and unsigned" "case 14" } */
> - x < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 15" } */
> - y < (tf?(tf<x):64); /* { dg-bogus "signed and unsigned" "case 16" } */
> -
> - x > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 17" } */
> - y > (tf?64:(tf>=x)); /* { dg-bogus "signed and unsigned" "case 18" } */
> - x > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 19" } */
> - y > (tf?(tf>=x):64); /* { dg-bogus "signed and unsigned" "case 20" } */
> -
> - x > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 21" } */
> - y > (tf?64:(tf<=x)); /* { dg-bogus "signed and unsigned" "case 22" } */
> - x > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 23" } */
> - y > (tf?(tf<=x):64); /* { dg-bogus "signed and unsigned" "case 24" } */
> -
> - x > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 25" } */
> - y > (tf?64:(tf&&x)); /* { dg-bogus "signed and unsigned" "case 26" } */
> - x > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 27" } */
> - y > (tf?(tf&&x):64); /* { dg-bogus "signed and unsigned" "case 28" } */
> -
> - x > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 29" } */
> - y > (tf?64:(tf||x)); /* { dg-bogus "signed and unsigned" "case 30" } */
> - x > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 31" } */
> - y > (tf?(tf||x):64); /* { dg-bogus "signed and unsigned" "case 32" } */
> -
> - x > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 33" } */
> - y > (tf?64:(!tf)); /* { dg-bogus "signed and unsigned" "case 34" } */
> - x > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 35" } */
> - y > (tf?(!tf):64); /* { dg-bogus "signed and unsigned" "case 36" } */
> + x > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 1" } */
> + y > (tf?64:(tf!=x)); /* { dg-bogus "changes signedness" "case 2" } */
> + x > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 3" } */
> + y > (tf?(tf!=x):64); /* { dg-bogus "changes signedness" "case 4" } */
> +
> + x > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 5" } */
> + y > (tf?64:(tf==x)); /* { dg-bogus "changes signedness" "case 6" } */
> + x > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 7" } */
> + y > (tf?(tf==x):64); /* { dg-bogus "changes signedness" "case 8" } */
> +
> + x > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 9" } */
> + y > (tf?64:(tf>x)); /* { dg-bogus "changes signedness" "case 10" } */
> + x > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 11" } */
> + y > (tf?(tf>x):64); /* { dg-bogus "changes signedness" "case 12" } */
> +
> + x < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 13" } */
> + y < (tf?64:(tf<x)); /* { dg-bogus "changes signedness" "case 14" } */
> + x < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 15" } */
> + y < (tf?(tf<x):64); /* { dg-bogus "changes signedness" "case 16" } */
> +
> + x > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 17" } */
> + y > (tf?64:(tf>=x)); /* { dg-bogus "changes signedness" "case 18" } */
> + x > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 19" } */
> + y > (tf?(tf>=x):64); /* { dg-bogus "changes signedness" "case 20" } */
> +
> + x > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 21" } */
> + y > (tf?64:(tf<=x)); /* { dg-bogus "changes signedness" "case 22" } */
> + x > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 23" } */
> + y > (tf?(tf<=x):64); /* { dg-bogus "changes signedness" "case 24" } */
> +
> + x > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 25" } */
> + y > (tf?64:(tf&&x)); /* { dg-bogus "changes signedness" "case 26" } */
> + x > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 27" } */
> + y > (tf?(tf&&x):64); /* { dg-bogus "changes signedness" "case 28" } */
> +
> + x > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 29" } */
> + y > (tf?64:(tf||x)); /* { dg-bogus "changes signedness" "case 30" } */
> + x > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 31" } */
> + y > (tf?(tf||x):64); /* { dg-bogus "changes signedness" "case 32" } */
> +
> + x > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 33" } */
> + y > (tf?64:(!tf)); /* { dg-bogus "changes signedness" "case 34" } */
> + x > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 35" } */
> + y > (tf?(!tf):64); /* { dg-bogus "changes signedness" "case 36" } */
>
> }
> diff --git gcc/testsuite/gcc.dg/compare7.c gcc/testsuite/gcc.dg/compare7.c
> index e2fbc04bfc2..b6fe6e78334 100644
> --- gcc/testsuite/gcc.dg/compare7.c
> +++ gcc/testsuite/gcc.dg/compare7.c
> @@ -6,5 +6,5 @@
>
> int f(unsigned a, int b)
> {
> - return a < b; /* { dg-bogus "signed and unsigned" } */
> + return a < b; /* { dg-bogus "changes signedness" } */
> }
> diff --git gcc/testsuite/gcc.dg/compare8.c gcc/testsuite/gcc.dg/compare8.c
> index d723c45a095..d09b69c53a2 100644
> --- gcc/testsuite/gcc.dg/compare8.c
> +++ gcc/testsuite/gcc.dg/compare8.c
> @@ -4,18 +4,18 @@
> int
> f(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
> + return ((a1+a2)|5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
> }
>
> int
> g(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
> + return ((a1+a2)&5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
> }
>
> int
> h(unsigned short a1, unsigned short a2, unsigned int b)
> {
> - return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "signed and unsigned" } */
> + return ((a1+a2)^5) > b ? 2 : 3; /* { dg-bogus "changes signedness" } */
> }
>
> diff --git gcc/testsuite/gcc.dg/compare9.c gcc/testsuite/gcc.dg/compare9.c
> index 02150cb1fb6..fba61e42a48 100644
> --- gcc/testsuite/gcc.dg/compare9.c
> +++ gcc/testsuite/gcc.dg/compare9.c
> @@ -22,20 +22,20 @@ enum mm2
>
> int f(enum mm1 x)
> {
> - return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */
> + return x == (tf?DI:SI); /* { dg-bogus "changes signedness" "case 1" } */
> }
>
> int g(enum mm1 x)
> {
> - return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */
> + return x == (tf?DI:-1); /* { dg-bogus "changes signedness" "case 2" } */
> }
>
> int h(enum mm2 x)
> {
> - return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */
> + return x == (tf?DI2:SI2); /* { dg-bogus "changes signedness" "case 3" } */
> }
>
> int i(enum mm2 x)
> {
> - return x == (tf?DI2:-1); /* { dg-bogus "signed and unsigned" "case 4" } */
> + return x == (tf?DI2:-1); /* { dg-bogus "changes signedness" "case 4" } */
> }
> diff --git gcc/testsuite/gcc.dg/pr11492.c gcc/testsuite/gcc.dg/pr11492.c
> index cf17712dde1..86435a83e79 100644
> --- gcc/testsuite/gcc.dg/pr11492.c
> +++ gcc/testsuite/gcc.dg/pr11492.c
> @@ -5,7 +5,7 @@ int main( void )
> {
> unsigned int a;
> unsigned char b;
> - for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison between signed and unsigned integer" } */
> + for ( a = 0, b = 2; a > b * 100; a++ ) /* { dg-bogus "comparison of integer expressions of different signedness" } */
> { ; }
>
> return 0;
>
> Marek
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-08-02 12:43 ` Marek Polacek
2017-08-08 11:28 ` Marek Polacek
@ 2017-08-08 21:48 ` Joseph Myers
2017-08-10 7:50 ` Andreas Schwab
2 siblings, 0 replies; 14+ messages in thread
From: Joseph Myers @ 2017-08-08 21:48 UTC (permalink / raw)
To: Marek Polacek; +Cc: David Malcolm, GCC Patches
On Wed, 2 Aug 2017, Marek Polacek wrote:
> Hmm, how about this, then?
>
> "operand of ?: changes signedness from %qT to %qT due to unsignedness of other operand"
>
> I couldn't come up with anything more brief yet conveying all the information.
> I don't like adding "second"/"third"/... very much; we should offer a good
> location already.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-08-02 12:43 ` Marek Polacek
2017-08-08 11:28 ` Marek Polacek
2017-08-08 21:48 ` Joseph Myers
@ 2017-08-10 7:50 ` Andreas Schwab
2017-08-10 9:25 ` Marek Polacek
2 siblings, 1 reply; 14+ messages in thread
From: Andreas Schwab @ 2017-08-10 7:50 UTC (permalink / raw)
To: Marek Polacek; +Cc: David Malcolm, GCC Patches, Joseph Myers
FAIL: gcc.dg/compare2.c case 20 (test for warnings, line 49)
FAIL: gcc.dg/compare2.c case 24 (test for warnings, line 57)
FAIL: gcc.dg/compare2.c (test for excess errors)
Excess errors:
/usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:49:12: warning: operand of ?: changes signedness from 'int' to 'unsigned int' due to unsignedness of other operand [-Wsign-compare]
/usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:57:12: warning: operand of ?: changes signedness from 'int' to 'unsigned int' due to unsignedness of other operand [-Wsign-compare]
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417)
2017-08-10 7:50 ` Andreas Schwab
@ 2017-08-10 9:25 ` Marek Polacek
0 siblings, 0 replies; 14+ messages in thread
From: Marek Polacek @ 2017-08-10 9:25 UTC (permalink / raw)
To: Andreas Schwab; +Cc: David Malcolm, GCC Patches, Joseph Myers
On Thu, Aug 10, 2017 at 09:35:07AM +0200, Andreas Schwab wrote:
> FAIL: gcc.dg/compare2.c case 20 (test for warnings, line 49)
> FAIL: gcc.dg/compare2.c case 24 (test for warnings, line 57)
> FAIL: gcc.dg/compare2.c (test for excess errors)
> Excess errors:
> /usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:49:12: warning: operand of ?: changes signedness from 'int' to 'unsigned int' due to unsignedness of other operand [-Wsign-compare]
> /usr/local/gcc/gcc-20170810/gcc/testsuite/gcc.dg/compare2.c:57:12: warning: operand of ?: changes signedness from 'int' to 'unsigned int' due to unsignedness of other operand [-Wsign-compare]
Fixed.
Marek
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-08-10 8:57 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-31 14:15 C PATCH to further improve diagnostic for -Wsign-compare (PR c/81417) Marek Polacek
2017-07-31 15:31 ` David Malcolm
2017-08-01 14:16 ` Marek Polacek
2017-08-01 20:49 ` David Malcolm
2017-08-02 2:36 ` Martin Sebor
2017-08-02 12:43 ` Marek Polacek
2017-08-08 11:28 ` Marek Polacek
2017-08-08 21:48 ` Joseph Myers
2017-08-10 7:50 ` Andreas Schwab
2017-08-10 9:25 ` Marek Polacek
2017-07-31 15:54 ` Martin Sebor
2017-07-31 16:05 ` Marek Polacek
2017-07-31 17:07 ` Martin Sebor
2017-08-01 20:18 ` David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).