Re: PATCH RFA: More -Wparentheses checks for C++

[Ian Lance Taylor <iant at google dot com>]

Mark Mitchell <mark@codesourcery.com> writes:

> Ian Lance Taylor wrote:
> 
> > Index: gcc/cp/semantics.c
> > ===================================================================
> > --- gcc/cp/semantics.c	(revision 120236)
> > +++ gcc/cp/semantics.c	(working copy)
> > @@ -587,6 +587,16 @@ maybe_convert_cond (tree cond)
> >  
> >    /* Do the conversion.  */
> >    cond = convert_from_reference (cond);
> > +
> > +  if (TREE_CODE (cond) == MODIFY_EXPR
> > +      && !TREE_NO_WARNING (cond)
> > +      && warn_parentheses)
> > +    {
> > +      warning (OPT_Wparentheses,
> > +	       "suggest parentheses around assignment used as truth value");
> > +      TREE_NO_WARNING (cond) = 1;
> > +    }
> > +
> >    return condition_conversion (cond);
> >  }
> 
> Will this warn for:
> 
>   // This is a POD.
>   struct S {
>     int i;
>   };
> 
>   S s;
> 
>   if (S(s)) {}
> 
> ?  Because, it would be entirely reasonable for the compiler to turn
> that into a MODIFY_EXPR.  Right now, it may use INIT_EXPR, but we
> vaguely expect INIT_EXPR to go away.
> 
> So, I'm afraid that because the C++ front end doesn't have a separate,
> later, lowering pass, this might be a dangerous change.

I turned that into this complete test case:

struct S {
  int i;
  operator bool() const
  { return i != 0; }
};

void foo(S s) {
  if (S(s)) {}
}

This does not get a warning today.  Right now the compiler uses
TARGET_EXPR for this code.

I added
  if (C(a))
    foo (28);
to both new test cases to catch this case.  If the code does change to
use MODIFY_EXPR for this case, there is an easy way to turn off the
warning: set TREE_NO_WARNING.  That is how we already avoid a warning for
  if ((a = b))
(in finish_parenthesized_expr).  In fact, TREE_NO_WARNING is already
set for INIT_EXPR; see build_modify_expr.

So I think we are OK on that front.  Is there another area which makes
you nervous?

Ian