4
\$\begingroup\$

I want to implement for_each_in_tuples that takes a functor and one or more tuples of the same size and applies this functor to i'th elements of each tuple for i = 0, ..., size - 1.

Example:

#include <iostream>
#include <tuple>

std::tuple t1(1, 2.2, false);
std::tuple t2(3.3, 'a', 888);

std::cout << std::boolalpha;
for_each_in_tuples(
    [](auto a1, auto a2)
    {
        std::cout << a1 << ' ' << a2 << '\n';
    }, t1, t2);

// Outputs:
// 1 3.3
// 2.2 a
// false 888

Implementation:

#include <cstddef>
#include <tuple>
#include <type_traits>

namespace impl
{
template<typename T, typename... Ts>
struct First { using Type = T; };

template<typename... Ts>
using First_t = typename First<Ts...>::Type;

template<auto value, auto... values>
inline constexpr auto all_same = (... && (value == values));

template<class Tuple>
inline constexpr auto tuple_size = std::tuple_size_v<std::remove_reference_t<Tuple>>;

template<std::size_t index = 0, class Function, class... Tuples>
constexpr void for_each_in_tuples(Function func, Tuples&&... tuples)
{
    constexpr auto size = tuple_size<First_t<Tuples...>>;

    func(std::get<index>(std::forward<Tuples>(tuples))...);
    if constexpr (index + 1 < size)
        for_each_in_tuples<index + 1>(func, std::forward<Tuples>(tuples)...);
}
}

template<class Function, class... Tuples>
constexpr void for_each_in_tuples(Function func, Tuples&&... tuples)
{
    static_assert(sizeof...(Tuples) > 0);
    static_assert(impl::all_same<impl::tuple_size<Tuples>...>);

    impl::for_each_in_tuples(func, std::forward<Tuples>(tuples)...);
}

At Compiler explorer: https://godbolt.org/g/cYknQT

Main questions:

  1. Is this implementation correct and can it be simplified?

  2. Which name is better, for_each_in_tuple or for_each_in_tuples (or ...)?

\$\endgroup\$
0
4
\$\begingroup\$
template<typename T, typename... Ts>
struct First { using Type = T; };

template<typename... Ts>
using First_t = typename First<Ts...>::Type;

This can be done using std::tuple_element in conjunction with std::tuple.

template <typename... Ts>
using First_t = std::tuple_element_t<0, std::tuple<Ts...>>;

template<auto value, auto... values>
inline constexpr auto all_same = (... && (value == values));

For an empty pack, do you really want the value to be undefined? The default behavior with empty packs are && is considered true and || is considered false.


static_assert(sizeof...(Tuples) > 0);

Instead of failing, maybe try to call the function with 0 arguments and see what happens?


static_assert(impl::all_same<impl::tuple_size<Tuples>...>);

If you want zipped-like behavior, you'll want to zip tuples until one of the tuples has exhausted its elements (min size instead first size).


Is this implementation correct and can it be simplified?

Yes. Use an iterative approach, like sequential expansion, over recursion.

Which name is better, for_each_in_tuple or for_each_in_tuples (or ...)?

Maybe for_each_zipped. Tuples can be gathered from the signature of the function.


If you want to avoid the recursion, use fold expressions, std::index_sequence, and std::make_index_sequence.

Start with a simple helper to just invoke the function with elements across all the tuples at a specific index.

template <std::size_t Index, typename Function, typename... Tuples>
constexpr void invoke_at(Function&& func, Tuples&&... tuples) {
    func(std::get<Index>(std::forward<Tuples>(tuples))...);
}

Now we need a way to sequentially call it (invoke_at<0>(args), invoke_at<1>(args), ..., invoke<N>(args)). Use fold expressions like you did for all_same, but with the comma operator and unary right fold ((invoke_at<N>(args), ...)). To generate the Ns that gets expanded, we use std::index_sequence.

template <std::size_t... Indices, typename Function, typename... Tuples>
constexpr void apply_sequence(Function&& func,  std::index_sequence<Indices...>, Tuples&&... tuples) {
    (((void)invoke_at<Indices>(std::forward<Function>(func), std::forward<Tuples>(tuples)...), ...));
}

Finally, write your function that does checks for preconditions, creates the index sequence, and forwards the arguments to the above helper.

template <typename Function, typename... Tuples>
constexpr void tuple_for_each(Function&& func, Tuples&&... tuples) {
    static_assert(sizeof...(tuples) > 0, "Must be called with at least one tuple argument");

    constexpr auto min_length = std::min({std::tuple_size_v<std::remove_reference_t<Tuples>>...});
    if constexpr (min_length != 0) {
        impl::apply_sequence(std::forward<Function>(func),
                             std::make_index_sequence<min_length>{},
                             std::forward<Tuples>(tuples)...);
    }
    else {
        func();
    }
}

Note - The expansion has a cast to void that disables any overloaded shenanigans abusing the comma operator.

\$\endgroup\$
3
\$\begingroup\$

There's not much I can criticize about your code; as far as I can see, everything seems well thought out and well written.

