Author: mitchell Date: 2026-02-27T01:29:53+08:00 New Revision: af15474262100ade9a8fcfd05f9e05c7ba23ff8c
URL: https://github.com/llvm/llvm-project/commit/af15474262100ade9a8fcfd05f9e05c7ba23ff8c DIFF: https://github.com/llvm/llvm-project/commit/af15474262100ade9a8fcfd05f9e05c7ba23ff8c.diff LOG: [clang-tidy] Improve bugprone-fold-init-type to support overloads (#183300) 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 --------- Co-authored-by: Victor Chernyakin <[email protected]> Co-authored-by: Baranov Victor <[email protected]> Added: Modified: clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/bugprone/fold-init-type.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/fold-init-type.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp index 96e5d5d06ad70..0a9b2106ad29f 100644 --- a/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/FoldInitTypeCheck.cpp @@ -50,38 +50,53 @@ 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 10b5e41dd79ed..2f3c4acda4ae4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -172,6 +172,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/docs/clang-tidy/checks/bugprone/fold-init-type.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/fold-init-type.rst index fefad9f5e535b..e8b3be35d9b66 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/fold-init-type.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/fold-init-type.rst @@ -5,10 +5,18 @@ bugprone-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: +that might result in loss of precision. + +The check supports the following functions: + +- ``std::accumulate`` +- ``std::reduce`` +- ``std::inner_product`` + +These functions fold an input range into an initial value using the type of the +latter. By default, ``std::accumulate`` and ``std::reduce`` use ``operator+`` +while ``std::inner_product`` uses ``operator+`` and ``operator*``. This can +cause loss of precision through: - Truncation: The following code uses a floating point range and an int initial value, so truncation will happen at every application of @@ -26,3 +34,13 @@ loss of precision through: auto a = {65536LL * 65536 * 65536}; return std::accumulate(std::begin(a), std::end(a), 0); + +The check handles overloads with the following transparent standard functors: + +- ``std::plus`` +- ``std::minus`` +- ``std::multiplies`` +- ``std::divides`` +- ``std::bit_and`` +- ``std::bit_or`` +- ``std::bit_xor`` 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..b343028b9a6dc 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,80 @@ 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' +} + +int accumulatePositiveExplicitFunctor1() { + float a[1] = {0.5f}; + return std::accumulate(a, a + 1, 0, std::plus<int>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int accumulatePositiveExplicitFunctor2() { + float a[1] = {0.5f}; + return std::accumulate(a, a + 1, 0, std::plus<float>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int accumulatePositiveExplicitFunctor3() { + float a[1] = {0.5f}; + return std::accumulate(a, a + 1, 0, std::multiplies<double>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'float' into type 'int' +} + +int accumulatePositiveExplicitFunctor4() { + double a[1] = {0.5}; + return std::accumulate(a, a + 1, 0.0f, std::plus<double>()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: folding type 'double' into type 'float' +} + // Negatives. int negative1() { @@ -191,6 +313,48 @@ 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 negativeOp3() { + float a[1] = {0.5f}; + // This is OK because types are compatible even with explicit functor. + return std::accumulate(a, a + 1, 0.0, std::plus<double>()); +} + +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> _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
