On Thu, Jan 23, 2014 at 05:50:23PM +0000, Joseph S. Myers wrote:
> On Thu, 23 Jan 2014, Marek Polacek wrote:
>
> > shorten_compare can produce a better locinfo if we pass location
> > from {,cp_}build_binary_op to it; op0/op1 there don't have location.
> > Furthermore, I see no reason why use input_location in
> > parser_build_binary_op when we can use more accurate location.
> >
> > I don't know if/how I can test the column info using dejagnu, so
> > no testcase attached. But to give you an idea, instead of
>
> The approach used is
>
> /* { dg-warning "14:comparison is always false" } */
>
> or similar, with the column number put at the start of the dg-error /
> dg-warning text.
Argh, not sure how I missed that. So with a testcase this time.
OK now?
2014-01-23 Marek Polacek <pola...@redhat.com>
PR c/59846
c-family/
* c-common.c (shorten_compare): Add location_t parameter.
* c-common.h (shorten_binary_op): Adjust declaration.
cp/
* typeck.c (cp_build_binary_op): Pass location to shorten_compare.
c/
* c-typeck.c (parser_build_binary_op): Use location instead of
input_location.
(build_binary_op): Pass location to shorten_compare.
testsuite/
* gcc.dg/pr59846.c: New test.
--- gcc/c-family/c-common.h.mp 2014-01-23 16:19:49.936114468 +0100
+++ gcc/c-family/c-common.h 2014-01-23 19:31:26.276522298 +0100
@@ -799,7 +799,8 @@ extern tree shorten_binary_op (tree resu
/* Subroutine of build_binary_op, used for comparison operations.
See if the operands have both been converted from subword integer types
and, if so, perhaps change them both back to their original type. */
-extern tree shorten_compare (tree *, tree *, tree *, enum tree_code *);
+extern tree shorten_compare (location_t, tree *, tree *, tree *,
+ enum tree_code *);
extern tree pointer_int_sum (location_t, enum tree_code, tree, tree,
bool = true);
--- gcc/c-family/c-common.c.mp 2014-01-23 16:19:49.935114463 +0100
+++ gcc/c-family/c-common.c 2014-01-23 19:31:26.279522313 +0100
@@ -3974,13 +3974,15 @@ expr_original_type (tree expr)
of build_binary_op: OP0_PTR is &OP0, OP1_PTR is &OP1,
RESTYPE_PTR is &RESULT_TYPE and RESCODE_PTR is &RESULTCODE.
+ LOC is the location of the comparison.
+
If this function returns nonzero, it means that the comparison has
a constant value. What this function returns is an expression for
that value. */
tree
-shorten_compare (tree *op0_ptr, tree *op1_ptr, tree *restype_ptr,
- enum tree_code *rescode_ptr)
+shorten_compare (location_t loc, tree *op0_ptr, tree *op1_ptr,
+ tree *restype_ptr, enum tree_code *rescode_ptr)
{
tree type;
tree op0 = *op0_ptr;
@@ -3989,7 +3991,6 @@ shorten_compare (tree *op0_ptr, tree *op
int real1, real2;
tree primop0, primop1;
enum tree_code code = *rescode_ptr;
- location_t loc = EXPR_LOC_OR_LOC (op0, input_location);
/* Throw away any conversions to wider types
already present in the operands. */
--- gcc/cp/typeck.c.mp 2014-01-23 16:19:49.939114483 +0100
+++ gcc/cp/typeck.c 2014-01-23 19:31:26.312522476 +0100
@@ -4838,7 +4838,8 @@ cp_build_binary_op (location_t location,
tree xop0 = op0, xop1 = op1, xresult_type = result_type;
enum tree_code xresultcode = resultcode;
tree val
- = shorten_compare (&xop0, &xop1, &xresult_type, &xresultcode);
+ = shorten_compare (location, &xop0, &xop1, &xresult_type,
+ &xresultcode);
if (val != 0)
return cp_convert (boolean_type_node, val, complain);
op0 = xop0, op1 = xop1;
--- gcc/c/c-typeck.c.mp 2014-01-23 19:30:54.392378861 +0100
+++ gcc/c/c-typeck.c 2014-01-23 19:31:26.324522535 +0100
@@ -3388,11 +3388,11 @@ parser_build_binary_op (location_t locat
/* Check for cases such as x+y<<z which users are likely
to misinterpret. */
if (warn_parentheses)
- warn_about_parentheses (input_location, code,
- code1, arg1.value, code2, arg2.value);
+ warn_about_parentheses (location, code, code1, arg1.value, code2,
+ arg2.value);
if (warn_logical_op)
- warn_logical_operator (input_location, code, TREE_TYPE (result.value),
+ warn_logical_operator (location, code, TREE_TYPE (result.value),
code1, arg1.value, code2, arg2.value);
/* Warn about comparisons against string literals, with the exception
@@ -10854,7 +10854,8 @@ build_binary_op (location_t location, en
tree xop0 = op0, xop1 = op1, xresult_type = result_type;
enum tree_code xresultcode = resultcode;
tree val
- = shorten_compare (&xop0, &xop1, &xresult_type, &xresultcode);
+ = shorten_compare (location, &xop0, &xop1, &xresult_type,
+ &xresultcode);
if (val != 0)
{
--- gcc/testsuite/gcc.dg/pr59846.c.mp 2014-01-23 19:34:32.790368466 +0100
+++ gcc/testsuite/gcc.dg/pr59846.c 2014-01-23 19:42:00.910793437 +0100
@@ -0,0 +1,39 @@
+/* PR c/59846 */
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-op -Wtype-limits" } */
+
+_Bool
+fn1 (unsigned int p)
+{
+ return 0UL > p; /* { dg-warning "14:comparison is always false due to
limited range of data type" } */
+}
+
+_Bool
+fn2 (unsigned int p)
+{
+ return 0UL <= p; /* { dg-warning "14:comparison is always true due to
limited range of data type" } */
+}
+
+_Bool
+fn3 (unsigned int p)
+{
+ return p >= 0U; /* { dg-warning "12:comparison of unsigned expression >= 0
is always true" } */
+}
+
+_Bool
+fn4 (unsigned int p)
+{
+ return p < 0U; /* { dg-warning "12:comparison of unsigned expression < 0 is
always false" } */
+}
+
+_Bool
+fn5 (_Bool p)
+{
+ return p || !p; /* { dg-warning "12:logical" } */
+}
+
+_Bool
+fn6 (_Bool p)
+{
+ return p && !p; /* { dg-warning "12:logical" } */
+}
Marek
