rnkovacs updated this revision to Diff 110539.
rnkovacs marked 3 inline comments as done.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
Thanks for the comments. I improved the docs and truncated the messages in the
test file.
We also had concerns about the nested `hasAncestor` matchers but thought that
it might be worth a try. If this solution proves to cause too much of a
performance burden I can rewrite it using RAVs.
https://reviews.llvm.org/D35932
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/IntegerDivisionCheck.cpp
clang-tidy/bugprone/IntegerDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-integer-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-integer-division.cpp
Index: test/clang-tidy/bugprone-integer-division.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-integer-division.cpp
@@ -0,0 +1,130 @@
+// RUN: %check_clang_tidy %s bugprone-integer-division %t
+
+// Functions expecting a floating-point parameter.
+void floatArg(float x) {}
+void doubleArg(double x) {}
+void longDoubleArg(long double x) {}
+
+// Functions expected to return a floating-point value.
+float singleDiv() {
+ int x = -5;
+ int y = 2;
+ return x/y;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: result of integer division used in
+}
+
+double wrongOrder(int x, int y) {
+ return x/y/0.1;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: result of integer division used in
+}
+
+long double rightOrder(int x, int y) {
+ return 0.1/x/y; // OK
+}
+
+// Typical mathematical functions.
+float sin(float);
+double acos(double);
+long double tanh(long double);
+
+namespace std {
+ using ::sin;
+}
+
+template <typename T>
+void intDivSin(T x) {
+ sin(x);
+}
+
+int intFunc(int);
+
+struct X {
+ int n;
+ void m() {
+ sin(n / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: result of integer division used in
+ }
+};
+
+void integerDivision() {
+ char a = 2;
+ short b = -5;
+ int c = 9784;
+ enum third { x, y, z=2 };
+ third d = z;
+ char e[] = {'a', 'b', 'c'};
+ char f = *(e + 1 / a);
+ bool g = 1;
+
+ sin(1 + c / (2 + 2));
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of integer division used in
+ sin(c / (1 + .5));
+ sin((c + .5) / 3);
+
+ sin(intFunc(3) / 5);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of integer division used in
+ acos(2 / intFunc(7));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: result of integer division used in
+
+ floatArg(1 + 2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: result of integer division used in
+ sin(1 + 2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of integer division used in
+ intFunc(sin(1 + 2 / 3));
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: result of integer division used in
+
+ floatArg(1 + intFunc(1 + 2 / 3));
+ floatArg(1 + 3 * intFunc(a / b));
+
+ 1 << (2 / 3);
+ 1 << intFunc(2 / 3);
+
+#define M_SIN sin(a / b);
+ M_SIN
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result of integer division used in
+
+ intDivSin<float>(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: result of integer division used in
+ intDivSin<double>(c / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: result of integer division used in
+ intDivSin<long double>(f / g);
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: result of integer division used in
+
+ floatArg(1 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of integer division used in
+ doubleArg(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of integer division used in
+ longDoubleArg(3 / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: result of integer division used in
+ floatArg(a / b / 0.1);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of integer division used in
+ doubleArg(1 / 3 / 0.1);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of integer division used in
+ longDoubleArg(2 / 3 / 5);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: result of integer division used in
+
+ std::sin(2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of integer division used in
+ ::acos(7 / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: result of integer division used in
+ tanh(f / g);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: result of integer division used in
+
+ floatArg(0.1 / a / b);
+ doubleArg(0.1 / 3 / 1);
+
+ singleDiv();
+ wrongOrder(a,b);
+ rightOrder(a,b);
+
+ sin(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of integer division used in
+ acos(f / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: result of integer division used in
+ tanh(c / g);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: result of integer division used in
+
+ sin(3.0 / a);
+ acos(b / 3.14);
+ tanh(3.14 / f / g);
+}
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) <cert-dcl03-c>
Index: docs/clang-tidy/checks/bugprone-integer-division.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-integer-division.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - bugprone-integer-division
+
+bugprone-integer-division
+=========================
+
+Finds cases where integer division in a floating point context is likely to
+cause unintended loss of precision.
+
+No reports are made if divisions 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/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -70,6 +70,11 @@
``AllowConditionalIntegerCasts`` -> ``AllowIntegerConditions``,
``AllowConditionalPointerCasts`` -> ``AllowPointerConditions``.
+- New `bugprone-integer-division
+ <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-integer-division.html>`_ check
+
+ Finds cases where integer division in a floating point context is likely to
+ cause unintended loss of precision.
- New `readability-static-accessed-through-instance
<http://clang.llvm.org/extra/clang-tidy/checks/readability-static-accessed-through-instance.html>`_ check
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 where integer division in a floating point context is likely to
+/// cause unintended loss of precision.
+///
+/// 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,57 @@
+//===--- 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<BinaryOperator>("IntDiv");
+ diag(IntDiv->getLocStart(), "result of integer division used in a floating "
+ "point context; possible loss of precision");
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
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/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<IntegerDivisionCheck>(
+ "bugprone-integer-division");
CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
"bugprone-suspicious-memset-usage");
CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
