alexfh added inline comments.
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:21
@@ +20,3 @@
+ Finder->addMatcher(
+ returnStmt(
+ has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"),
----------------
danielmarjamaki wrote:
> alexfh wrote:
> > Any reason to limit this to returnStmt, varDecl and assignment? This
> > pattern can appear in any expression and lead to equally incorrect results.
> Yes. There could be some extra pattern we want to look for later. But I don't
> want to warn in general. Example:
>
> int A,B,C;
> long ABC = (long)(A * B) + C;
>
> That code makes perfect sense if the calculation A*B won't overflow and you
> want that the addition is done with long precision.
>
Then you don't need to repeat the inner matcher. Assign it to an auto variable
and then use it in the other matchers:
auto CastExpr = cStyleCastExpr(...);
Finder->addMatcher(returnStmt(has(CastExpr)));
Finder->addMatcher(varDecl(has(CastExpr)));
...
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:55
@@ +54,3 @@
+ llvm::APSInt Val;
+ if (Bop->getRHS()->EvaluateAsInt(Val, C)) {
+ return Val.getActiveBits();
----------------
nit: Braces are not needed around one-line `if` bodies.
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:61
@@ +60,3 @@
+ llvm::APSInt Bits;
+ if (Bop->getRHS()->EvaluateAsInt(Bits, C))
+ // We don't handle negative values and large values well. It is assumed
----------------
Please insert braces around the `if` body here: it's hard to see its scope
otherwise.
Repository:
rL LLVM
http://reviews.llvm.org/D16310
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
