aaron.ballman added inline comments.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:27
@@ -25,1 +26,3 @@
+static bool incrementWithoutOverflow(const llvm::APSInt &Value,
+ llvm::APSInt &Result) {
----------------
I think this could be implemented using APInt overflow checks, no?
`APInt::sadd_ov()`?
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:138
@@ +137,3 @@
+ incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
+ llvm::APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
+ return true;
----------------
Ah, this may be confusion on my part. I was thinking "equivalent ranges"
meaning "does one range cover another range", e.g., `x < 12` is also covered by
`x < 4` in an expression like `if (x < 4 && x < 12)`.
I think this code is fine as-is.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:274
@@ +273,3 @@
+ Value = -Value;
+ }
+}
----------------
> In this case -128 (8-bits) will give -128.
So negating -128 doesn't yield 128, it yields -128? That seems weird.
> The APSInt is behaving the same way than a real value of the same width and
> signedness.
A real value of the same width and signedness has UB with that case, which is
why I was asking. The range of an 8-bit signed int is -128 to 127, so negating
-128 yields an out-of-range value. I want to make sure we aren't butchering
that by canonicalizing the negate expression.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.h:31
@@ +30,3 @@
+private:
+ void checkArithmeticExpr(const ast_matchers::MatchFinder::MatchResult &R);
+ void checkBitwiseExpr(const ast_matchers::MatchFinder::MatchResult &R);
----------------
Hmmm, good point. Drat!
http://reviews.llvm.org/D21392
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