As to question number 1: Yes, I think your implementation is correct. I also don't think there's much to be simplified; the implementation is pretty concise, and getting rid of the recursion should be impossible until we get constexpr while or constexpr for (if ever).

As to question 2, I'm much in favor of for_each_in_tuples, since you're clearly iterating multiple tuples at once.

If there's anything to improve, I'd say it's First. Since parameter packs are "like" lists of types, the name Head would fit much better, in my opinion. Also, just as with normal function parameters, I consider it good style to leave unused parameters unnamed, i.e.

template <typename T, typename...>
struct Head { using Type = T; };

This is really just personal preference, though.

There is, however, at least one real "issue" with your code, which is related to the func parameter in for_each_in_tuples: Since you're passing func around by value, you could cause performance degradation if func is not a lightweight function object (such as a lambda capturing some object with expensive copy operations). Furthermore, this pass-by-value semantics also lock the user out from using function objects that don't support copy operations at all, such as, e.g., a lambda capturing anything of a move-only type. To fix this, I suggest you simply std::move func into each recursive call, which will cover almost all cases (except for expensive-to-move types, but those are quite rare).

\$\endgroup\$
2
  • \$\begingroup\$ I'm unsure about this but did you miss an s in lock the user out? \$\endgroup\$
    – yuri
    Aug 7 '18 at 10:11
  • \$\begingroup\$ @yuri No, not really. The meaning doesn't change much, whether it's lock the user out or lock the users out, I think. \$\endgroup\$ Aug 7 '18 at 14:37
2
\$\begingroup\$

Your implementation is correct even if Snowhawk did a good job pointing at what could be improved (although I'd say your recursive approach isn't more complex than "sequential expansion").

My main concern would be with the more general design: a function that 1) zips tuples and 2) applies a function over the zipped elements does one thing too many. It should be composed of two functions: one applying the function to a tuple's members (let's say tuple_for_each) and one zipping the tuples (let's say tuples_zip). That means you can use tuple_for_each also in situations when you don't need to zip, and that you can zip tuples also when you don't need to apply a function to the zipped elements.

Moreover, for_each is another loss of generality, since it can't be used when you need to recover the result of the function you're applying (you could with lambda capture, but that's a contortion). So, I would advocate for a tuple_map function, returning a tuple containing the result of the application in the case where there is one.

tuple_map would look like this:

#include <iostream>
#include <tuple>

template <typename Fn, typename Tuple, std::size_t... Ns>
constexpr auto tuple_map_impl(Fn&& fn, std::index_sequence<Ns...>, Tuple&& tuple) {
    // if fn's return type is void, do not return a tuple since it'd be invalid
    using fn_return_type = decltype(fn(std::get<0>(tuple)));
    if constexpr (std::is_same_v<fn_return_type, void>)
        (std::forward<Fn>(fn)(std::get<Ns>(std::forward<Tuple>(tuple))),...);
    else  
        return std::forward_as_tuple(std::forward<Fn>(fn)(std::get<Ns>(std::forward<Tuple>(tuple)))...);
    }

template <typename Fn, typename Tuple>
constexpr auto tuple_map(Fn&& fn, Tuple&& tuple) {
    constexpr auto tsz = std::tuple_size_v<std::decay_t<Tuple>>;
    // we need to handle the case where tuple size == 0 because we check fn(std::get<0>(tuple)) in tuple_map_impl
    if constexpr (tsz == 0) return std::forward<Tuple>(tuple);
    else return tuple_map_impl(std::forward<Fn>(fn),
                               std::make_index_sequence<std::tuple_size_v<std::decay_t<Tuple>>>(),
                               std::forward<Tuple>(tuple));
    }

and tuple_zip like this:

template <std::size_t N, typename... Tuples>
auto zip_tuples_at(Tuples&&... tuples) {
    return std::forward_as_tuple(std::get<N>(std::forward<Tuples>(tuples))...);
    }

template <typename Tuple, typename... Other_tuples>
constexpr auto indexes() {
    return std::make_index_sequence<std::tuple_size_v<std::decay_t<Tuple>>>();
    }

template <std::size_t... Ns, typename... Tuples>
constexpr auto tuples_zip_impl(std::index_sequence<Ns...>, Tuples&&... tuples) {
    return std::make_tuple(zip_tuples_at<Ns>(tuples...)...);
    }

template <typename... Tuples>
constexpr auto tuples_zip(Tuples&&... tuples) {
    return tuples_zip_impl(indexes<Tuples...>(), std::forward<Tuples>(tuples)...);
    }

We can then compose zipped_tuples_map (my suggestion for the name of your function) with those two functions and std::apply:

template <typename Fn, typename... Tuples>
auto zipped_tuples_map(Fn&& fn, Tuples&&... tuples) {
    return tuple_map([&fn](auto&& zipped_elements) { return std::apply(std::forward<Fn>(fn), zipped_elements); },
                     tuples_zip(std::forward<Tuples>(tuples)...));
    } 

A link to the complete example: wandbox.

\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.