baloghadamsoftware updated this revision to Diff 51633.
baloghadamsoftware added a comment.
Requested revision done.
http://reviews.llvm.org/D17987
Files:
clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
clang-tidy/misc/MisplacedWideningCastCheck.cpp
clang-tidy/misc/MisplacedWideningCastCheck.h
docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
test/clang-tidy/misc-misplaced-widening-cast.cpp
Index: test/clang-tidy/misc-misplaced-widening-cast.cpp
===================================================================
--- test/clang-tidy/misc-misplaced-widening-cast.cpp
+++ test/clang-tidy/misc-misplaced-widening-cast.cpp
@@ -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 @@
// 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 {
Index: test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
@@ -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;
+ }
+}
Index: docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
===================================================================
--- docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
+++ docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
@@ -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 @@
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
--------------
Index: clang-tidy/misc/MisplacedWideningCastCheck.h
===================================================================
--- clang-tidy/misc/MisplacedWideningCastCheck.h
+++ clang-tidy/misc/MisplacedWideningCastCheck.h
@@ -16,19 +16,27 @@
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
Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp
===================================================================
--- clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -10,29 +10,48 @@
#include "MisplacedWideningCastCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/DenseMap.h"
using namespace clang::ast_matchers;
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("-"),
hasOperatorName("*"), hasOperatorName("<<"))),
unaryOperator(hasOperatorName("~"))),
hasType(isInteger()))
.bind("Calc");
- auto Cast = explicitCastExpr(anyOf(cStyleCastExpr(), cxxStaticCastExpr(),
- cxxReinterpretCastExpr()),
- hasDestinationType(isInteger()), has(Calc))
- .bind("Cast");
+ 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(has(Cast)), this);
- Finder->addMatcher(returnStmt(has(Cast)), this);
+ 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 @@
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 @@
// 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
Index: clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
===================================================================
--- clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
+++ clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
@@ -61,7 +61,8 @@
*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())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
