[PATCH] D16310: new clang-tidy checker misc-long-cast

[Alexander Kornienko via cfe-commits]

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