llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: mitchell (zeyi2) <details> <summary>Changes</summary> This PR extends the AST matchers to handle the overloaded versions of `std::accumulate`, `std::reduce` and `std::inner_product`. Closes https://github.com/llvm/llvm-project/issues/85757 --- Full diff: https://github.com/llvm/llvm-project/pull/183300.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp (+32-18) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp (+134) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp index 96e5d5d06ad70..35696dadb71f7 100644 --- a/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp @@ -50,38 +50,52 @@ void FoldInitTypeCheck::registerMatchers(MatchFinder *Finder) { hasType(hasCanonicalType(IteratorWithValueType("Iter2ValueType")))); const auto InitParam = parmVarDecl(hasType(BuiltinTypeWithId("InitType"))); + // Transparent standard functors that preserve arithmetic conversion semantics + const auto TransparentFunctor = expr(hasType( + hasCanonicalType(recordType(hasDeclaration(cxxRecordDecl(hasAnyName( + "::std::plus", "::std::minus", "::std::multiplies", "::std::divides", + "::std::bit_and", "::std::bit_or", "::std::bit_xor"))))))); + // std::accumulate, std::reduce. Finder->addMatcher( - callExpr(callee(functionDecl( - hasAnyName("::std::accumulate", "::std::reduce"), - hasParameter(0, IteratorParam), hasParameter(2, InitParam))), - argumentCountIs(3)) + callExpr( + callee(functionDecl(hasAnyName("::std::accumulate", "::std::reduce"), + hasParameter(0, IteratorParam), + hasParameter(2, InitParam))), + anyOf(argumentCountIs(3), + allOf(argumentCountIs(4), hasArgument(3, TransparentFunctor)))) .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)) + callExpr( + callee(functionDecl( + hasName("::std::inner_product"), hasParameter(0, IteratorParam), + hasParameter(2, Iterator2Param), hasParameter(3, InitParam))), + anyOf(argumentCountIs(4), + allOf(argumentCountIs(6), hasArgument(4, TransparentFunctor), + hasArgument(5, TransparentFunctor)))) .bind("Call"), this); // std::reduce with a policy. Finder->addMatcher( - callExpr(callee(functionDecl(hasName("::std::reduce"), - hasParameter(1, IteratorParam), - hasParameter(3, InitParam))), - argumentCountIs(4)) + callExpr( + callee(functionDecl(hasName("::std::reduce"), + hasParameter(1, IteratorParam), + hasParameter(3, InitParam))), + anyOf(argumentCountIs(4), + allOf(argumentCountIs(5), hasArgument(4, TransparentFunctor)))) .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)) + callExpr( + callee(functionDecl( + hasName("::std::inner_product"), hasParameter(1, IteratorParam), + hasParameter(3, Iterator2Param), hasParameter(4, InitParam))), + anyOf(argumentCountIs(5), + allOf(argumentCountIs(7), hasArgument(5, TransparentFunctor), + hasArgument(6, TransparentFunctor)))) .bind("Call"), this); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index cf8dd0dba9f12..5d61bef5a7b25 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -165,6 +165,11 @@ Changes in existing checks for unannotated functions, enabling reporting when no explicit ``throw`` is seen and allowing separate tuning for known and unknown implementations. +- Improved :doc:`bugprone-fold-init-type + <clang-tidy/checks/bugprone/fold-init-type>` check by detecting precision + loss in overloads with transparent standard functors (e.g. ``std::plus<>``) + for ``std::accumulate``, ``std::reduce``, and ``std::inner_product``. + - Improved :doc:`bugprone-macro-parentheses <clang-tidy/checks/bugprone/macro-parentheses>` check by printing the macro definition in the warning message if the macro is defined on command line. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp index c813213c3dd0f..cc0d388b13665 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp @@ -10,12 +10,22 @@ T accumulate(InputIt first, InputIt last, T init) { (void)*first; return init; } +template <class InputIt, class T, class BinaryOp> +T accumulate(InputIt first, InputIt last, T init, BinaryOp op) { + (void)*first; + return init; +} template <class InputIt, class T> T reduce(InputIt first, InputIt last, T init) { (void)*first; return init; } +template <class InputIt, class T, class BinaryOp> +T reduce(InputIt first, InputIt last, T init, BinaryOp op) { (void)*first; return init; } template <class ExecutionPolicy, class InputIt, class T> T reduce(ExecutionPolicy &&policy, InputIt first, InputIt last, T init) { (void)*first; return init; } +template <class ExecutionPolicy, class InputIt, class T, class BinaryOp> +T reduce(ExecutionPolicy &&policy, + InputIt first, InputIt last, T init, BinaryOp op) { (void)*first; return init; } struct parallel_execution_policy {}; constexpr parallel_execution_policy par{}; @@ -23,10 +33,48 @@ constexpr parallel_execution_policy par{}; template <class InputIt1, class InputIt2, class T> T inner_product(InputIt1 first1, InputIt1 last1, InputIt2 first2, T value) { (void)*first1; (void)*first2; return value; } +template <class InputIt1, class InputIt2, class T, class BinaryOp1, class BinaryOp2> +T inner_product(InputIt1 first1, InputIt1 last1, + InputIt2 first2, T value, BinaryOp1 op1, BinaryOp2 op2) { (void)*first1; (void)*first2; return value; } template <class ExecutionPolicy, class InputIt1, class InputIt2, class T> T inner_product(ExecutionPolicy &&policy, InputIt1 first1, InputIt1 last1, InputIt2 first2, T value) { (void)*first1; (void)*first2; return value; } +template <class ExecutionPolicy, class InputIt1, class InputIt2, class T, class BinaryOp1, class BinaryOp2> +T inner_product(ExecutionPolicy &&policy, InputIt1 first1, InputIt1 last1, + InputIt2 first2, T value, BinaryOp1 op1, BinaryOp2 op2) { (void)*first1; (void)*first2; return value; } + +template <class T = void> struct plus { + T operator()(T a, T b) const { return a + b; } +}; +template <> struct plus<void> { + template <class T, class U> + auto operator()(T a, U b) const { return a + b; } +}; + +template <class T = void> struct minus { + T operator()(T a, T b) const { return a - b; } +}; +template <> struct minus<void> { + template <class T, class U> + auto operator()(T a, U b) const { return a - b; } +}; + +template <class T = void> struct multiplies { + T operator()(T a, T b) const { return a * b; } +}; +template <> struct multiplies<void> { + template <class T, class U> + auto operator()(T a, U b) const { return a * b; } +}; + +template <class T = void> struct bit_or { + T operator()(T a, T b) const { return a | b; } +}; +template <> struct bit_or<void> { + template <class T, class U> + auto operator()(T a, U b) const { return a | b; } +}; } // namespace std @@ -159,6 +207,56 @@ int innerProductPositive2() { // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' } +int accumulatePositiveOp1() { + float a[1] = {0.5f}; + return std::accumulate(a, a + 1, 0, std::plus<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int accumulatePositiveOp2() { + float a[1] = {0.5f}; + return std::accumulate(a, a + 1, 0, std::minus<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int accumulatePositiveOp3() { + float a[1] = {0.5f}; + return std::accumulate(a, a + 1, 0, std::multiplies<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int reducePositiveOp1() { + float a[1] = {0.5f}; + return std::reduce(a, a + 1, 0, std::plus<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int reducePositiveOp2() { + float a[1] = {0.5f}; + return std::reduce(std::par, a, a + 1, 0, std::plus<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int innerProductPositiveOp1() { + float a[1] = {0.5f}; + int b[1] = {1}; + return std::inner_product(a, a + 1, b, 0, std::plus<>(), std::multiplies<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int innerProductPositiveOp2() { + float a[1] = {0.5f}; + int b[1] = {1}; + return std::inner_product(std::par, a, a + 1, b, 0, std::plus<>(), std::multiplies<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int accumulatePositiveBitOr() { + unsigned a[1] = {1}; + return std::accumulate(a, a + 1, 0, std::bit_or<>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'unsigned int' into type 'int' +} + // Negatives. int negative1() { @@ -191,6 +289,42 @@ int negative5() { return std::inner_product(std::par, a, a + 1, b, 0.0f); } +int negativeOp1() { + float a[1] = {0.5f}; + // This is OK because types match. + return std::accumulate(a, a + 1, 0.0f, std::plus<>()); +} + +int negativeOp2() { + float a[1] = {0.5f}; + // This is OK because double is bigger than float. + return std::reduce(a, a + 1, 0.0, std::plus<>()); +} + +int negativeLambda1() { + float a[1] = {0.5f}; + // This is currently a known limitation. + return std::accumulate(a, a + 1, 0, [](int acc, float val) { + return acc + (val > 0.0f ? 1 : 0); + }); +} + +int negativeLambda2() { + float a[1] = {0.5f}; + // This is currently a known limitation. + return std::reduce(a, a + 1, 0, [](int acc, float val) { + return acc + static_cast<int>(val); + }); +} + +int negativeInnerProductMixedOps() { + float a[1] = {0.5f}; + int b[1] = {1}; + // Only one op is transparent, the other is a lambda. + return std::inner_product(a, a + 1, b, 0, std::plus<>(), + [](float x, int y) { return x * y; }); +} + namespace blah { namespace std { template <class InputIt, class T> `````````` </details> https://github.com/llvm/llvm-project/pull/183300 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
