[cfe-dev] parentheses flag warning

[Arthur O'Dwyer via cfe-dev]

On Mon, May 18, 2020 at 1:50 PM Roman Lebedev via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> > On Mon, May 18, 2020 at 6:39 PM David Blaikie <dblaikie at gmail.com>
> wrote:
> >> On Mon, May 18, 2020 at 6:53 AM Billy Araujo via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>>
> >>> -Wparentheses warns for line 3 but not for the ternary expression in
> line 2.
> >>>
> >>> 1 static void foo(int a, int b, int x) {
> >>> 2    x = (x = 10) ? a : b;
> >>> 3    if (x = 10) { x = a; } else { x = b; }
> >>> 4 }
> >>>
> >>> Is this a bug?
> This seems pretty correct to me.
>
> You can't avoid braces on line 3, so those braces don't count as extra
> braces
> to silence the diagnostic, while the situation on the line 2 is opposite,
> no braces are needed there, so the braces count as silencers.
>
> I.e. if line 2 *would* be diagnosed, i'd say that is a false-positive.
>

Besides what David subsequently pointed out — that the parens in `(x = 10)
? a : b` *do* affect the precedence — I believe you're looking at
-Wparentheses in the wrong light. The criterion for avoiding false
positives is not that it should never warn about *any number of parens more
than the physically minimal amount*. The criterion is that it should not
warn if the number of parens is high enough to indicate that *the user is
aware of the issue and actively trying to suppress the warning*.

For example,
    if ((x < 5) || (x = 7)) ;
should warn, because it is "quite obvious" that the user meant (x == 7),
not (x = 7).  How do we know?  Well, we imagine that the user had written
`==` instead of `=`, and we look to see whether the expression would have
been idiomatic.
    if ((x = 7)) ;
historically does not warn, and by this criterion *should* not warn. Why?
Well, because if we imagine that the user had written `==` instead of `=`,
we'd get
    if ((x == 7)) ;
which has an unidiomatic extra pair of parens. This indicates that the user
must be going out of their way to suppress the diagnostic.

Neither GCC nor Clang currently warn on `((x < 5) || (x = 7))`. I guarantee
that as soon as you start warning about it, you'll uncover real bugs in
real codebases.

–Arthur
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200519/a9917ba2/attachment.html>