[C PATCH] Warn about unused RHS of COMPOUND_EXPR (PR c/59871)

Jakub Jelinek jakub@redhat.com
Tue Jan 21 18:50:00 GMT 2014


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



More information about the Gcc-patches mailing list