Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -11,6 +11,7 @@ MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp MiscTidyModule.cpp + MisplacedWideningCastCheck.cpp MoveConstantArgumentCheck.cpp MoveConstructorInitCheck.cpp NewDeleteOverloadsCheck.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -19,6 +19,7 @@ #include "InefficientAlgorithmCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" +#include "MisplacedWideningCastCheck.h" #include "MoveConstantArgumentCheck.h" #include "MoveConstructorInitCheck.h" #include "NewDeleteOverloadsCheck.h" @@ -60,6 +61,8 @@ "misc-macro-parentheses"); CheckFactories.registerCheck( "misc-macro-repeated-side-effects"); + CheckFactories.registerCheck( + "misc-misplaced-widening-cast"); CheckFactories.registerCheck( "misc-move-const-arg"); CheckFactories.registerCheck( Index: clang-tidy/misc/MisplacedWideningCastCheck.h =================================================================== --- clang-tidy/misc/MisplacedWideningCastCheck.h +++ clang-tidy/misc/MisplacedWideningCastCheck.h @@ -0,0 +1,38 @@ +//===--- MisplacedWideningCastCheck.h - clang-tidy---------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISPLACED_WIDENING_CAST_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISPLACED_WIDENING_CAST_H + +#include "../ClangTidy.h" + +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. +/// +/// 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) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISPLACED_WIDENING_CAST_H Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp =================================================================== --- clang-tidy/misc/MisplacedWideningCastCheck.cpp +++ clang-tidy/misc/MisplacedWideningCastCheck.cpp @@ -0,0 +1,106 @@ +//===--- MisplacedWideningCastCheck.cpp - clang-tidy-----------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "MisplacedWideningCastCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +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"); + + Finder->addMatcher(varDecl(has(Cast)), this); + Finder->addMatcher(returnStmt(has(Cast)), this); + Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this); +} + +static unsigned getMaxCalculationWidth(ASTContext &Context, const Expr *E) { + E = E->IgnoreParenImpCasts(); + + if (const auto *Bop = dyn_cast(E)) { + unsigned LHSWidth = getMaxCalculationWidth(Context, Bop->getLHS()); + unsigned RHSWidth = getMaxCalculationWidth(Context, Bop->getRHS()); + if (Bop->getOpcode() == BO_Mul) + return LHSWidth + RHSWidth; + if (Bop->getOpcode() == BO_Add) + return std::max(LHSWidth, RHSWidth) + 1; + if (Bop->getOpcode() == BO_Rem) { + llvm::APSInt Val; + if (Bop->getRHS()->EvaluateAsInt(Val, Context)) + return Val.getActiveBits(); + } else if (Bop->getOpcode() == BO_Shl) { + llvm::APSInt Bits; + if (Bop->getRHS()->EvaluateAsInt(Bits, Context)) { + // We don't handle negative values and large values well. It is assumed + // that compiler warnings are written for such values so the user will + // fix that. + return LHSWidth + Bits.getExtValue(); + } + + // Unknown bitcount, assume there is truncation. + return 1024U; + } + } else if (const auto *Uop = dyn_cast(E)) { + // There is truncation when ~ is used. + if (Uop->getOpcode() == UO_Not) + return 1024U; + + QualType T = Uop->getType(); + return T->isIntegerType() ? Context.getIntWidth(T) : 1024U; + } else if (const auto *I = dyn_cast(E)) { + return I->getValue().getActiveBits(); + } + + return Context.getIntWidth(E->getType()); +} + +void MisplacedWideningCastCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Cast = Result.Nodes.getNodeAs("Cast"); + if (Cast->getLocStart().isMacroID()) + return; + + const auto *Calc = Result.Nodes.getNodeAs("Calc"); + if (Calc->getLocStart().isMacroID()) + return; + + ASTContext &Context = *Result.Context; + + QualType CastType = Cast->getType(); + QualType CalcType = Calc->getType(); + + if (Context.getIntWidth(CastType) <= Context.getIntWidth(CalcType)) + return; + + if (Context.getIntWidth(CalcType) >= getMaxCalculationWidth(Context, Calc)) + return; + + diag(Cast->getLocStart(), "either cast from %0 to %1 is ineffective, or " + "there is loss of precision before the conversion") + << CalcType << CastType; +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -52,6 +52,7 @@ misc-inefficient-algorithm misc-macro-parentheses misc-macro-repeated-side-effects + misc-misplaced-widening-cast misc-move-constructor-init misc-new-delete-overloads misc-noexcept-move-constructor 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 @@ -0,0 +1,39 @@ +.. title:: clang-tidy - misc-misplaced-widening-cast + +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. + +Example code:: + + long f(int x) { + return (long)(x*1000); + } + +The result x*1000 is first calculated using int precision. If the result +exceeds int precision there is loss of precision. Then the result is casted to +long. + +If there is no loss of precision then the cast can be removed or you can +explicitly cast to int instead. + +If you want to avoid loss of precision then put the cast in a proper location, +for instance:: + + long f(int x) { + return (long)x * 1000; + } + +Floating point +-------------- + +Currently warnings are only written for integer conversion. No warning is +written for this code:: + + double f(float x) { + return (double)(x * 10.0f); + } 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 @@ -0,0 +1,64 @@ +// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t + +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 = (long)(a << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = (long)b << 8; + + l = static_cast(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] +} + +void init(unsigned int n) { + long l = (long)(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'unsigned int' +} + +long ret(int a) { + return (long)(a * 1000); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: either cast from 'int' to 'long' +} + +void dontwarn1(unsigned char a, int i, unsigned char *p) { + long l; + // the result is a 9 bit value, there is no truncation in the implicit cast. + l = (long)(a + 15); + // 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); + // the result is a 16 bit value, there is no truncation in the implicit cast. + l = (long)(((*p)<<8) + *(p+1)); +} + +template struct DontWarn2 { + void assign(T a, T b) { + long l; + l = (long)(a * b); + } +}; +DontWarn2 DW2; + +// Cast is not suspicious when casting macro. +#define A (X<<2) +long macro1(int X) { + return (long)A; +} + +// Don't warn about cast in macro. +#define B(X,Y) (long)(X*Y) +long macro2(int x, int y) { + return B(x,y); +} + +void floatingpoint(float a, float b) { + double d = (double)(a * b); // Currently we don't warn for this. +} +