[PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

[David Blaikie via cfe-commits]

I'd still wonder if this meets the bar for false positives (generally we
try to only add warnings that would be enabled by default, even in new
codebases - where most of what they find in a newcodebase are latent bugs,
not noise (so the cleanup is at least fairly justified as being beneficial
in itself rather than only as a means to enable the warning to catch some
bugs in the future))

But I know we made some exception for the &&/|| version of -Wparentheses,
so maybe this variation meets that same bar. (if Richard Trieu doesn't have
enough context to make that call, I'd probably rope in Richard Smith (&
possibly Chandler Carruth - I recall him commenting on the &&/||
-Wparentheses form on more than one occasion))

On Tue, Sep 27, 2016 at 10:01 AM Bruno Cardoso Lopes <
bruno.cardoso at gmail.com> wrote:

> bruno added a subscriber: bruno.
> bruno added a comment.
>
> Hi Daniel,
>
> This is very nice.
>
> In https://reviews.llvm.org/D24861#553606, @danielmarjamaki wrote:
>
> > Compiling 2064 projects resulted in 904 warnings
> >
> > Here are the results:
> >
> https://drive.google.com/file/d/0BykPmWrCOxt2N04tYl8zVHA3MXc/view?usp=sharing
> >
> > The results looks acceptable imho. The code looks intentional in many
> cases so I believe there are users that will disable this warning. Probably
> there are true positives where the evaluation order is not really known.
> There were many warnings about macro arguments where the macro bitshifts
> the argument - these macros look very shaky to me.
> >
> > I saw some warnings about such code:
> >
> >   a * b << c
> >
> >
> > Maybe we should not warn about this. As far as I see, the result will be
> the same if (a*b) or (b<<c) is evaluated first - unless there is some
> overflow or signedness issues. What do you think? I'll keep these warnings
> for now.
>
>
> Any idea on how expensive would be to reason about these false positives
> and avoid them?
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24861
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160927/0595b029/attachment.html>