Reduce boilerplate 2, electric boogaloo, and extras

[ChemiCalChems]

More boilerplate removal, this time I managed to get the only reference to the word boilerplate to be in include/rfl/internal/bind_to_tuple.hpp, whereas before they were still dancing around in a couple of files.

This has the added advantage that we won't have to maintain the > 100 fields static_assert in multiple places, as well as the obvious. If we ever want to increase the limit for any reason (or find a less tedious way of providing the same functionality), we can do so in one single file too!

The interface provided by rfl::internal::bind_to_tuple allows for great flexibility when mapping structs to tuples, so that's cool.

I also took the liberty of upgrading rfl::internal::to_ptr_field_tuple in the same way rfl::internal::copy/move_to_field_tuple where upgraded a couple of days ago, by wrapping the struct in rfl::Fields as necessary if not previously done, in accordance to the function names.

[liuzicheng1987]

Cool, I will take a look.

[liuzicheng1987]

@ChemiCalChems , it looks really great so far. The proposed changes make a lot of sense and it compiles on GCC and Clang. I want to make sure it works for MSVC as well, though. But it's a bit late, so I'll do that tomorrow.

Thank you very much for this PR.

[liuzicheng1987]

@ChemiCalChems, I have one quick question: Is there any particular reason you are forwarding the lambda functions?

Specifically, instead of doing this:

template <class T, typename F>
constexpr auto bind_to_tuple(T& _t, F&& f) {
  auto view = tuple_view(_t);
  return [&]<std::size_t... Is>(std::index_sequence<Is...>) {
    return std::make_tuple(std::forward<F>(f)(std::get<Is>(view))...);
  }(std::make_index_sequence<std::tuple_size_v<decltype(view)>>());
}

Why don't we just do this?

template <class T, typename F>
constexpr auto bind_to_tuple(T& _t, const F& _f) {
  auto view = tuple_view(_t);
  return [&]<std::size_t... Is>(std::index_sequence<Is...>) {
    return std::make_tuple(_f(std::get<Is>(view))...);
  }(std::make_index_sequence<std::tuple_size_v<decltype(view)>>());
}

The reason I am asking is that I don't really think there should ever be a necessity for the function f to have a side-effect and that should be reflected in the code. Also, it is slightly easier to read.

[liuzicheng1987]

@ChemiCalChems , the tests on MSVC compile and run through without any issues. I am going to merge this. I think the forwardings vs const reference issue is such a minor thing, we can discuss (and potentially fix) that later.

Again, thank you so much for your great contribution.

[ChemiCalChems]

Forwarding is less about the function having side-effects and more about avoiding unnecessary copies, but you're right, in this particular sense it doesn't really matter much since we are just gonna be using the function in-place.

[ecatmur]

This breaks serialization for structs with fields of type std::reference_wrapper<T>, since std::ref collapses. Would it be possible to change std::make_tuple(std::ref(f1), ...) to std::tie(f1, ...)?

[ChemiCalChems]

This breaks serialization for structs with fields of type std::reference_wrapper<T>, since std::ref collapses. Would it be possible to change std::make_tuple(std::ref(f1), ...) to std::tie(f1, ...)?

Fair point, agreed.