[Bug c/44949] extend Wparentheses from & to &=

["manu at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44949

Manuel LÃpez-IbÃÃez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2016-04-03
                 CC|                            |manu at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Manuel LÃpez-IbÃÃez <manu at gcc dot gnu.org> ---
(In reply to Marc Glisse from comment #1)
> Created attachment 24935 [details]
> hack
> 
> It is an easy enough hack to call the Wparentheses code for &= and others
> (the warning says "&" instead of "&=", it isn't a clean patch) so I can test
> on real code. And it seems painful. It warns on code like: a |= b & c; where
> it is quite clear we could never intend (a|=b)&c;. Restricting the warning
> to the case where the result of the expression is used should improve
> things, except that I don't know how to access that information...

I think the problem cannot be solved like this because assignment operators
have a different precedence than their non-assignment equivalents. That is,

  i&=c|b;
  i = i & c|b;

are not equivalent. Do you want to warn about the above?

It would probably help you to create a very long testcase listing everything
you wish to warn and not warn about, then start from a new function instead
from warn_about_parentheses. If it turns out that some parts can be shared,
then great. (In any case, the warnings in warn_about_parentheses are very
clumsy and obscure: Clang provides a better explanation about what is wrong and
how to fix it, so starting from scratch seems better).

It may also be worth looking at the point where we give:

    case MODIFY_EXPR:
      if (!TREE_NO_WARNING (expr)
          && warn_parentheses)
        {
          warning (OPT_Wparentheses,
                   "suggest parentheses around assignment used as truth
value");
          TREE_NO_WARNING (expr) = 1;
        }
      break;

since it may be the point to catch most of:

  if (i&=2 == 0)

but you may need to revert the hack done here:

  if (code == NOP_EXPR)
    ret.original_code = MODIFY_EXPR;
  else
    {
      TREE_NO_WARNING (ret.value) = 1;
      ret.original_code = ERROR_MARK;
    }

and devise a way to keep the info that this was originally more than an
assignment operator (set ret.original_code to BIT_AND_EXPR, etc ?).