[PATCH] D35932: [clang-tidy] Add integer division check
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310589: [clang-tidy] Add integer division check (authored by
xazax).
Changed prior to commit:
https://reviews.llvm.org/D35932?vs=110539&id=110570#toc
Repository:
rL LLVM
https://reviews.llvm.org/D35932
Files:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/bugprone/IntegerDivisionCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/IntegerDivisionCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-integer-division.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-integer-division.cpp
Index: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyBugproneModule
BugproneTidyModule.cpp
+ IntegerDivisionCheck.cpp
SuspiciousMemsetUsageCheck.cpp
UndefinedMemoryManipulationCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/bugprone/IntegerDivisionCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/IntegerDivisionCheck.h
+++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-tidy/bugprone/IntegerDivisionCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/IntegerDivisionCheck.cpp
+++ clang-tools-extra/trunk/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("&"), has
[PATCH] D35932: [clang-tidy] Add integer division check
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D35932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35932: [clang-tidy] Add integer division check
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
+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(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: result of integer division used in
+ intDivSin(c / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: result of integer division used in
+ intDivSin(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/clan
[PATCH] D35932: [clang-tidy] Add integer division check
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
A few more nits.
Comment at: clang-tidy/bugprone/IntegerDivisionCheck.cpp:40-44
+ hasAncestor(
+ castExpr(hasCastKind(CK_IntegralToFloating)).bind("FloatCast")),
+ unless(hasAncestor(
+ expr(Exceptions,
+ hasAncestor(castExpr(equalsBoundNode("FloatCast")))
The way `hasAncestor` is used in this code makes me uneasy. Each nested
ancestor traversal can potentially slow down matching by a factor of the depth
of the AST tree, which may lead to poor performance on certain types of code
(nested descendant traversal is much worse though). Some of the overhead may be
compensated by memoization, but it's hard to predict where it will actually
work.
It's usually better to avoid nested ancestor traversals, if there are good
alternatives. Here I don't see a better possibility with matchers, but it's to
go one level lower and use RecursiveASTVisitor directly. Without constructing /
discovering a problematic case it's hard to tell whether the performance win of
switching to RAV is worth the added complexity, so I'm not suggesting to do
that yet. Just mentioning a possible issue to be aware of.
Comment at: clang-tidy/bugprone/IntegerDivisionCheck.cpp:51
+ const auto *IntDiv = Result.Nodes.getNodeAs("IntDiv");
+ diag(IntDiv->getLocStart(), "integer division; possible precision loss");
+}
Maybe expand the message to describe the pattern the check matches more
precisely? E.g. "result of an integer division is used in a floating point
context; possible loss of precision"?
Comment at: docs/ReleaseNotes.rst:63-64
+
+ Finds cases of integer divisions that seem to alter the meaning of the
+ surrounding code.
The description is somewhat vague. How about "Finds cases where integer
division in floating point context is likely to cause unintended loss of
precision."? Same in the docs below.
Comment at: test/clang-tidy/bugprone-integer-division.cpp:14
+ return x / y - 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: integer division; possible
precision loss [bugprone-integer-division]
+}
It's better to truncate repeated fragments of the messages, especially when
they exceed 80 characters (better on a word boundary ;).
https://reviews.llvm.org/D35932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35932: [clang-tidy] Add integer division check
rnkovacs updated this revision to Diff 109497.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
Uploaded a more thought-out version of the check with more cases covered and
hopefully clearer docs. It produces no hits on LLVM&Clang.
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,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
[PATCH] D35932: [clang-tidy] Add integer division check
rnkovacs added a comment. I run the check on LLVM-Clang, and got this one hit: /home/reka/codechecker_dev_env/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1000:43: warning: integer division; possible precision loss [bugprone-integer-division] SDValue TWOHW = DAG.getConstantFP(1 << (BW / 2), DL, Op.getValueType()); ^ It warns because `getConstantFP()` expects a floating-point value as its first argument. Still seems to be a false positive. I might want to add an exception for bitshift operators. Comment at: docs/clang-tidy/checks/bugprone-integer-division.rst:21-24 + // Do not warn, there are signs of deliberateness. + sin(abs(3) / 5); + sin(1 + abs(1 + 7 / 2)); + 1 << 2 / 3; alexfh wrote: > I'd argue that the presence of `abs` here doesn't add any signal, since the > function has overloads both for integer and floating point types. Well, I meant specifically the above declared function, but you're right, the naming was misleading, so I changed that. To be honest, this check doesn't work for most of the std math functions involving template magic. Custom user-defined functions could be mainly targeted. https://reviews.llvm.org/D35932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35932: [clang-tidy] Add integer division check
rnkovacs updated this revision to Diff 108605.
rnkovacs edited the summary of this revision.
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: integer division; possible precision loss [bugprone-integer-division]
+}
+
+double wrongOrder(int x, int y) {
+ return x/y/0.1;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: integer division; possible precision loss [bugprone-integer-division]
+}
+
+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
+void intDivSin(T x) {
+ sin(x);
+}
+
+int intFunc(int);
+
+struct X {
+ int n;
+ void m() {
+sin(n / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: integer division; possible precision loss [bugprone-integer-division]
+ }
+};
+
+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: integer division; possible precision loss [bugprone-integer-division]
+ sin(c / (1 + .5));
+ sin((c + .5) / 3);
+
+ sin(intFunc(3) / 5);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: integer division; possible precision loss [bugprone-integer-division]
+ acos(2 / intFunc(7));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ floatArg(1 + 2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: integer division; possible precision loss [bugprone-integer-division]
+ sin(1 + 2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: integer division; possible precision loss [bugprone-integer-division]
+ intFunc(sin(1 + 2 / 3));
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ 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: integer division; possible precision loss [bugprone-integer-division]
+
+ intDivSin(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: integer division; possible precision loss [bugprone-integer-division]
+ intDivSin(c / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: integer division; possible precision loss [bugprone-integer-division]
+ intDivSin(f / g);
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ floatArg(1 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division]
+ doubleArg(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: integer division; possible precision loss [bugprone-integer-division]
+ longDoubleArg(3 / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: integer division; possible precision loss [bugprone-integer-division]
+ floatArg(a / b / 0.1);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division]
+ doubleArg(1 / 3 / 0.1);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: integer division; possible precision loss [bugprone-integer-division]
+ longDoubleArg(2 / 3 / 5);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ std::sin(2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division]
+ ::acos(7 / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: integer division; possible precision loss [bugprone-integer-division]
+ tanh(f / g);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ 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: integer division; possible precision los
[PATCH] D35932: [clang-tidy] Add integer division check
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Could you run the check over LLVM+Clang code and post a summary of results here? https://reviews.llvm.org/D35932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35932: [clang-tidy] Add integer division check
alexfh added inline comments. Comment at: docs/clang-tidy/checks/bugprone-integer-division.rst:21-24 + // Do not warn, there are signs of deliberateness. + sin(abs(3) / 5); + sin(1 + abs(1 + 7 / 2)); + 1 << 2 / 3; I'd argue that the presence of `abs` here doesn't add any signal, since the function has overloads both for integer and floating point types. https://reviews.llvm.org/D35932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35932: [clang-tidy] Add integer division check
rnkovacs created this revision.
rnkovacs added a project: clang-tools-extra.
Herald added subscribers: baloghadamsoftware, JDevlieghere, mgorny.
Finds integer divisions in environments expecting floating-point values.
Examples of possibly unintended precision loss:
sin(7 / (2 + 3));
abs(sin(1 + 7 / 2));
The following expressions are presumably deliberate:
sin(abs(1 + 7 / 2));
1 << 2 / 3;
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: integer division; possible precision loss [bugprone-integer-division]
+}
+
+double wrongOrder(int x, int y) {
+ return x/y/0.1;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: integer division; possible precision loss [bugprone-integer-division]
+}
+
+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
+void intDivSin(T x) {
+ sin(x);
+}
+
+int intFunc(int);
+
+struct X {
+ int n;
+ void m() {
+sin(n / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: integer division; possible precision loss [bugprone-integer-division]
+ }
+};
+
+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(c / (2 + 2));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: integer division; possible precision loss [bugprone-integer-division]
+ sin(c / (1 + .5));
+ sin((c + .5) / 3);
+
+ sin(intFunc(3) / 5);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: integer division; possible precision loss [bugprone-integer-division]
+ acos(2 / intFunc(7));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ floatArg(1 + 2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: integer division; possible precision loss [bugprone-integer-division]
+ sin(1 + 2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: integer division; possible precision loss [bugprone-integer-division]
+ intFunc(sin(1 + 2 / 3));
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ 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: integer division; possible precision loss [bugprone-integer-division]
+
+ intDivSin(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: integer division; possible precision loss [bugprone-integer-division]
+ intDivSin(c / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: integer division; possible precision loss [bugprone-integer-division]
+ intDivSin(f / g);
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ floatArg(1 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division]
+ doubleArg(a / b);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: integer division; possible precision loss [bugprone-integer-division]
+ longDoubleArg(3 / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: integer division; possible precision loss [bugprone-integer-division]
+ floatArg(a / b / 0.1);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division]
+ doubleArg(1 / 3 / 0.1);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: integer division; possible precision loss [bugprone-integer-division]
+ longDoubleArg(2 / 3 / 5);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: integer division; possible precision loss [bugprone-integer-division]
+
+ std::sin(2 / 3);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: integer division; possible precision loss [bugprone-integer-division]
+ ::acos(7 / d);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: integer division; possible precision loss [bugprone-integer-division]
+ tanh(f / g);
+// CHECK-MESSAGES: :[[
