Improving fold function

Question

I have implemented a simple fold function in C++ that accepts a lambda, and can fold multiple vectors at the same time at compile time. I am wondering if it could be simplified in some manner (I have provided both a recursive version and an iteratively recursive version - I am unsure which should have better performance): https://godbolt.org/z/39pW81

Performance optimizations are also welcome - in that regard is any of the two approaches faster?

template<int I, typename type_identity, typename type_head, int N, typename ...type_tail, int ...N_tail,  typename Function>
auto foldHelperR(Function&& func, const type_identity& id, const tvecn<type_head, N>& head, const tvecn<type_tail, N_tail>&... tail)
{
    if constexpr (I>0)
    {
        return func(foldHelperR<I-1>(std::forward<Function>(func), id, head, tail...), head[I], tail[I]...);
    }
    else
    {
        return func(id, head[0], tail[0]...);
    }
}

template<int I, typename type_identity, typename type_head, int N, typename ...type_tail, int ...N_tail,  typename Function>
auto foldHelperI(Function&& func, const type_identity id, const tvecn<type_head, N>& head, const tvecn<type_tail, N_tail>&... tail)
{
    if constexpr (I<N-1)
    {
        return foldHelperI<I+1>(std::forward<Function>(func), func(id, head[I], tail[I]...), head, tail...);
    }
    else
    {
        return func(id, head[N-1], tail[N-1]...);
    }
}

template<typename type_identity, typename type_head, int N_head, typename ...type_tail, int ...N_tail, typename Function = void (const type_identity&, const type_head&, const type_tail&...)>
constexpr auto fold(Function&& func, const type_identity& id, const tvecn<type_head, N_head>& head, const tvecn<type_tail, N_tail>&... tail)
{
    static_assert(std::is_invocable_v<Function, const type_identity&, const type_head&, const type_tail &...>,
     "The function cannot be invoked with these zip arguments (possibly wrong argument count).");
    static_assert(all_equal_v<N_head, N_tail...>, "Vector sizes must match.");

    //return foldHelperR<N_head-1>(std::forward<Function>(func), id, head, tail...);
    return foldHelperI<0>(std::forward<Function>(func), id, head, tail...);
}

int main()
{
    tvecn<int,3> a(1,2,3);
    return fold([](auto x, auto y, auto z) {return x+y+z;}, 0, a, a);
}

This might be better suited for codereview.stackexchange.com? — Max Langhof, Oct 21 '19 at 09:47
@bartop I have added the example invocation in main from the godbolt link I provided. Is that enough, or did you mean something else? — lightxbulb, Oct 21 '19 at 09:49
@MaxLanghof Should I delete the question here and post it on codereview, or should I keep this one, and post an identical one on codreview? I am unsure what the usual approach is. — lightxbulb, Oct 21 '19 at 10:47
@Jarod42 We used to call this iterative recursion (the foldHelperI) when I studied functional programming in uni (note - I did not study functional programming in English, so I am not certain about the exact translation). — lightxbulb, Oct 21 '19 at 11:00

max66 · Accepted Answer · 2019-10-21T11:05:50.793

and can fold multiple vectors at the same time at compile time

Not exactly: if you want to operate compile-time

(1) you have to define constexpr the tvecn constructor and

(2) you have to define constexpr the foldhelper function and

(3) you have to declare constexpr a

 // VVVVVVVVV
    constexpr tvecn<int,3> a(1,2,3);

(4) you have to place the result of fold in a constexpr variable (or, more generally speaking, in a place where the value is required compile time, as the size field of a C-style array, or a template value parameter, or a static_assert() test)

constexpr auto f = fold([](auto x, auto y, auto z) {return x+y+z;},
                        0, a, a);

I am wondering if it could be simplified in some manner

Sure.

First of all: if you can, avoid to reinventing the weel: your tvecn is a simplified version of std::array.

Suggestion: use std::array (if you can obviously)

