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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
