alexfh added a comment.
A few nits.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:135
@@ +134,3 @@
+ llvm::APSInt ValueLHS_plus1;
+ if (((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) ||
+ (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) &&
----------------
How about replacing `if (x) return true; return false;` with `return x;`?
BTW, next function looks fine in this regard, since it has a chain of `if (x)
return true; if (y) return true; ...`.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:182
@@ +181,3 @@
+ incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
+ llvm::APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0) {
+ return true;
----------------
Remove braces for consistency.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:191
@@ +190,3 @@
+// expressions covers the whole domain (i.e. x < 10 and x > 0).
+static bool doRangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
+ const llvm::APSInt &ValueLHS,
----------------
I'd slightly prefer it without "do": `rangesFullyCoverDomain`,
`rangeSubsumesRange`, etc.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:266
@@ +265,3 @@
+ }
+ return false;
+}
----------------
The last return seems to be dead code.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:364
@@ +363,3 @@
+
+ // A cast can be matched as a comparator to zero.
+ const auto CastExpr =
----------------
Not sure I understand this comment.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:628
@@ +627,3 @@
+
+ llvm::APSInt LhsValue, RhsValue;
+ const Expr *LhsSymbol = nullptr;
----------------
`using llvm::APSInt;` to remove some boilerplate?
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:651
@@ +650,3 @@
+ if (LhsOpcode == BO_Or && (LhsConstant | RhsConstant) != RhsConstant) {
+ if (Opcode == BO_EQ)
+ diag(ComparisonOperator->getOperatorLoc(),
----------------
Please add braces, when the body is more than one line.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:655
@@ +654,3 @@
+ else if (Opcode == BO_NE)
+ diag(ComparisonOperator->getOperatorLoc(),
+ "logical expression is always true");
----------------
`ComparisonOperator->getOperatorLoc()` and the message are repeated too many
times. Pull them to local variables / constants?
http://reviews.llvm.org/D21392
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
