* [C PATCH] Improve locinfo a bit (PR c/59846)
@ 2014-01-23 16:37 Marek Polacek
2014-01-23 17:50 ` Joseph S. Myers
0 siblings, 1 reply; 8+ messages in thread
From: Marek Polacek @ 2014-01-23 16:37 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph S. Myers, Jason Merrill, Jakub Jelinek
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
tt.c:4:3: warning: comparison is always false due to limited range of data type [-Wtype-limits]
return 0UL > p;
^
we now show
tt.c:4:14: warning: comparison is always false due to limited range of data type [-Wtype-limits]
return 0UL > p;
^
Regtested/bootstrapped on x86_64-linux, ok for trunk?
2014-01-23 Marek Polacek <polacek@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.
--- gcc/c-family/c-common.h.mp 2014-01-23 16:19:49.936114468 +0100
+++ gcc/c-family/c-common.h 2014-01-23 16:19:55.407138560 +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 16:19:55.407138560 +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 16:19:55.424138642 +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 16:19:49.938114478 +0100
+++ gcc/c/c-typeck.c 2014-01-23 17:15:56.404366164 +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)
{
Marek
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C PATCH] Improve locinfo a bit (PR c/59846)
2014-01-23 16:37 [C PATCH] Improve locinfo a bit (PR c/59846) Marek Polacek
@ 2014-01-23 17:50 ` Joseph S. Myers
2014-01-23 18:45 ` Marek Polacek
0 siblings, 1 reply; 8+ messages in thread
From: Joseph S. Myers @ 2014-01-23 17:50 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Jason Merrill, Jakub Jelinek
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.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C PATCH] Improve locinfo a bit (PR c/59846)
2014-01-23 17:50 ` Joseph S. Myers
@ 2014-01-23 18:45 ` Marek Polacek
2014-01-23 20:53 ` Joseph S. Myers
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Marek Polacek @ 2014-01-23 18:45 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: GCC Patches, Jason Merrill, Jakub Jelinek
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 <polacek@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C PATCH] Improve locinfo a bit (PR c/59846)
2014-01-23 18:45 ` Marek Polacek
@ 2014-01-23 20:53 ` Joseph S. Myers
2014-01-23 23:18 ` H.J. Lu
2014-01-24 15:07 ` Andreas Schwab
2 siblings, 0 replies; 8+ messages in thread
From: Joseph S. Myers @ 2014-01-23 20:53 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Jason Merrill, Jakub Jelinek
On Thu, 23 Jan 2014, Marek Polacek wrote:
> 2014-01-23 Marek Polacek <polacek@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.
OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C PATCH] Improve locinfo a bit (PR c/59846)
2014-01-23 18:45 ` Marek Polacek
2014-01-23 20:53 ` Joseph S. Myers
@ 2014-01-23 23:18 ` H.J. Lu
2014-01-23 23:27 ` Marek Polacek
2014-01-24 15:07 ` Andreas Schwab
2 siblings, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2014-01-23 23:18 UTC (permalink / raw)
To: Marek Polacek; +Cc: Joseph S. Myers, GCC Patches, Jason Merrill, Jakub Jelinek
On Thu, Jan 23, 2014 at 10:44 AM, Marek Polacek <polacek@redhat.com> wrote:
> 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 <polacek@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.
>
It failed on Linux/x86:
FAIL: gcc.dg/pr59846.c (test for excess errors)
FAIL: gcc.dg/pr59846.c (test for warnings, line 14)
FAIL: gcc.dg/pr59846.c (test for warnings, line 8)
--
H.J.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C PATCH] Improve locinfo a bit (PR c/59846)
2014-01-23 23:18 ` H.J. Lu
@ 2014-01-23 23:27 ` Marek Polacek
0 siblings, 0 replies; 8+ messages in thread
From: Marek Polacek @ 2014-01-23 23:27 UTC (permalink / raw)
To: H.J. Lu; +Cc: Joseph S. Myers, GCC Patches, Jason Merrill, Jakub Jelinek
On Thu, Jan 23, 2014 at 03:18:55PM -0800, H.J. Lu wrote:
> It failed on Linux/x86:
>
> FAIL: gcc.dg/pr59846.c (test for excess errors)
> FAIL: gcc.dg/pr59846.c (test for warnings, line 14)
> FAIL: gcc.dg/pr59846.c (test for warnings, line 8)
Ooops, yeah. I'll install a fix tomorrow.
Marek
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C PATCH] Improve locinfo a bit (PR c/59846)
2014-01-23 18:45 ` Marek Polacek
2014-01-23 20:53 ` Joseph S. Myers
2014-01-23 23:18 ` H.J. Lu
@ 2014-01-24 15:07 ` Andreas Schwab
2014-01-24 17:01 ` Marek Polacek
2 siblings, 1 reply; 8+ messages in thread
From: Andreas Schwab @ 2014-01-24 15:07 UTC (permalink / raw)
To: Marek Polacek; +Cc: Joseph S. Myers, GCC Patches, Jason Merrill, Jakub Jelinek
Marek Polacek <polacek@redhat.com> writes:
> --- 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" } */
> +}
FAIL: gcc.dg/pr59846.c (test for warnings, line 8)
FAIL: gcc.dg/pr59846.c (test for warnings, line 14)
FAIL: gcc.dg/pr59846.c (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20140124/gcc/testsuite/gcc.dg/pr59846.c:8:14: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
/daten/aranym/gcc/gcc-20140124/gcc/testsuite/gcc.dg/pr59846.c:14:14: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C PATCH] Improve locinfo a bit (PR c/59846)
2014-01-24 15:07 ` Andreas Schwab
@ 2014-01-24 17:01 ` Marek Polacek
0 siblings, 0 replies; 8+ messages in thread
From: Marek Polacek @ 2014-01-24 17:01 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Joseph S. Myers, GCC Patches, Jason Merrill, Jakub Jelinek
On Fri, Jan 24, 2014 at 04:06:50PM +0100, Andreas Schwab wrote:
> FAIL: gcc.dg/pr59846.c (test for warnings, line 8)
> FAIL: gcc.dg/pr59846.c (test for warnings, line 14)
> FAIL: gcc.dg/pr59846.c (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20140124/gcc/testsuite/gcc.dg/pr59846.c:8:14: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
> /daten/aranym/gcc/gcc-20140124/gcc/testsuite/gcc.dg/pr59846.c:14:14: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
Should be already fixed by r207026.
Marek
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-01-24 17:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-23 16:37 [C PATCH] Improve locinfo a bit (PR c/59846) Marek Polacek
2014-01-23 17:50 ` Joseph S. Myers
2014-01-23 18:45 ` Marek Polacek
2014-01-23 20:53 ` Joseph S. Myers
2014-01-23 23:18 ` H.J. Lu
2014-01-23 23:27 ` Marek Polacek
2014-01-24 15:07 ` Andreas Schwab
2014-01-24 17:01 ` Marek Polacek
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).