danielmarjamaki created this revision.
danielmarjamaki added subscribers: cfe-commits, alexfh.
This is a new checker for clang-tidy.
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.
No warning is written when it is seen that the cast is ineffective.
I tested this on debian projects.. and I see quite little noise. in 1212
projects I got 76 warnings. The cast is either ineffective or there is loss of
precision in all these cases. I don't see warnings where I think the warning is
wrong. But I don't want to warn when it can be determined that the cast is just
ineffective, I think that the perfect checker would be better at determining
this.
http://reviews.llvm.org/D16310
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/LongCastCheck.cpp
clang-tidy/misc/LongCastCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-long-cast.rst
test/clang-tidy/misc-long-cast.cpp
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);
+}
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: 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: 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<InefficientAlgorithmCheck>(
"misc-inefficient-algorithm");
+ CheckFactories.registerCheck<LongCastCheck>(
+ "misc-long-cast");
CheckFactories.registerCheck<MacroParenthesesCheck>(
"misc-macro-parentheses");
CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
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<BinaryOperator>(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<UnaryOperator>(E)) {
+ QualType T = Uop->getType();
+ return T->isIntegerType() ? C.getIntWidth(T) : 1024U;
+ }
+
+ else if (const auto *I = dyn_cast<IntegerLiteral>(E)) {
+ return I->getValue().getActiveBits();
+ }
+
+ return C.getIntWidth(E->getType());
+}
+
+void LongCastCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Cast = Result.Nodes.getNodeAs<CStyleCastExpr>("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/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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
