njames93 added a comment.
For the record `X < Y < Z ` does have a mathematical meaning, Y is constrained
between X and Z.
However in the context of `C` the expression isnt parsed like that.
If someone writes this they likely wanted `(X < Y) && (Y < Z)`
For this specific check as you pointed out we wouldn't want to make that
assumption though there is a case for adding notes to silence the warning by
wrapping one of the comparisons in parenthesis.
It appears that this check fire multiple times for the case of `W < X < Y < Z`
Once for `(W < X < Y) < Z` and another time for `(W < X) < Y`
This again likely wont be visible as the warning currently is emitted at the
start of the expression
================
Comment at: clang-tools-extra/clang-tidy/misc/ComplexConditionsCheck.cpp:28-33
+ const auto *If = Result.Nodes.getNodeAs<IfStmt>("complex_binop");
+ const auto *Statement = Result.Nodes.getNodeAs<Stmt>("complex_binop");
+ if (If) {
+ diag(If->getBeginLoc(),
+ "comparisons like `X<=Y<=Z` have no mathematical meaning");
+ }
----------------
The `If` looks suspicious, as you match on a `BinaryOperator`, the `If` will
always be nullptr and should likely be removed.
Likewise `Statement` will match but should likely be changed to BinOp as that's
what it is.
================
Comment at: clang-tools-extra/clang-tidy/misc/ComplexConditionsCheck.cpp:34
+ }
+ if (Statement) {
+ diag(Statement->getBeginLoc(),
----------------
Elide braces
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84898/new/
https://reviews.llvm.org/D84898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