Second: you tagged C++17 so you can use folding

Suggestion: use it also for all_equal

template <auto V0, auto ... Vs>
struct all_equal : public std::bool_constant<((V0 == Vs) && ...)>
 { };

template<auto ...N_pack>
constexpr bool all_equal_v = all_equal<N_pack...>::value;

More in general: when you have to define a custom type traits that has to provide a number, inherit (if possible) from std::integral_constant (or std::bool_constant, or std::true_type, or std::false_type: all std::integral_constant specializations). So you automatically inherit all std::integral_constant facilities.

Third: almost all C++ standard uses std::size_t, not int, for sizes.

Suggestion: when you have to do with sizes, use std::size_t, not int. This way you can avoid a lot of annoying troubles.

Fourth: from main() you should return only EXIT_SUCCESS (usually zero) or EXIT_FAILURE (usually 1)

Suggestion: avoid things as

return fold([](auto x, auto y, auto z) {return x+y+z;}, 0, a, a);

Fifth: never underestimate the power of the comma operator.

Suggestion: avoid recursion at all and use template folding also for the helper function; by example

template <std::size_t ... Is, typename F, typename T, typename ... As>
constexpr auto foldHelperF (std::index_sequence<Is...>,
                            F const & f, T id, As const & ... arrs)
 { return ( ..., (id = [&](auto i){ return f(id, arrs[i]...); }(Is))); }

that you can call as follows from fold()

return foldHelperF(std::make_index_sequence<N_head>{}, 
                   std::forward<Function>(func),
                   id, head, tail...);

The following is a full compiling, and simplified, example

#include <array>
#include <utility>
#include <iostream>
#include <type_traits>

template <auto V0, auto ... Vs>
struct all_equal : public std::bool_constant<((V0 == Vs) && ...)>
 { };

template<auto ...N_pack>
constexpr bool all_equal_v = all_equal<N_pack...>::value;


template <std::size_t ... Is, typename F, typename T, typename ... As>
constexpr auto foldHelperF (std::index_sequence<Is...>,
                            F const & f, T id, As const & ... arrs)
 { return ( ..., (id = [&](auto i){ return f(id, arrs[i]...); }(Is))); }


template <typename type_identity, typename type_head, std::size_t N_head,
          typename ...type_tail, std::size_t ...N_tail,
          typename Function = void (type_identity const &,
                                    type_head const &,
                                    type_tail const & ...)>
constexpr auto fold (Function && func, type_identity const & id,
                     std::array<type_head, N_head> const & head,
                     std::array<type_tail, N_tail> const & ... tail)
 {
   static_assert( std::is_invocable_v<Function, const type_identity&,
                  const type_head&, const type_tail &...>,
                  "The function cannot be invoked with these zip arguments"
                  " (possibly wrong argument count).");

   static_assert( all_equal_v<N_head, N_tail...>,
                 "Vector sizes must match.");

   return foldHelperF(std::make_index_sequence<N_head>{}, 
                      std::forward<Function>(func),
                      id, head, tail...);
}

int main()
 {
   constexpr std::array<int, 3u> b{2, 5, 7};

   constexpr auto f = fold([](auto x, auto y, auto z) {return x+y+z;},
                           0, b, b);

   std::cout << f << std::endl;
 }

