[clang-tools-extra] r267542 - A clang-tidy check for std:accumulate.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 26 03:05:45 PDT 2016
Author: alexfh
Date: Tue Apr 26 05:05:45 2016
New Revision: 267542
URL: http://llvm.org/viewvc/llvm-project?rev=267542&view=rev
Log:
A clang-tidy check for std:accumulate.
Summary:
For folds (e.g. std::accumulate), check matches between the provided init value and the range's value_type. A typical error is "std::accumulate(begin, end, 0);", where begin and end have float value_type. See the documentation for more examples.
For now we check std::accumulate, std::reduce and std::inner_product.
Reviewers: hokein, alexfh
Subscribers: Prazek, aaron.ballman, cfe-commits, courbet
Patch by Clément Courbet!
Differential Revision: http://reviews.llvm.org/D18442
Added:
clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-fold-init-type.rst
clang-tools-extra/trunk/test/clang-tidy/misc-fold-init-type.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=267542&r1=267541&r2=267542&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Tue Apr 26 05:05:45 2016
@@ -7,6 +7,7 @@ add_clang_library(clangTidyMiscModule
BoolPointerImplicitConversionCheck.cpp
DanglingHandleCheck.cpp
DefinitionsInHeadersCheck.cpp
+ FoldInitTypeCheck.cpp
ForwardDeclarationNamespaceCheck.cpp
InaccurateEraseCheck.cpp
IncorrectRoundings.cpp
Added: clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.cpp?rev=267542&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.cpp Tue Apr 26 05:05:45 2016
@@ -0,0 +1,140 @@
+//===--- FoldInitTypeCheck.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 "FoldInitTypeCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void FoldInitTypeCheck::registerMatchers(MatchFinder *Finder) {
+ // We match functions of interest and bind the iterator and init value types.
+ // Note: Right now we check only builtin types.
+ const auto BuiltinTypeWithId = [](const char *ID) {
+ return hasCanonicalType(builtinType().bind(ID));
+ };
+ const auto IteratorWithValueType = [&BuiltinTypeWithId](const char *ID) {
+ return anyOf(
+ // Pointer types.
+ pointsTo(BuiltinTypeWithId(ID)),
+ // Iterator types.
+ recordType(hasDeclaration(has(typedefNameDecl(
+ hasName("value_type"), hasType(BuiltinTypeWithId(ID)))))));
+ };
+
+ const auto IteratorParam = parmVarDecl(
+ hasType(hasCanonicalType(IteratorWithValueType("IterValueType"))));
+ const auto Iterator2Param = parmVarDecl(
+ hasType(hasCanonicalType(IteratorWithValueType("Iter2ValueType"))));
+ const auto InitParam = parmVarDecl(hasType(BuiltinTypeWithId("InitType")));
+
+ // std::accumulate, std::reduce.
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(
+ hasAnyName("::std::accumulate", "::std::reduce"),
+ hasParameter(0, IteratorParam), hasParameter(2, InitParam))),
+ argumentCountIs(3))
+ .bind("Call"),
+ this);
+ // std::inner_product.
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("::std::inner_product"),
+ hasParameter(0, IteratorParam),
+ hasParameter(2, Iterator2Param),
+ hasParameter(3, InitParam))),
+ argumentCountIs(4))
+ .bind("Call"),
+ this);
+ // std::reduce with a policy.
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("::std::reduce"),
+ hasParameter(1, IteratorParam),
+ hasParameter(3, InitParam))),
+ argumentCountIs(4))
+ .bind("Call"),
+ this);
+ // std::inner_product with a policy.
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("::std::inner_product"),
+ hasParameter(1, IteratorParam),
+ hasParameter(3, Iterator2Param),
+ hasParameter(4, InitParam))),
+ argumentCountIs(5))
+ .bind("Call"),
+ this);
+}
+
+/// Returns true if ValueType is allowed to fold into InitType, i.e. if:
+/// static_cast<InitType>(ValueType{some_value})
+/// does not result in trucation.
+static bool isValidBuiltinFold(const BuiltinType &ValueType,
+ const BuiltinType &InitType,
+ const ASTContext &Context) {
+ const auto ValueTypeSize = Context.getTypeSize(&ValueType);
+ const auto InitTypeSize = Context.getTypeSize(&InitType);
+ // It's OK to fold a float into a float of bigger or equal size, but not OK to
+ // fold into an int.
+ if (ValueType.isFloatingPoint())
+ return InitType.isFloatingPoint() && InitTypeSize >= ValueTypeSize;
+ // It's OK to fold an int into:
+ // - an int of the same size and signedness.
+ // - a bigger int, regardless of signedness.
+ // - FIXME: should it be a warning to fold into floating point?
+ if (ValueType.isInteger()) {
+ if (InitType.isInteger()) {
+ if (InitType.isSignedInteger() == ValueType.isSignedInteger())
+ return InitTypeSize >= ValueTypeSize;
+ return InitTypeSize > ValueTypeSize;
+ }
+ if (InitType.isFloatingPoint())
+ return InitTypeSize >= ValueTypeSize;
+ }
+ return false;
+}
+
+/// Prints a diagnostic if IterValueType doe snot fold into IterValueType (see
+// isValidBuiltinFold for details).
+void FoldInitTypeCheck::doCheck(const BuiltinType &IterValueType,
+ const BuiltinType &InitType,
+ const ASTContext &Context,
+ const CallExpr &CallNode) {
+ if (!isValidBuiltinFold(IterValueType, InitType, Context)) {
+ diag(CallNode.getExprLoc(), "folding type %0 into type %1 might result in "
+ "loss of precision")
+ << IterValueType.desugar() << InitType.desugar();
+ }
+}
+
+void FoldInitTypeCheck::check(const MatchFinder::MatchResult &Result) {
+ // Given the iterator and init value type retreived by the matchers,
+ // we check that the ::value_type of the iterator is compatible with
+ // the init value type.
+ const auto *InitType = Result.Nodes.getNodeAs<BuiltinType>("InitType");
+ const auto *IterValueType =
+ Result.Nodes.getNodeAs<BuiltinType>("IterValueType");
+ assert(InitType != nullptr);
+ assert(IterValueType != nullptr);
+
+ const auto *CallNode = Result.Nodes.getNodeAs<CallExpr>("Call");
+ assert(CallNode != nullptr);
+
+ doCheck(*IterValueType, *InitType, *Result.Context, *CallNode);
+
+ if (const auto *Iter2ValueType =
+ Result.Nodes.getNodeAs<BuiltinType>("Iter2ValueType"))
+ doCheck(*Iter2ValueType, *InitType, *Result.Context, *CallNode);
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.h?rev=267542&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/FoldInitTypeCheck.h Tue Apr 26 05:05:45 2016
@@ -0,0 +1,44 @@
+//===--- FoldInitTypeCheck.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_FOLD_INIT_TYPE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FOLD_INIT_TYPE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find and flag invalid initializer values in folds, e.g. std::accumulate.
+/// Example:
+/// \code
+/// auto v = {65536L * 65536 * 65536};
+/// std::accumulate(begin(v), end(v), 0 /* int type is too small */);
+/// \endcode
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-fold-init-type.html
+class FoldInitTypeCheck : public ClangTidyCheck {
+public:
+ FoldInitTypeCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void doCheck(const BuiltinType &IterValueType, const BuiltinType &InitType,
+ const ASTContext &Context, const CallExpr &CallNode);
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FOLD_INIT_TYPE_H
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=267542&r1=267541&r2=267542&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Tue Apr 26 05:05:45 2016
@@ -16,6 +16,7 @@
#include "BoolPointerImplicitConversionCheck.h"
#include "DanglingHandleCheck.h"
#include "DefinitionsInHeadersCheck.h"
+#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
#include "InaccurateEraseCheck.h"
#include "IncorrectRoundings.h"
@@ -67,6 +68,8 @@ public:
"misc-dangling-handle");
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
+ CheckFactories.registerCheck<FoldInitTypeCheck>(
+ "misc-fold-init-type");
CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
"misc-forward-declaration-namespace");
CheckFactories.registerCheck<InaccurateEraseCheck>(
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=267542&r1=267541&r2=267542&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Apr 26 05:05:45 2016
@@ -53,6 +53,7 @@ Clang-Tidy Checks
misc-bool-pointer-implicit-conversion
misc-dangling-handle
misc-definitions-in-headers
+ misc-fold-init-type
misc-forward-declaration-namespace
misc-inaccurate-erase
misc-incorrect-roundings
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-fold-init-type.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-fold-init-type.rst?rev=267542&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-fold-init-type.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-fold-init-type.rst Tue Apr 26 05:05:45 2016
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - misc-fold-init-type
+
+misc-fold-init-type
+===================
+
+The check flags type mismatches in
+`folds <https://en.wikipedia.org/wiki/Fold_(higher-order_function)>`_
+like `std::accumulate` that might result in loss of precision.
+`std::accumulate` folds an input range into an initial value using the type of
+the latter, with `operator+` by default. This can cause loss of precision
+through:
+
+- Truncation: The following code uses a floating point range and an int
+ initial value, so trucation wil happen at every application of `operator+`
+ and the result will be `0`, which might not be what the user expected.
+
+.. code:: c++
+
+ auto a = {0.5f, 0.5f, 0.5f, 0.5f};
+ return std::accumulate(std::begin(a), std::end(a), 0);
+
+- Overflow: The following code also returns `0`.
+
+.. code:: c++
+
+ auto a = {65536LL * 65536 * 65536};
+ return std::accumulate(std::begin(a), std::end(a), 0);
Added: clang-tools-extra/trunk/test/clang-tidy/misc-fold-init-type.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-fold-init-type.cpp?rev=267542&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-fold-init-type.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-fold-init-type.cpp Tue Apr 26 05:05:45 2016
@@ -0,0 +1,158 @@
+// RUN: %check_clang_tidy %s misc-fold-init-type %t
+
+namespace std {
+template <class InputIt, class T>
+T accumulate(InputIt first, InputIt last, T init);
+
+template <class InputIt, class T>
+T reduce(InputIt first, InputIt last, T init);
+template <class ExecutionPolicy, class InputIt, class T>
+T reduce(ExecutionPolicy &&policy,
+ InputIt first, InputIt last, T init);
+
+struct parallel_execution_policy {};
+constexpr parallel_execution_policy par{};
+
+template <class InputIt1, class InputIt2, class T>
+T inner_product(InputIt1 first1, InputIt1 last1,
+ InputIt2 first2, T value);
+
+template <class ExecutionPolicy, class InputIt1, class InputIt2, class T>
+T inner_product(ExecutionPolicy &&policy, InputIt1 first1, InputIt1 last1,
+ InputIt2 first2, T value);
+
+} // namespace std
+
+struct FloatIterator {
+ typedef float value_type;
+};
+template <typename ValueType>
+struct TypedefTemplateIterator { typedef ValueType value_type; };
+template <typename ValueType>
+struct UsingTemplateIterator { using value_type = ValueType; };
+template <typename ValueType>
+struct DependentTypedefTemplateIterator { typedef typename ValueType::value_type value_type; };
+template <typename ValueType>
+struct DependentUsingTemplateIterator : public TypedefTemplateIterator<ValueType> { using typename TypedefTemplateIterator<ValueType>::value_type; };
+using TypedeffedIterator = FloatIterator;
+
+// Positives.
+
+int accumulatePositive1() {
+ float a[1] = {0.5f};
+ return std::accumulate(a, a + 1, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
+}
+
+int accumulatePositive2() {
+ FloatIterator it;
+ return std::accumulate(it, it, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
+}
+
+int accumulatePositive3() {
+ double a[1] = {0.0};
+ return std::accumulate(a, a + 1, 0.0f);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'double' into type 'float'
+}
+
+int accumulatePositive4() {
+ TypedefTemplateIterator<unsigned> it;
+ return std::accumulate(it, it, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int'
+}
+
+int accumulatePositive5() {
+ UsingTemplateIterator<unsigned> it;
+ return std::accumulate(it, it, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int'
+}
+
+int accumulatePositive6() {
+ DependentTypedefTemplateIterator<UsingTemplateIterator<unsigned>> it;
+ return std::accumulate(it, it, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int'
+}
+
+int accumulatePositive7() {
+ TypedeffedIterator it;
+ return std::accumulate(it, it, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
+}
+
+int accumulatePositive8() {
+ DependentUsingTemplateIterator<unsigned> it;
+ return std::accumulate(it, it, 0);
+ // FIXME: this one should trigger too.
+}
+
+int reducePositive1() {
+ float a[1] = {0.5f};
+ return std::reduce(a, a + 1, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
+}
+
+int reducePositive2() {
+ float a[1] = {0.5f};
+ return std::reduce(std::par, a, a + 1, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
+}
+
+int innerProductPositive1() {
+ float a[1] = {0.5f};
+ int b[1] = {1};
+ return std::inner_product(std::par, a, a + 1, b, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
+}
+
+int innerProductPositive2() {
+ float a[1] = {0.5f};
+ int b[1] = {1};
+ return std::inner_product(std::par, a, a + 1, b, 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int'
+}
+
+// Negatives.
+
+int negative1() {
+ float a[1] = {0.5f};
+ // This is OK because types match.
+ return std::accumulate(a, a + 1, 0.0);
+}
+
+int negative2() {
+ float a[1] = {0.5f};
+ // This is OK because double is bigger than float.
+ return std::accumulate(a, a + 1, 0.0);
+}
+
+int negative3() {
+ float a[1] = {0.5f};
+ // This is OK because the user explicitly specified T.
+ return std::accumulate<float *, float>(a, a + 1, 0);
+}
+
+int negative4() {
+ TypedefTemplateIterator<unsigned> it;
+ // For now this is OK.
+ return std::accumulate(it, it, 0.0);
+}
+
+int negative5() {
+ float a[1] = {0.5f};
+ float b[1] = {1.0f};
+ return std::inner_product(std::par, a, a + 1, b, 0.0f);
+}
+
+namespace blah {
+namespace std {
+template <class InputIt, class T>
+T accumulate(InputIt, InputIt, T); // We should not care about this one.
+}
+
+int negative5() {
+ float a[1] = {0.5f};
+ // Note that this is using blah::std::accumulate.
+ return std::accumulate(a, a + 1, 0);
+}
+}
More information about the cfe-commits
mailing list