Index: clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tidy/bugprone/BugproneTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "IntegerDivisionCheck.h" #include "SuspiciousMemsetUsageCheck.h" #include "UndefinedMemoryManipulationCheck.h" @@ -20,6 +21,8 @@ class BugproneModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "bugprone-integer-division"); CheckFactories.registerCheck( "bugprone-suspicious-memset-usage"); CheckFactories.registerCheck( Index: clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tidy/bugprone/CMakeLists.txt +++ clang-tidy/bugprone/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyBugproneModule BugproneTidyModule.cpp + IntegerDivisionCheck.cpp SuspiciousMemsetUsageCheck.cpp UndefinedMemoryManipulationCheck.cpp Index: clang-tidy/bugprone/IntegerDivisionCheck.h =================================================================== --- /dev/null +++ clang-tidy/bugprone/IntegerDivisionCheck.h @@ -0,0 +1,36 @@ +//===--- IntegerDivisionCheck.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_BUGPRONE_INTEGER_DIVISION_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INTEGER_DIVISION_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Finds cases of integer divisions that seem to alter the meaning of the +/// surrounding code. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-integer-division.html +class IntegerDivisionCheck : public ClangTidyCheck { +public: + IntegerDivisionCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INTEGER_DIVISION_H Index: clang-tidy/bugprone/IntegerDivisionCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/bugprone/IntegerDivisionCheck.cpp @@ -0,0 +1,56 @@ +//===--- IntegerDivisionCheck.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 "IntegerDivisionCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +void IntegerDivisionCheck::registerMatchers(MatchFinder *Finder) { + const auto IntType = hasType(isInteger()); + + const auto BinaryOperators = binaryOperator(anyOf( + hasOperatorName("%"), hasOperatorName("<<"), hasOperatorName(">>"), + hasOperatorName("<<"), hasOperatorName("^"), hasOperatorName("|"), + hasOperatorName("&"), hasOperatorName("||"), hasOperatorName("&&"), + hasOperatorName("<"), hasOperatorName(">"), hasOperatorName("<="), + hasOperatorName(">="), hasOperatorName("=="), hasOperatorName("!="))); + + const auto UnaryOperators = + unaryOperator(anyOf(hasOperatorName("~"), hasOperatorName("!"))); + + const auto Exceptions = + anyOf(BinaryOperators, conditionalOperator(), binaryConditionalOperator(), + callExpr(IntType), explicitCastExpr(IntType), UnaryOperators); + + Finder->addMatcher( + binaryOperator( + hasOperatorName("/"), hasLHS(expr(IntType)), hasRHS(expr(IntType)), + hasAncestor( + castExpr(hasCastKind(CK_IntegralToFloating)).bind("FloatCast")), + unless(hasAncestor( + expr(Exceptions, + hasAncestor(castExpr(equalsBoundNode("FloatCast"))))))) + .bind("IntDiv"), + this); +} + +void IntegerDivisionCheck::check(const MatchFinder::MatchResult &Result) { + const auto *IntDiv = Result.Nodes.getNodeAs("IntDiv"); + diag(IntDiv->getLocStart(), "integer division; possible precision loss"); +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,7 +57,11 @@ Improvements to clang-tidy -------------------------- -The improvements are... +- New `bugprone-integer-division + `_ check + + Finds cases of integer divisions that seem to alter the meaning of the + surrounding code. Improvements to include-fixer ----------------------------- Index: docs/clang-tidy/checks/bugprone-integer-division.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/bugprone-integer-division.rst @@ -0,0 +1,41 @@ +.. title:: clang-tidy - bugprone-integer-division + +bugprone-integer-division +========================= + +Finds cases of integer divisions that seem to alter the meaning of the +surrounding code. This is assumed to happen in environments expecting +floating-point values. + +Divisions are not reported if they are part of the following expressions: + +- operands of operators expecting integral or bool types, +- call expressions of integral or bool types, and +- explicit cast expressions to integral or bool types, + +as these are interpreted as signs of deliberateness from the programmer. + +Examples: + +.. code-block:: c++ + + float floatFunc(float); + int intFunc(int); + double d; + int i = 42; + + // Warn, floating-point values expected. + d = 32 * 8 / (2 + i); + d = 8 * floatFunc(1 + 7 / 2); + d = i / (1 << 4); + + // OK, no integer division. + d = 32 * 8.0 / (2 + i); + d = 8 * floatFunc(1 + 7.0 / 2); + d = (double)i / (1 << 4); + + // OK, there are signs of deliberateness. + d = 1 << (i / 2); + d = 9 + intFunc(6 * i / 32); + d = (int)(i / 32) - 8; + Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -9,6 +9,7 @@ android-cloexec-open android-cloexec-socket boost-use-to-string + bugprone-integer-division bugprone-suspicious-memset-usage bugprone-undefined-memory-manipulation cert-dcl03-c (redirects to misc-static-assert) Index: test/clang-tidy/bugprone-integer-division.cpp =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-integer-division.cpp @@ -0,0 +1,150 @@ +// RUN: %check_clang_tidy %s bugprone-integer-division %t + +void floatArg(float x) {} +void doubleArg(double x) {} +void longDoubleArg(long double x) {} + +float floatReturn(unsigned x, int y, bool z) { + return (x * y) / z; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: integer division; possible precision loss [bugprone-integer-division] +} + +double doubleReturn(int x, char y) { + return x / y - 1; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: integer division; possible precision loss [bugprone-integer-division] +} + +long double longDoubleReturn(char x, unsigned y) { + return (1 + x / y) + 3; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: integer division; possible precision loss [bugprone-integer-division] +} + +struct X { + int n; + void m() { + floatArg(n / 3); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: integer division; possible precision loss [bugprone-integer-division] + } +}; + +struct Y { + void f(){ + auto l = [] { floatArg(2 / 3); }; + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: integer division; possible precision loss [bugprone-integer-division] + } +}; + +template +void arbitraryArg(T x) { + longDoubleArg(x); +} + +void floatEnvironment() { + char a = 2; + int b = -5; + unsigned c = 9784; + enum third { x, y, z=2 }; + third d = z; + char e[] = {'a', 'b', 'c'}; + char f = *(e + 1 / a); + bool g = 1; + + // Implicit cast to float: function argument. + floatArg(a / g - 1); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division] + doubleArg(2 + b / f); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: integer division; possible precision loss [bugprone-integer-division] + longDoubleArg(c + (e[0] / d)); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: integer division; possible precision loss [bugprone-integer-division] + + // Implicit cast to float: function return value. + long double q; + q = floatReturn(c, b, g); + q = doubleReturn(d, a); + q = longDoubleReturn(f, c); + + // Explicit casts. + q = (float)(a / b + 2); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: integer division; possible precision loss [bugprone-integer-division] + q = double((1 - c) / d); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: integer division; possible precision loss [bugprone-integer-division] + q = static_cast(2 + e[1] / g); + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: integer division; possible precision loss [bugprone-integer-division] + +#define THIRD float(1 / 3); + THIRD + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: integer division; possible precision loss [bugprone-integer-division] + + arbitraryArg(a / b); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: integer division; possible precision loss [bugprone-integer-division] +} + +int intArg(int x) { return x; } +char charArg(char x) { return x; } +bool boolArg(bool x) { return x; } + +int intReturn(int x, int y) { + return (x - 5) / y + 1; +} + +char charReturn(int x, unsigned y) { + return 1 - x / (y * 4); +} + +bool boolReturn(int x, char y) { + return (x / y + 1) * 5; +} + +void integerCastInFloatEnvironment() { + unsigned k = 87; + int l = -7; + char m = 'q'; + bool n = 0; + + // We can assume that the following cases are intended. + // Function calls expecting integers: + floatArg(0.3 + intArg(6 * k / m)); + doubleArg(charArg(n / l) * 9); + longDoubleArg(3 - boolArg(m / (k - 2))); + + // Function calls returning integers: + double o; + o = intReturn(-2, 99); + o = charReturn(1, 7); + o = boolReturn(42, 'c'); + + // Explicit casts: + floatArg(0.3 + int(6 * k / m)); + doubleArg((int)(n / l) * 9); + longDoubleArg(3 - static_cast(m / (k - 2))); + + // Operators expecting integral types: + o = 1 << (2 / m); + o = 1 << intArg(4 + k / 64); + o = ~(k / 8 + 3); + o = (32 - k / 8) ^ 1; + o = ((k / 8 + 1) * 32) | 1; + o = (1 & (k / 8)) - 2; + o = ((k - 8) / 32) % m; + + // Relational, logical, and conditional operators: + o = k / m <= 0; + o = (k * l - 5) / m != n; + o = !(l / m * 8 - 1); + o = n / m || l == -7; + o = n / m ? 1 : 0; + o = n / m ?: 1; + + // Precision loss can still be unintended + // if the cast happens before the division. + floatArg(m / (1 << 3)); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division] + floatArg(k + 1 / (m != 3)); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: integer division; possible precision loss [bugprone-integer-division] + floatArg(0.3 + int(k) / m); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: integer division; possible precision loss [bugprone-integer-division] + doubleArg((int)n / l * 9); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: integer division; possible precision loss [bugprone-integer-division] + longDoubleArg(3 - static_cast(m) / (k - 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: integer division; possible precision loss [bugprone-integer-division] +}