<div dir="ltr"><div dir="ltr">On Mon, May 18, 2020 at 1:50 PM Roman Lebedev via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> On Mon, May 18, 2020 at 6:39 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>>> On Mon, May 18, 2020 at 6:53 AM Billy Araujo via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
>>><br>
>>> -Wparentheses warns for line 3 but not for the ternary expression in line 2.<br>
>>><br>
>>> 1 static void foo(int a, int b, int x) {<br>
>>> 2    x = (x = 10) ? a : b;<br>
>>> 3    if (x = 10) { x = a; } else { x = b; }<br>
>>> 4 }<br>
>>><br>
>>> Is this a bug?<br>
This seems pretty correct to me.<br>
<br>
You can't avoid braces on line 3, so those braces don't count as extra braces<br>
to silence the diagnostic, while the situation on the line 2 is opposite,<br>
no braces are needed there, so the braces count as silencers.<br>
<br>
I.e. if line 2 *would* be diagnosed, i'd say that is a false-positive.<br></blockquote><div><br></div><div>Besides what David subsequently pointed out — that the parens in `(x = 10) ? a : b` <i>do</i> 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 <i>any number of parens more than the physically minimal amount</i>. The criterion is that it should not warn if the number of parens is high enough to indicate that <i>the user is aware of the issue and actively trying to suppress the warning</i>.</div><div><br></div><div>For example,</div><div>    if ((x < 5) || (x = 7)) ;</div><div>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.</div><div>    if ((x = 7)) ;</div><div>historically does not warn, and by this criterion <i>should</i> not warn. Why? Well, because if we imagine that the user had written `==` instead of `=`, we'd get</div><div>    if ((x == 7)) ;</div><div>which has an unidiomatic extra pair of parens. This indicates that the user must be going out of their way to suppress the diagnostic.</div><div><br></div><div>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.</div><div><br></div><div>–Arthur</div></div></div>