[C/C++ PATCH] Implement -Wbool-compare (PR c++/62153)

[Marek Polacek]

On Mon, Aug 18, 2014 at 10:57:58PM +0200, Manuel López-Ibáñez wrote:
> On 18 August 2014 22:04, Marek Polacek <polacek@redhat.com> wrote:
> > +void
> > +maybe_warn_bool_compare (location_t loc, enum tree_code code, tree op0,
> > +                        tree op1)
> > +{
> > +  if (TREE_CODE_CLASS (code) != tcc_comparison)
> > +    return;
> > +
> > +  /* <boolean> CMP <cst> */
> > +  if (TREE_CODE (op1) == INTEGER_CST
> > +      && !integer_zerop (op1)
> > +      && !integer_onep (op1))
> > +    {
> > +      if (code == EQ_EXPR
> > +         || ((code == GT_EXPR || code == GE_EXPR)
> > +             && tree_int_cst_sgn (op1) == 1)
> > +         || ((code == LT_EXPR || code == LE_EXPR)
> > +             && tree_int_cst_sgn (op1) == -1))
> > +       warning_at (loc, OPT_Wbool_compare, "comparison of constant %qE "
> > +                   "with boolean expression is always false", op1);
> > +      else
> > +       warning_at (loc, OPT_Wbool_compare, "comparison of constant %qE "
> > +                   "with boolean expression is always true", op1);
> > +    }
> > +  /* <cst> CMP <boolean> */
> > +  else if (TREE_CODE (op0) == INTEGER_CST
> > +          && !integer_zerop (op0)
> > +          && !integer_onep (op0))
> > +    {
> > +      if (code == EQ_EXPR
> > +         || ((code == GT_EXPR || code == GE_EXPR)
> > +             && tree_int_cst_sgn (op0) == -1)
> > +         || ((code == LT_EXPR || code == LE_EXPR)
> > +             && tree_int_cst_sgn (op0) == 1))
> > +       warning_at (loc, OPT_Wbool_compare, "comparison of constant %qE "
> > +                   "with boolean expression is always false", op0);
> > +      else
> > +       warning_at (loc, OPT_Wbool_compare, "comparison of constant %qE "
> > +                   "with boolean expression is always true", op0);
> > +    }
> 
> Perhaps you can save some repetition by doing first:
> 
> tree op = (TREE_CODE (op1) == INTEGER_CST) ? op1 : op0;
> if (TREE_CODE (op) == INTEGER_CST
>     && !integer_zerop (op)

Not sure about that: it matters whether the CST is a LHS or a RHS
- because we want to say if the comparison is always true or false.
I tried to introduce some bool flag, but that didn't really help
readability IMHO.  (The tree_int_cst_sgn is compared to 1 and -1, or
to -1 and 1.)

	Marek