[cfe-dev] Fwd: [cfe-commits] r133526 - /cfe/trunk/lib/Sema/SemaExpr.cpp

[Chandler Carruth]

On Tue, Jun 21, 2011 at 5:27 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> > I think 'silence' operation is about equally likely to be the correct
> > response for
> >
> > -Wparentheses for mixed && and ||
> > -Wparentheses for mixed & and |
>
> When we turned this warning on for our codebase, we found only one bug
> among about 20 warnings.
>

I think this is very similar to the assignment-in-condition-expression
warning: there is a non-trivial "false-positive" rate when you first turn it
on because it's  essentially asking for extra clarification on a dangerous
construct. I put false-positive in quotes, because with both the presumption
is that this is a sufficiently confusing construct to merit clarification
from the programmer in either direction.

For both of these, the real question is what the false positive rate is once
widely used as people do development...

<snip>

For the && versus || warning, 'silence the warning' is presumably the most
> likely response (since everyone should get it right /at least/ half the
> time).


There are lots of factors here. I actually view this warning as "I didn't
group these explicitly" so its always a problem in my code. The reader will
be confused otherwise even if the program happens to behave correctly.

Also, given that pattern, I tend to always group them explicitly. I suspect
this is a fairly common trend.

Once that trend is established, the most common way I encounter this warning
is by *changing* a set of logical operations without re-grouping them. In
these cases I really do think its completely random whether or not the
change breaks the code.


> I don't think we should strive to avoid such warnings (but I'm not
> fussed about the order the notes appear for that one.)


Sure, but it's important to note that such warnings do have a high bar: it
has to be reasonable to require an explicit clarification of intent, even if
the behavior would have been correct. That's not to say we shouldn't have
such warnings, just that they need to be suitably justified.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110621/dc71a1db/attachment.html>