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. 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. Jakub