[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:14010
+      << Bop->getSourceRange() << OpLoc;
+  SuggestParentheses(Self, Bop->getOperatorLoc(),
+                     Self.PDiag(diag::note_precedence_silence)
----------------
vabridgers wrote:
> vabridgers wrote:
> > lebedev.ri wrote:
> > > Should we also suggest the fix to rewrite into what user likely intended?
> > > `(x op1 y) && (y op2 z)`
> > I'll work on this, post in a next update. Thank you!
> Hi @lebedev.ri , the warning emits a note that says "place parentheses around the '<op>' expression to silence this warning" (see the test cases below). Is this sufficient, or are you looking for something else?
https://godbolt.org/z/d1dG1o
For the very similar situation `(x & 1 == 0)`, Clang suggests //two different fixits//, and I believe Roman was suggesting that you should do the same kind of thing here.
```
<source>:3:16: warning: & has lower precedence than ==; == will be evaluated first [-Wparentheses]
    int y = (x & 1 == 0);
               ^~~~~~~~
<source>:3:16: note: place parentheses around the '==' expression to silence this warning
    int y = (x & 1 == 0);
               ^
                 (     )
<source>:3:16: note: place parentheses around the & expression to evaluate it first
    int y = (x & 1 == 0);
               ^
             (    )
```
For our situation here it would be something like
```
<source>:3:16: warning: chained comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z' [-Wcompare-op-parentheses]
    int w = (x < y < z);
               ^~~~~~~~
<source>:3:16: note: place parentheses around the first '<' expression to silence this warning
    int w = (x < y < z);
             ^
             (    )
<source>:3:16: note: separate the expression into two clauses to give it the mathematical meaning
    int y = (x < y < z);
               ^
             (    ) && (y    )
```
Watch out that you don't suggest a wrong fixit for `(0 <= sideeffect() < 10)`, though. I seem to recall another warning that recently had a lot of trouble phrasing its fixit in a way that wouldn't invoke UB or change the behavior of the code in corner cases, but I forget the details.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85097/new/

https://reviews.llvm.org/D85097