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

[Daniel Marjamäki via cfe-commits]

danielmarjamaki marked an inline comment as done.

================
Comment at: clang-tidy/misc/LongCastCheck.cpp:21
@@ +20,3 @@
+  Finder->addMatcher(
+      returnStmt(
+          has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"),
----------------
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.


================
Comment at: clang-tidy/misc/LongCastCheck.cpp:22
@@ +21,3 @@
+      returnStmt(
+          has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"),
+                                                      hasOperatorName("*"),
----------------
alexfh wrote:
> Why only c-style casts? The problem applies to static_cast as well. Not sure how likely a reinterpret_cast is to appear in this situation, maybe it should be handled as well.
ok, I will handle that too in next patch.

================
Comment at: clang-tidy/misc/LongCastCheck.cpp:23
@@ +22,3 @@
+          has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"),
+                                                      hasOperatorName("*"),
+                                                      hasOperatorName("<<")))))
----------------
alexfh wrote:
> Why other operators are not considered here? Subtraction, for example, can suffer the same problem.
yes. true. the ~ operator also. I can't say how noisy that would be but I will test it.

================
Comment at: clang-tidy/misc/MiscTidyModule.cpp:61
@@ -59,1 +60,3 @@
+    CheckFactories.registerCheck<LongCastCheck>(
+        "misc-long-cast");
     CheckFactories.registerCheck<MacroParenthesesCheck>(
----------------
alexfh wrote:
> danielmarjamaki wrote:
> > LegalizeAdulthood wrote:
> > > The documentation describes this check as one that looks for a cast to a "bigger type", but the name of the check implies that it only works for casts to `long`.
> > > 
> > > The name of the check should be made more generic to reflect reality.
> > > 
> > > Perhaps `misc-redundant-cast-to-larger-type` or `misc-redundant-bigger-type-cast`?
> > Yes I agree.. will fix..
> > 
> > I used long because that is the typical case that I envision.
> How about misc-misplaced-widening-cast?
I already changed.. but I like misc-misplaced-widening-cast better so I will change again..


Repository:
  rL LLVM

http://reviews.llvm.org/D16310