[cfe-dev] Strange difference between Clang and GCC's -Wparentheses

[dblaikie at gmail.com]

For context, here's Agyrios's commit that implemented this functionality:
http://llvm.org/viewvc/llvm-project?view=revision&revision=119533 - I
personally don't have email history going back that far to know if this was
the result of some on-list review/discussion to provide more context to the
decision.

My thoughts from the peanut gallery follow (but I don't want them to be a
loud voice since I think Apple probably has some relevant input here)

On Mon Jan 06 2014 at 11:52:58 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> Currently Clang and GCC warn about:
>
> (a || b && c)
>
> Due to the very common mistake by programmers which ignores the relative
> operator precedence of || and &&. This forces the user to place explicit
> parentheses to group either (a || b) or (b && c) to be evaluated first.
>
> However, GCC warns and Clang is silent about this:
>
> (a || b && 1)
>
> Worse, the same is true for:
>
> (a || b && "foo")
>
> Now, I find Clang's rationale seems somewhat reasonable: the grouping
> doesn't change the truth table for this expression. However, I find this
> clever interpretation problematic for several reasons:
>
> 1) It's a complex rule to teach programmers with dubious gains. This
> exception doesn't realistically make the warning's false positives rare, it
> just eliminates one common (but not the only common) pattern. On the flip
> side, it is not a trivial rule to teach or predict for programmers. *Any*
> constant that we can evaluate to true will satisfy this.
>

I've never really thought of teaching these rules to people. But maybe
people are more habit-driven than I am - I tend to just write the code &
stick in the parans when the compiler tells me (or I'm sufficiently
confused by the precedence myself - which perhaps comes first so I don't
see these warnings much even though I do tend to write rather dense code
that can include many conditions).


> 2) If the programmer expected the || to be evaluated first, they might
> well also expect the "a" expression to be evaluated first and short
> circuiting to take place. Even though the truth table is unimpacted,
> 'assert(!ptr || ptr->empty() && "foo");' can segfault, which I think might
> surprise users.
>

How could that code segfault? the first of the three expressions should
cause the shortcircuit no matter which parse occurred.


> 3) It makes it annoying to track warnings in both Clang and GCC. If you
> mostly develop with Clang, but someone else checks out your code with GCC
> it'll suddenly start warning (much like it just did in a recent commit.)
>

Echoing my comments on IRC: Our usual approach is to disable GCC's warning
if this sort of thing comes up. But how did this not fail on our buildbots?
Do we already have GCC's -Wparentheses disabled on them for this or other
reasons?


> Thoughts? I'm happy to fix this, the change is trivial in SemaExpr.cpp,
> but wanted to get some confirmation from Richard and Argyrios who have
> worked on this warning that such a change would be OK.
>
> -Chandler
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140107/e2591f26/attachment.html>