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
