Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -8,6 +8,7 @@ DefinitionsInHeadersCheck.cpp InaccurateEraseCheck.cpp InefficientAlgorithmCheck.cpp + LongCastCheck.cpp MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp MiscTidyModule.cpp Index: clang-tidy/misc/LongCastCheck.h =================================================================== --- clang-tidy/misc/LongCastCheck.h +++ clang-tidy/misc/LongCastCheck.h @@ -0,0 +1,33 @@ +//===--- LongCastCheck.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_LONG_CAST_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_LONG_CAST_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// 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. +class LongCastCheck : public ClangTidyCheck { +public: + LongCastCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_LONG_CAST_H Index: clang-tidy/misc/LongCastCheck.cpp =================================================================== --- clang-tidy/misc/LongCastCheck.cpp +++ clang-tidy/misc/LongCastCheck.cpp @@ -0,0 +1,114 @@ +//===--- LongCastCheck.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 "LongCastCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +void LongCastCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + returnStmt( + has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"), + hasOperatorName("*"), + hasOperatorName("<<"))))) + .bind("cast"))), + this); + Finder->addMatcher( + varDecl(has(cStyleCastExpr(has(binaryOperator(anyOf( + hasOperatorName("+"), hasOperatorName("*"), + hasOperatorName("<<"))))) + .bind("cast"))), + this); + Finder->addMatcher( + binaryOperator( + hasOperatorName("="), + hasRHS(cStyleCastExpr(has(binaryOperator(anyOf( + hasOperatorName("+"), hasOperatorName("*"), + hasOperatorName("<<"))))) + .bind("cast"))), + this); +} + +static unsigned getMaxCalculationWidth(ASTContext &C, const Expr *E) { + E = E->IgnoreParenImpCasts(); + + if (const auto *Bop = dyn_cast(E)) { + unsigned LHSWidth = getMaxCalculationWidth(C, Bop->getLHS()); + unsigned RHSWidth = getMaxCalculationWidth(C, 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, C)) { + return Val.getActiveBits(); + } + } + if (Bop->getOpcode() == BO_Shl) { + llvm::APSInt Bits; + if (Bop->getRHS()->EvaluateAsInt(Bits, C)) + // 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(); + else + // Unknown bitcount, assume there is truncation. + return 1024U; + } + } + + else if (const auto *Uop = dyn_cast(E)) { + QualType T = Uop->getType(); + return T->isIntegerType() ? C.getIntWidth(T) : 1024U; + } + + else if (const auto *I = dyn_cast(E)) { + return I->getValue().getActiveBits(); + } + + return C.getIntWidth(E->getType()); +} + +void LongCastCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Cast = Result.Nodes.getNodeAs("cast"); + if (!Cast) + return; + + const Expr *SubExpr = Cast->getSubExpr()->IgnoreParenImpCasts(); + + if (Cast->getLocStart().isMacroID() || SubExpr->getLocStart().isMacroID()) + return; + + QualType CastType = Cast->getType(); + QualType SubType = SubExpr->getType(); + + if (!CastType->isIntegerType() || !SubType->isIntegerType()) + return; + + ASTContext &C = *Result.Context; + + if (C.getIntWidth(CastType) <= C.getIntWidth(SubType)) + return; + + if (C.getIntWidth(SubType) >= getMaxCalculationWidth(C, SubExpr)) + return; + + diag(Cast->getLocStart(), "either cast from %0 to %1 is ineffective, or " + "there is loss of precision before the conversion") + << SubType << CastType; +} + +} // namespace tidy +} // namespace clang Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -17,6 +17,7 @@ #include "DefinitionsInHeadersCheck.h" #include "InaccurateEraseCheck.h" #include "InefficientAlgorithmCheck.h" +#include "LongCastCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" #include "MoveConstantArgumentCheck.h" @@ -56,6 +57,8 @@ "misc-inaccurate-erase"); CheckFactories.registerCheck( "misc-inefficient-algorithm"); + CheckFactories.registerCheck( + "misc-long-cast"); CheckFactories.registerCheck( "misc-macro-parentheses"); CheckFactories.registerCheck( Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -49,6 +49,7 @@ misc-definitions-in-headers misc-inaccurate-erase misc-inefficient-algorithm + misc-long-cast misc-macro-parentheses misc-macro-repeated-side-effects misc-move-constructor-init Index: docs/clang-tidy/checks/misc-long-cast.rst =================================================================== --- docs/clang-tidy/checks/misc-long-cast.rst +++ docs/clang-tidy/checks/misc-long-cast.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - misc-long-cast + +misc-long-cast +============== + +This checker 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; + } Index: test/clang-tidy/misc-long-cast.cpp =================================================================== --- test/clang-tidy/misc-long-cast.cpp +++ test/clang-tidy/misc-long-cast.cpp @@ -0,0 +1,48 @@ +// RUN: %check_clang_tidy %s misc-long-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 + l = (long)a * b; + + l = (long)(a << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = (long)b << 8; +} + +void init(unsigned int n) { + long l = (long)(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' +} + +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)); +} + +// 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); +}