Message ID | 20140121223058.GR8907@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, Jan 21, 2014 at 11:30:58PM +0100, Marek Polacek wrote: > I made another small change: in the second hunk, in > emit_side_effect_warnings, I got rid of the while, since it seems to > me we never get COMPOUND_EXPR in TREE_OPERAND (r, 1). For bar (), 1, 2, 3, 4, 5, 6, 7, 8; sure, but what about (1, (2, (3, (4, (5, (6, (bar (), 7))))))); ? Jakub
On Tue, Jan 21, 2014 at 11:34:42PM +0100, Jakub Jelinek wrote: > On Tue, Jan 21, 2014 at 11:30:58PM +0100, Marek Polacek wrote: > > I made another small change: in the second hunk, in > > emit_side_effect_warnings, I got rid of the while, since it seems to > > me we never get COMPOUND_EXPR in TREE_OPERAND (r, 1). > > For bar (), 1, 2, 3, 4, 5, 6, 7, 8; sure, but what about > (1, (2, (3, (4, (5, (6, (bar (), 7))))))); ? For that `expr' that emit_side_effect_warnings gets is: <compound_expr 0x7ffe2724b848 type <integer_type 0x7ffe27144690 int public SI size <integer_cst 0x7ffe271463c0 constant 32> unit size <integer_cst 0x7ffe271463e0 constant 4> align 32 symtab 0 alias set -1 canonical type 0x7ffe27144690 precision 32 min <integer_cst 0x7ffe27146360 -2147483648> max <integer_cst 0x7ffe27146380 2147483647> pointer_to_this <pointer_type 0x7ffe27150738>> side-effects arg 0 <call_expr 0x7ffe2726a840 type <integer_type 0x7ffe27144690 int> side-effects fn <addr_expr 0x7ffe2725a720 type <pointer_type 0x7ffe2726e0a8> constant arg 0 <function_decl 0x7ffe27250600 bar> uu.c:19:32> uu.c:19:32> arg 1 <integer_cst 0x7ffe2725a760 type <integer_type 0x7ffe27144690 int> constant 7> uu.c:19:5> so we should be fine even in that case. Marek
On Tue, Jan 21, 2014 at 11:54:03PM +0100, Marek Polacek wrote: > On Tue, Jan 21, 2014 at 11:34:42PM +0100, Jakub Jelinek wrote: > > On Tue, Jan 21, 2014 at 11:30:58PM +0100, Marek Polacek wrote: > > > I made another small change: in the second hunk, in > > > emit_side_effect_warnings, I got rid of the while, since it seems to > > > me we never get COMPOUND_EXPR in TREE_OPERAND (r, 1). > > > > For bar (), 1, 2, 3, 4, 5, 6, 7, 8; sure, but what about > > (1, (2, (3, (4, (5, (6, (bar (), 7))))))); ? > > For that `expr' that emit_side_effect_warnings gets is: > <compound_expr 0x7ffe2724b848 > type <integer_type 0x7ffe27144690 int public SI > size <integer_cst 0x7ffe271463c0 constant 32> > unit size <integer_cst 0x7ffe271463e0 constant 4> > align 32 symtab 0 alias set -1 canonical type 0x7ffe27144690 precision 32 min <integer_cst 0x7ffe27146360 -2147483648> max <integer_cst 0x7ffe27146380 2147483647> > pointer_to_this <pointer_type 0x7ffe27150738>> > side-effects > arg 0 <call_expr 0x7ffe2726a840 type <integer_type 0x7ffe27144690 int> > side-effects > fn <addr_expr 0x7ffe2725a720 type <pointer_type 0x7ffe2726e0a8> > constant arg 0 <function_decl 0x7ffe27250600 bar> > uu.c:19:32> > uu.c:19:32> > arg 1 <integer_cst 0x7ffe2725a760 type <integer_type 0x7ffe27144690 int> constant 7> > uu.c:19:5> > so we should be fine even in that case. And for (bar (), (bar (), (bar (), (bar (), (bar (), (bar (), (bar (), 7))))))); ? On the other side, there is warn_if_unused_value, which specifically has: /* Let people do `(foo (), 0)' without a warning. */ if (TREE_CONSTANT (TREE_OPERAND (exp, 1))) return false; so perhaps it is intentional that we don't warn about this? That stuff dates all the way back to r759. Though I wonder why it would be desirable to warn about this for C++ and not for C. Jakub
--- gcc/libdecnumber/decNumberLocal.h.mp 2014-01-21 18:34:32.235540589 +0100 +++ gcc/libdecnumber/decNumberLocal.h 2014-01-21 22:04:34.331223218 +0100 @@ -153,10 +153,9 @@ see the files COPYING3 and COPYING.RUNTI #define UBTOUI(b) (memcpy((void *)&uiwork, b, 4), uiwork) /* Store a uInt, etc., into bytes starting at a char* or uByte*. */ - /* Returns i, evaluated, for convenience; has to use uiwork because */ - /* i may be an expression. */ - #define UBFROMUS(b, i) (uswork=(i), memcpy(b, (void *)&uswork, 2), uswork) - #define UBFROMUI(b, i) (uiwork=(i), memcpy(b, (void *)&uiwork, 4), uiwork) + /* Has to use uiwork because i may be an expression. */ + #define UBFROMUS(b, i) (uswork=(i), memcpy(b, (void *)&uswork, 2)) + #define UBFROMUI(b, i) (uiwork=(i), memcpy(b, (void *)&uiwork, 4)) /* X10 and X100 -- multiply integer i by 10 or 100 */ /* [shifts are usually faster than multiply; could be conditional] */ --- gcc/gcc/c/c-typeck.c.mp 2014-01-21 11:59:33.221215248 +0100 +++ gcc/gcc/c/c-typeck.c 2014-01-21 22:06:32.349778039 +0100 @@ -4776,6 +4776,23 @@ build_compound_expr (location_t loc, tre "left-hand operand of comma expression has no effect"); } } + else if (TREE_CODE (expr1) == COMPOUND_EXPR + && warn_unused_value) + { + tree r = expr1; + location_t cloc = loc; + while (TREE_CODE (r) == COMPOUND_EXPR) + { + if (EXPR_HAS_LOCATION (r)) + cloc = EXPR_LOCATION (r); + r = TREE_OPERAND (r, 1); + } + if (!TREE_SIDE_EFFECTS (r) + && !VOID_TYPE_P (TREE_TYPE (r)) + && !CONVERT_EXPR_P (r)) + warning_at (cloc, OPT_Wunused_value, + "right-hand operand of comma expression has no effect"); + } /* With -Wunused, we should also warn if the left-hand operand does have side-effects, but computes a value which is not used. For example, in @@ -9641,6 +9658,16 @@ emit_side_effect_warnings (location_t lo if (!VOID_TYPE_P (TREE_TYPE (expr)) && !TREE_NO_WARNING (expr)) warning_at (loc, OPT_Wunused_value, "statement with no effect"); } + else if (TREE_CODE (expr) == COMPOUND_EXPR) + { + tree r = TREE_OPERAND (expr, 1); + if (!TREE_SIDE_EFFECTS (r) + && !VOID_TYPE_P (TREE_TYPE (r)) + && !CONVERT_EXPR_P (r) + && !TREE_NO_WARNING (expr)) + warning_at (EXPR_LOC_OR_LOC (expr, loc), OPT_Wunused_value, + "right-hand operand of comma expression has no effect"); + } else warn_if_unused_value (expr, loc); } --- gcc/gcc/testsuite/gcc.dg/20020220-2.c.mp 2014-01-21 14:47:58.888754509 +0100 +++ gcc/gcc/testsuite/gcc.dg/20020220-2.c 2014-01-21 23:13:23.758405040 +0100 @@ -1,5 +1,5 @@ /* PR c/4697 - Test whether value computed not used warning is given for compound + Test whether operand has no effect warning is given for compound expression. */ /* { dg-do compile } */ /* { dg-options "-O2 -Wunused" } */ @@ -7,6 +7,6 @@ int b; int foo (int a) { - a = a + 1, 5 * b; /* { dg-warning "value computed is not used" } */ + a = a + 1, 5 * b; /* { dg-warning "right-hand operand of comma expression has no effect" } */ return a; } --- gcc/gcc/testsuite/gcc.dg/pr59871.c.mp 2014-01-21 16:17:49.000000000 +0100 +++ gcc/gcc/testsuite/gcc.dg/pr59871.c 2014-01-21 16:13:39.000000000 +0100 @@ -0,0 +1,15 @@ +/* PR c/59871 */ +/* { dg-do compile } */ +/* { dg-options "-Wunused" } */ + +extern int bar (); + +void +foo (int *p) +{ + p[0] = (bar (), 1, bar ()); /* { dg-warning "right-hand operand of comma expression has no effect" } */ + p[1] = (1, bar ()); /* { dg-warning "left-hand operand of comma expression has no effect" } */ + bar (), 1, bar (); /* { dg-warning "right-hand operand of comma expression has no effect" } */ + bar (), 1; /* { dg-warning "right-hand operand of comma expression has no effect" } */ + 1, bar (); /* { dg-warning "left-hand operand of comma expression has no effect" } */ +}