On Tue, Jan 21, 2014 at 07:50:13PM +0100, Jakub Jelinek wrote:
> On Tue, Jan 21, 2014 at 07:38:10PM +0100, Marek Polacek wrote:
> > --- gcc/gcc/c/c-typeck.c.mp 2014-01-21 11:59:33.221215248 +0100
> > +++ gcc/gcc/c/c-typeck.c 2014-01-21 18:10:53.900279750 +0100
> > @@ -4757,6 +4757,18 @@ build_compound_expr (location_t loc, tre
> > expr2 = TREE_OPERAND (expr2, 0);
> > }
> >
> > + if (TREE_CODE (expr1) == COMPOUND_EXPR
> > + && warn_unused_value)
> > + {
> > + tree r = expr1;
> > + while (TREE_CODE (r) == COMPOUND_EXPR)
> > + r = TREE_OPERAND (r, 1);
> > + if (!TREE_SIDE_EFFECTS (r)
> > + && !VOID_TYPE_P (TREE_TYPE (r))
> > + && !CONVERT_EXPR_P (r))
> > + warning_at (loc, OPT_Wunused_value,
> > + "right-hand operand of comma expression has no effect");
> > + }
> > if (!TREE_SIDE_EFFECTS (expr1))
> > {
> > /* The left-hand operand of a comma expression is like an expression
>
> Won't this warn twice if !TREE_SIDE_EFFECTS (expr1) and expr1 is a
> COMPOUND_EXPR? I would be expecting this whole if as else if below the
> !TREE_SIDE_EFFECTS case. I mean, in emit_side_effect_warnings you do it
> the same way, and if nothing in the expr1 COMPOUND_EXPR has any
> !side-effects, then it is ok to complain about the expr1 as whole, no
> need to complain particularly about the rhs of it.
Ooops, for some reason I moved the whole if above. I've changed it
so it's the same as in the emit_side_effect_warnings now, meaning that for
1, 1, 1; we emit what GCC 4.8 emits.
> Also, shouldn't you use EXPR_LOCATION of the COMPOUND_EXPR r is rhs of
> in the end? I.e. do something like:
> location_t cloc = loc;
> while (TREE_CODE (r) == COMPOUND_EXPR)
> {
> if (EXPR_HAS_LOCATION (r)) cloc = EXPR_LOCATION (r);
> r = TREE_OPERAND (r, 1);
> }
> and use cloc in warning_at.
Yes, this produces better locus, so thanks for the hint.
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).
Furthermore, I incorporated a change into decNumberLocal.h, which Marc
suggested.
Regtested/bootstrapped on x86_64-linux, ok for trunk?
2014-01-21 Marek Polacek <pola...@redhat.com>
PR c/59871
c/
* c-typeck.c (build_compound_expr): Warn even for right-hand operand
of comma expression.
(emit_side_effect_warnings): Likewise.
libdecnumber/
* decNumberLocal.h (UBFROMUS, UBFROMUI): Remove last argument.
testsuite/
* gcc.dg/20020220-2.c: Adjust dg-warning message.
* gcc.dg/pr59871.c: New test.
--- 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" } */
+}
Marek
