njames93 added inline comments.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:14047
+static bool isComparisonOpSamePrecedence(BinaryOperatorKind Opc) {
+ switch (Opc) {
----------------
Quuxplusone wrote:
> Same precedence as what?
> I think this should just be called `isRelationalOperator`, and I would bet
> money that such a classifier function already exists somewhere in the
> codebase.
`isRelationalOp` and its a member function of `BinaryOperator` so there is no
need to even access the OperatorKind.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:14010
+ << Bop->getSourceRange() << OpLoc;
+ SuggestParentheses(Self, Bop->getOperatorLoc(),
+ Self.PDiag(diag::note_precedence_silence)
----------------
Quuxplusone wrote:
> 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.
Surely you'd just need to check `y` for side effects before creating the fix-it.
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