Thank you for all the suggestions. The return from main was to make godbolt actually output a value. Could you elaborate on `Suggestion: avoid things as`? Is there no danger of creating temporaries with: `Suggestion: avoid recursion at all and use template folding`? I am assuming it would produce a result of the form: `id = func(id,...), ..., id = func(id,...)`, as opposed to my code which should produce `func(...func(func(id,h[0],t[0]),h[1],t[1])...)`. — lightxbulb, Oct 21 '19 at 11:10
@lightxbulb - As opposed? It seems to me that the result should be the same. — max66, Oct 21 '19 at 11:12
@lightxbulb - Anyway, as far I know, recursion is inefficient; avoid it when possible; when recursion involve templates, you also have compiler-limits (not very wides) over the number of recursion level you can reach. — max66, Oct 21 '19 at 11:15
As opposed to the recursive variant I had, which expands the function as `func(...func(func(id,h[0],t[0]),h[1],t[1])...)`. Also could you explain why I should avoid things as `fold([]...)`? — lightxbulb, Oct 21 '19 at 11:17
With `( ..., (id = [&](auto i){ return f(id, arrs[i]...); }(Is)));` first you calculate `func(id,h[0],t[0])` and place the result in `id`, then you calculate `func(id,h[1],t[1])`, that is `func(func(id,h[0],t[0]),h[1],t[1])` given the original `id`, then you calculate `func(id,h[2],t[2])`, that is `func(func(func(id,h[0],t[0]),h[1],t[1]),h[2],t[2])` given the original `id`... it seems the same to me. — max66, Oct 21 '19 at 11:22
About `return fold([]...)`, I suggest to avoid because, from `main()`, you should return only zero (`EXIT_SUCCESS`) and 1 (`EXIT_FAILURE`). From other functions I don't see problems. — max66, Oct 21 '19 at 11:24
Ah you meant it only about the return - sure, I just put it in the return so that godbolt would actually produce the result in the assembly. I think the generated code with the C++ fold expr is: `id = func(id,h[0],t[0]), id = func(id, h[1],t[1]), ..., id = func(id,h[N-1],t[N-1])`, while with the recursive approach it is `func(func(func...)...)`. I hope that the two do not generate something with different performance, but I think I will test it and see what happens. Thanks again for all the suggestions, you have helped me a great deal with your answer these last few days, much appreciated. — lightxbulb, Oct 21 '19 at 11:27
@lightxbulb - yes, only about the return in `main()`; it's a general suggestion; obviously you can merrily ignore it if you have a reason; about the performances... frankly I have no idea but I suspect that strictly depend from the specific compiler you're using. — max66, Oct 21 '19 at 11:42

score 2 · Answer 2 · answered Oct 21 '19 at 10:49

2

With Fold expression, it might be:

template <typename F, typename Init, std::size_t... Is, typename... Arrays>
constexpr auto fold_impl(F&& f, Init init, std::index_sequence<Is...>, Arrays&&... arrays)
{
    auto l = [&](Init init, std::size_t i){ return f(init, arrays[i]...); };
    return ((init = l(init, Is)), ...);
}


template <typename F, typename Init, typename Array, typename ... Arrays>
constexpr auto fold(F&& f, Init init, Array&& array, Arrays&&... arrays)
{
    static_assert(((arrays.size() == array.size()) && ...));
    return fold_impl(f, init, std::make_index_sequence<array.size()>{}, array, arrays...);
}

Demo

answered Oct 21 '19 at 10:49

Jarod42

203,559
14
181
302

Is size() for std::array in msvc not a constant expression? See: https://godbolt.org/z/hgB3yT – lightxbulb Oct 21 '19 at 11:38
1

@lightxbulb: It is more complicated than that, `arrays.size()` doesn't depend of `arrays` so could be know at compile time, whereas Msvc requires `constexpr arrays` for that :( . work around is possible: [Demo](https://godbolt.org/z/_9-Xvg). – Jarod42 Oct 21 '19 at 11:54
So if my vectors have a constexpr static size function this should be a non-issue? Also is there a specific reason you pass the arrays in as `Array&&` as oposed to `const Array&`? – lightxbulb Oct 21 '19 at 12:01
1

I hope `constexpr static` should be fine with msvc, to be tested. And no specific reasons to use `Array&&` over `const Array&`. That allows to modify array in `f`, which seems not what you want, so const ref seems what you want... – Jarod42 Oct 21 '19 at 12:50

Improving fold function

2 Answers2