[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/MisplacedWideningCastCheck.cpp:116
@@ +115,3 @@
+
+  if (RelativeIntSizes.find(First) != RelativeIntSizes.end() &&
+      RelativeIntSizes.find(Second) != RelativeIntSizes.end()) {
----------------
This code shouldn't repeat the lookups. You can, for example, use iterators to keep the results of `.find()`.

It also seems more appropriate to use `SmallDenseMap`.

================
Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:154-155
@@ -98,16 +153,4 @@
+
   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 (!CastType->isSpecificBuiltinType(BuiltinType::Long) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::ULong) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::LongLong) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::ULongLong))
-        return;
-    } else if (CalcType->isSpecificBuiltinType(BuiltinType::Long) ||
-               CalcType->isSpecificBuiltinType(BuiltinType::ULong)) {
-      // There should be a warning when casting from long to long long.
-      if (!CastType->isSpecificBuiltinType(BuiltinType::LongLong) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::ULongLong))
-        return;
-    } else {
+    const auto CastBuiltinType =
+        dyn_cast<BuiltinType>(CastType->getUnqualifiedDesugaredType());
----------------
Looks much better now, thanks!

================
Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:155
@@ -98,16 +154,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 (!CastType->isSpecificBuiltinType(BuiltinType::Long) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::ULong) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::LongLong) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::ULongLong))
-        return;
-    } else if (CalcType->isSpecificBuiltinType(BuiltinType::Long) ||
-               CalcType->isSpecificBuiltinType(BuiltinType::ULong)) {
-      // There should be a warning when casting from long to long long.
-      if (!CastType->isSpecificBuiltinType(BuiltinType::LongLong) &&
-          !CastType->isSpecificBuiltinType(BuiltinType::ULongLong))
-        return;
-    } else {
+    const auto CastBuiltinType =
+        dyn_cast<BuiltinType>(CastType->getUnqualifiedDesugaredType());
----------------
Please use `const auto *` to make it obvious it's a pointer.

================
Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:160
@@ -114,1 +159,3 @@
+    if (CastBuiltinType && CalcBuiltinType &&
+        !isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind())) {
       return;
----------------
No need for the braces around single-line `if` bodies.

================
Comment at: test/clang-tidy/misc-misplaced-widening-cast.cpp:1
@@ -1,1 +1,2 @@
-// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t
+// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, value: 1}]}" --
+
----------------
We should also have a test with this option turned off.


http://reviews.llvm.org/D17987