[clang-tools-extra] r265532 - [clang-tidy] Extension of checker misc-misplaced-widening-cast
Gabor Horvath via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 6 05:04:52 PDT 2016
Author: xazax
Date: Wed Apr 6 07:04:51 2016
New Revision: 265532
URL: http://llvm.org/viewvc/llvm-project?rev=265532&view=rev
Log:
[clang-tidy] Extension of checker misc-misplaced-widening-cast
Summary:
Existing checker misc-misplaced-widening-cast was extended:
- New use cases: casted expression as lhs or rhs of a logical comparison or function argument
- New types: beside int, long and long long various char types, short and int128 added
- New option to check implicit casts: forgetting a cast is at least as common and as dangerous as misplacing it. This option can be disabled.
This patch depends on AST Matcher patches D17986 and D18243 and also contains fix for checker misc-bool-pointer-implicit-conversion needed because of the fix in the AST Matcher patch.
Reviewers: hokein, alexfh
Subscribers: o.gyorgy, xazax.hun, cfe-commits
Differential Revision: http://reviews.llvm.org/D17987
Added:
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp?rev=265532&r1=265531&r2=265532&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp Wed Apr 6 07:04:51 2016
@@ -61,7 +61,8 @@ void BoolPointerImplicitConversionCheck:
*Result.Context).empty() ||
// FIXME: We should still warn if the paremater is implicitly converted to
// bool.
- !match(findAll(callExpr(hasAnyArgument(DeclRef))), *If, *Result.Context)
+ !match(findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(DeclRef)))),
+ *If, *Result.Context)
.empty() ||
!match(findAll(cxxDeleteExpr(has(expr(DeclRef)))), *If, *Result.Context)
.empty())
Modified: clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp?rev=265532&r1=265531&r2=265532&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp Wed Apr 6 07:04:51 2016
@@ -10,6 +10,7 @@
#include "MisplacedWideningCastCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/DenseMap.h"
using namespace clang::ast_matchers;
@@ -17,6 +18,16 @@ namespace clang {
namespace tidy {
namespace misc {
+MisplacedWideningCastCheck::MisplacedWideningCastCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ CheckImplicitCasts(Options.get("CheckImplicitCasts", true)) {}
+
+void MisplacedWideningCastCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "CheckImplicitCasts", CheckImplicitCasts);
+}
+
void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) {
auto Calc = expr(anyOf(binaryOperator(anyOf(
hasOperatorName("+"), hasOperatorName("-"),
@@ -25,14 +36,22 @@ void MisplacedWideningCastCheck::registe
hasType(isInteger()))
.bind("Calc");
- auto Cast = explicitCastExpr(anyOf(cStyleCastExpr(), cxxStaticCastExpr(),
- cxxReinterpretCastExpr()),
- hasDestinationType(isInteger()), has(Calc))
- .bind("Cast");
-
- Finder->addMatcher(varDecl(has(Cast)), this);
- Finder->addMatcher(returnStmt(has(Cast)), this);
+ auto ExplicitCast =
+ explicitCastExpr(hasDestinationType(isInteger()), has(Calc));
+ auto ImplicitCast =
+ implicitCastExpr(hasImplicitDestinationType(isInteger()), has(Calc));
+ auto Cast = expr(anyOf(ExplicitCast, ImplicitCast)).bind("Cast");
+
+ Finder->addMatcher(varDecl(hasInitializer(Cast)), this);
+ Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this);
+ Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this);
Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this);
+ Finder->addMatcher(
+ binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!="),
+ hasOperatorName("<"), hasOperatorName("<="),
+ hasOperatorName(">"), hasOperatorName(">=")),
+ anyOf(hasLHS(Cast), hasRHS(Cast))),
+ this);
}
static unsigned getMaxCalculationWidth(ASTContext &Context, const Expr *E) {
@@ -75,8 +94,72 @@ static unsigned getMaxCalculationWidth(A
return Context.getIntWidth(E->getType());
}
+static llvm::SmallDenseMap<int, int> createRelativeIntSizesMap() {
+ llvm::SmallDenseMap<int, int> Result(14);
+ Result[BuiltinType::UChar] = 1;
+ Result[BuiltinType::SChar] = 1;
+ Result[BuiltinType::Char_U] = 1;
+ Result[BuiltinType::Char_S] = 1;
+ Result[BuiltinType::UShort] = 2;
+ Result[BuiltinType::Short] = 2;
+ Result[BuiltinType::UInt] = 3;
+ Result[BuiltinType::Int] = 3;
+ Result[BuiltinType::ULong] = 4;
+ Result[BuiltinType::Long] = 4;
+ Result[BuiltinType::ULongLong] = 5;
+ Result[BuiltinType::LongLong] = 5;
+ Result[BuiltinType::UInt128] = 6;
+ Result[BuiltinType::Int128] = 6;
+ return Result;
+}
+
+static llvm::SmallDenseMap<int, int> createRelativeCharSizesMap() {
+ llvm::SmallDenseMap<int, int> Result(6);
+ Result[BuiltinType::UChar] = 1;
+ Result[BuiltinType::SChar] = 1;
+ Result[BuiltinType::Char_U] = 1;
+ Result[BuiltinType::Char_S] = 1;
+ Result[BuiltinType::Char16] = 2;
+ Result[BuiltinType::Char32] = 3;
+ return Result;
+}
+
+static llvm::SmallDenseMap<int, int> createRelativeCharSizesWMap() {
+ llvm::SmallDenseMap<int, int> Result(6);
+ Result[BuiltinType::UChar] = 1;
+ Result[BuiltinType::SChar] = 1;
+ Result[BuiltinType::Char_U] = 1;
+ Result[BuiltinType::Char_S] = 1;
+ Result[BuiltinType::WChar_U] = 2;
+ Result[BuiltinType::WChar_S] = 2;
+ return Result;
+}
+
+static bool isFirstWider(BuiltinType::Kind First, BuiltinType::Kind Second) {
+ static const llvm::SmallDenseMap<int, int> RelativeIntSizes(
+ createRelativeIntSizesMap());
+ static const llvm::SmallDenseMap<int, int> RelativeCharSizes(
+ createRelativeCharSizesMap());
+ static const llvm::SmallDenseMap<int, int> RelativeCharSizesW(
+ createRelativeCharSizesWMap());
+
+ int FirstSize, SecondSize;
+ if ((FirstSize = RelativeIntSizes.lookup(First)) &&
+ (SecondSize = RelativeIntSizes.lookup(Second)))
+ return FirstSize > SecondSize;
+ if ((FirstSize = RelativeCharSizes.lookup(First)) &&
+ (SecondSize = RelativeCharSizes.lookup(Second)))
+ return FirstSize > SecondSize;
+ if ((FirstSize = RelativeCharSizesW.lookup(First)) &&
+ (SecondSize = RelativeCharSizesW.lookup(Second)))
+ return FirstSize > SecondSize;
+ return false;
+}
+
void MisplacedWideningCastCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *Cast = Result.Nodes.getNodeAs<ExplicitCastExpr>("Cast");
+ const auto *Cast = Result.Nodes.getNodeAs<CastExpr>("Cast");
+ if (!CheckImplicitCasts && isa<ImplicitCastExpr>(Cast))
+ return;
if (Cast->getLocStart().isMacroID())
return;
@@ -95,24 +178,15 @@ void MisplacedWideningCastCheck::check(c
// If CalcType and CastType have same size then there is no real danger, but
// there can be a portability problem.
+
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());
+ const auto *CalcBuiltinType =
+ dyn_cast<BuiltinType>(CalcType->getUnqualifiedDesugaredType());
+ if (CastBuiltinType && CalcBuiltinType &&
+ !isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
return;
- }
}
// Don't write a warning if we can easily see that the result is not
Modified: clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.h?rev=265532&r1=265531&r2=265532&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.h Wed Apr 6 07:04:51 2016
@@ -16,19 +16,27 @@ namespace clang {
namespace tidy {
namespace misc {
-/// Find explicit redundant casts of calculation results to bigger type.
-/// Typically from int to long. If the intention of the cast is to avoid loss
-/// of precision then the cast is misplaced, and there can be loss of
-/// precision. Otherwise such cast is ineffective.
+/// Find casts of calculation results to bigger type. Typically from int to
+/// long. If the intention of the cast is to avoid loss of precision then
+/// the cast is misplaced, and there can be loss of precision. Otherwise
+/// such cast is ineffective.
+///
+/// There is one option:
+///
+/// - `CheckImplicitCasts`: Whether to check implicit casts as well which may
+// be the most common case. Enabled by default.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/misc-misplaced-widening-cast.html
class MisplacedWideningCastCheck : public ClangTidyCheck {
public:
- MisplacedWideningCastCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ MisplacedWideningCastCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const bool CheckImplicitCasts;
};
} // namespace misc
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-misplaced-widening-cast.rst?rev=265532&r1=265531&r2=265532&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-misplaced-widening-cast.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-misplaced-widening-cast.rst Wed Apr 6 07:04:51 2016
@@ -3,10 +3,10 @@
misc-misplaced-widening-cast
============================
-This check will warn when there is a explicit redundant cast of a calculation
-result to a bigger type. If the intention of the cast is to avoid loss of
-precision then the cast is misplaced, and there can be loss of precision.
-Otherwise the cast is ineffective.
+This check will warn when there is a cast of a calculation result to a bigger
+type. If the intention of the cast is to avoid loss of precision then the cast
+is misplaced, and there can be loss of precision. Otherwise the cast is
+ineffective.
Example code::
@@ -28,6 +28,17 @@ for instance::
return (long)x * 1000;
}
+Implicit casts
+--------------
+
+Forgetting to place the cast at all is at least as dangerous and at least as
+common as misplacing it. If option ``CheckImplicitCasts`` is enabled (default)
+the checker also detects these cases, for instance::
+
+ long f(int x) {
+ return x * 1000;
+ }
+
Floating point
--------------
Added: clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp?rev=265532&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp Wed Apr 6 07:04:51 2016
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, value: 0}]}" --
+
+void func(long arg) {}
+
+void assign(int a, int b) {
+ long l;
+
+ l = a * b;
+ l = (long)(a * b);
+ // 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 = (long)a * b;
+
+ l = a << 8;
+ l = (long)(a << 8);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+ l = (long)b << 8;
+
+ l = static_cast<long>(a * b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+}
+
+void compare(int a, int b, long c) {
+ bool l;
+
+ l = a * b == c;
+ l = c == a * b;
+ l = (long)(a * b) == c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+ l = c == (long)(a * b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+ l = (long)a * b == c;
+ l = c == (long)a * b;
+}
+
+void init(unsigned int n) {
+ long l1 = n << 8;
+ long l2 = (long)(n << 8);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
+ long l3 = (long)n << 8;
+}
+
+void call(unsigned int n) {
+ func(n << 8);
+ func((long)(n << 8));
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
+ func((long)n << 8);
+}
+
+long ret(int a) {
+ if (a < 0) {
+ return a * 1000;
+ } else if (a > 0) {
+ return (long)(a * 1000);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+ } else {
+ return (long)a * 1000;
+ }
+}
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp?rev=265532&r1=265531&r2=265532&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp Wed Apr 6 07:04:51 2016
@@ -1,29 +1,67 @@
-// 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}]}" --
+
+void func(long arg) {}
void assign(int a, int b) {
long l;
l = a * b;
- l = (long)(a * b);
// 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 = (long)(a * b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = (long)a * b;
+ l = a << 8;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = (long)(a << 8);
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = (long)b << 8;
l = static_cast<long>(a * b);
- // 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]
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+}
+
+void compare(int a, int b, long c) {
+ bool l;
+
+ l = a * b == c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+ l = c == a * b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+ l = (long)(a * b) == c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+ l = c == (long)(a * b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+ l = (long)a * b == c;
+ l = c == (long)a * b;
}
void init(unsigned int n) {
- long l = (long)(n << 8);
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'unsigned int'
+ long l1 = n << 8;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
+ long l2 = (long)(n << 8);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
+ long l3 = (long)n << 8;
+}
+
+void call(unsigned int n) {
+ func(n << 8);
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
+ func((long)(n << 8));
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
+ func((long)n << 8);
}
long ret(int a) {
- return (long)(a * 1000);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: either cast from 'int' to 'long'
+ if (a < 0) {
+ return a * 1000;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+ } else if (a > 0) {
+ return (long)(a * 1000);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+ } else {
+ return (long)a * 1000;
+ }
}
void dontwarn1(unsigned char a, int i, unsigned char *p) {
@@ -33,9 +71,9 @@ void dontwarn1(unsigned char a, int i, u
// The result is a 12 bit value, there is no truncation in the implicit cast.
l = (long)(a << 4);
// The result is a 3 bit value, there is no truncation in the implicit cast.
- l = (long)((i%5)+1);
+ l = (long)((i % 5) + 1);
// The result is a 16 bit value, there is no truncation in the implicit cast.
- l = (long)(((*p)<<8) + *(p+1));
+ l = (long)(((*p) << 8) + *(p + 1));
}
template <class T> struct DontWarn2 {
More information about the cfe-commits
mailing list