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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
