[PATCH] D17987: [clang-tidy] Extension of checker misc-misplaced-widening-cast

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp:64
@@ -63,3 +63,3 @@
       // bool.
-      !match(findAll(callExpr(hasAnyArgument(DeclRef))), *If, *Result.Context)
+      !match(findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(DeclRef)))), *If, *Result.Context)
            .empty() ||
----------------
The line exceeds 80 columns limit. Please clang-format the change.

================
Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:98
@@ +97,3 @@
+  const auto *Cast = Result.Nodes.getNodeAs<CastExpr>("Cast");
+  if (!CheckImplicitCasts && dyn_cast<ImplicitCastExpr>(Cast))
+    return;
----------------
`isa<T>` should be used instead of `dyn_cast<T>`, when you don't need the resulting pointer.

================
Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:119
@@ -98,4 +118,3 @@
   if (Context.getIntWidth(CastType) == Context.getIntWidth(CalcType)) {
-    if (CalcType->isSpecificBuiltinType(BuiltinType::Int) ||
-        CalcType->isSpecificBuiltinType(BuiltinType::UInt)) {
-      // There should be a warning when casting from int to long or long long.
+    if (CalcType->isSpecificBuiltinType(BuiltinType::SChar) ||
+        CalcType->isSpecificBuiltinType(BuiltinType::UChar) ||
----------------
`isSpecificBuiltinType` is not the best tool here. Instead, we could get the builtin type kinds and work with them (`CalcType->getAs<BuiltinType>()->getKind()` and `CastType->getAs<BuiltinType>()->getKind()` after checking that both are `BuiltinTypes`).

================
Comment at: test/clang-tidy/misc-misplaced-widening-cast.cpp:28
@@ +27,3 @@
+  l = a * b == c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast]
+  l = c == a * b;
----------------
Please truncate "is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast]" in all CHECK-MESSAGES lines but the first one.

================
Comment at: test/clang-tidy/misc-misplaced-widening-cast.cpp:41
@@ +40,3 @@
+  long l1 = n << 8;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int'
+  long l2 = (long)(n << 8);
----------------
It makes sense to leave the `to '<target type'` part in all messages.


http://reviews.llvm.org/D17987