0

I wrote the following snippet to test if I could perfectly forward values through a tuple and std::invoke. However the generated assembly looks kind of odd.

Demo

#include <concepts>
#include <string>
#include <cstdio>
#include <tuple>
#include <functional>


auto foo(std::string str, std::string str2 = "temp")
{
    printf("Foo executed with %s and %s!\n", str.data(), str2.data());
}


template <typename Cb, typename... Args>
auto async(Cb&& fn, Args&&... args)
{
    constexpr std::size_t cnt = sizeof...(Args);
    auto tuple = std::make_tuple<Args...>(std::forward<Args>(args)...);

    std::invoke([&]<std::size_t... I>(std::index_sequence<I...>){
        foo(std::get<I>(tuple)...);
    }, std::make_index_sequence<cnt>{});
}


int main()
{
    async(&foo, std::string("mystring"), std::string("other"));
}

x86-46 gcc 12.2 -Wall -Os --std=c++20

Excerpt from line 30:

        call    std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&) [complete object constructor]
        lea     rsi, [rsp+16]
        lea     rdi, [rsp+176]
        call    std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&) [complete object constructor]
        lea     rsi, [rsp+144]
        lea     rdi, [rsp+112]
        call    std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) [complete object constructor]
        lea     rsi, [rsp+176]
        lea     rdi, [rsp+80]
        call    std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) [complete object constructor]
        lea     rsi, [rsp+112]
        lea     rdi, [rsp+80]
        call    foo(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)

As you can see there's a call to the copy constructor of std::string. Where does it originate from? My suspicion is the call to foo() using std::get<I>(tuple) which doesn't seem to perfectly forward the values. In this regard I'm lacking understanding: How is "rvalue-ness" routed through a std::tuple? Is the value category preserved inside the tuple or is everything converted to either copy or lvalue-reference? Is there a chance to optimize this as well?

Clarification:

My goal is essentially to achieve perfect forwarding from the callsite of async() to foo(). That means value categories should be preserved as-is and no unnecessary copies should be made in between.

E.g. When I call async(&foo, std::string("string1"), std::string("string2")) it should invoke the move constructor for the strings constructed in foo's parameter list.

E.g. When I call async(&foo, str1, str2) with predefined strings they should be picked up as rvlaue-references instead and foo's parameters should be copy-constructed.

glades
  • 3,778
  • 1
  • 12
  • 34
  • I suspect you want `auto tuple = std::forward_as_tuple(std::forward(args)...);` or just `auto tuple = std::forward_as_tuple(args...);` (I'm not 100% sure) – Ted Lyngmo Jan 20 '23 at 20:49
  • @TedLyngmo That's better, it gets rid of the rvalue_ref constructor! Still there's the copy constructor which I suppose is there because the foo() parameters are constructed by copy and not by rvalue ref... Can that be changed as well? – glades Jan 20 '23 at 20:54
  • `Can that be changed as well?` Soooooo `foo(std::string& str, std::string& str2` ? – KamilCuk Jan 20 '23 at 20:59
  • I must say I'm not following exactly what you expect. Instead of using a `std::string`, can't you use a [`foo`](https://godbolt.org/z/j5zzqWojT) that logs every instantiation/copy/move/destruction? It's a lot easier to see the effects that way. – Ted Lyngmo Jan 20 '23 at 20:59
  • @KamilCuk No the std::string parameters should be move-constructed rather than copy constructed. – glades Jan 20 '23 at 21:06
  • Then `std::move(std::get(tuple))` no? – KamilCuk Jan 20 '23 at 21:13
  • 1
    @KamilCuk But that would move in any case, even if the intent was to copy construct. I think I must clarify the question – glades Jan 20 '23 at 21:17
  • There is another problem with your code that might be very confusing if it manifests. Because you are using the name `asnyc` for your function and you call it with unqualified name and you pass it a `std` type, it is actually unspecified whether your program is well-formed. If the standard library function `std::async` of the same name is included directly or indirectly, then it will be found by ADL and be ambiguous with your template. Avoid using names already used int he standard library and avoid calling with unqualified name if you do not intent for ADL. (https://godbolt.org/z/zan7YorYT) – user17732522 Jan 20 '23 at 21:33

1 Answers1

4

std::make_tuple makes a new tuple of values of the passed types. The result isn't a tuple storing references at all.

If you want a tuple of references you need std::forward_as_tuple instead. If you save the result in a variable, you are of course responsible to make sure that the referenced objects outlive the result tuple.

Also, neither std::make_tuple nor std::forward_as_tuple are supposed to be given template arguments explicitly. That will break their expected properties. (I guess for std::make_tuple it isn't that strict of a rule, but for std::forward_as_tuple it is.)

// tuple is a `std::tuple` of references of appropriate kind
// matching the value categories of `args`
auto tuple = std::forward_as_tuple(std::forward<Args>(args)...);

Furthermore, std::get will forward the value category only if you pass the tuple as rvalue. This means you need to use std::move explicitly. This prevents that an element referenced by the tuple can be moved without any explicit indication of that in the code:

std::invoke([&]<std::size_t... I>(std::index_sequence<I...>){
    foo(std::get<I>(std::move(tuple))...);
}, std::make_index_sequence<cnt>{});

However, the standard library already has std::apply which does exactly this for you:

std::apply(fn, std::move(tuple));

Also, I assume that you need the tuple for something else as well, because otherwise you can simply do

std::invoke(fn, std::forward<Args>(args)...);

(And you probably want std::forward<Cb>(fn) instead of fn as well in case that is something more complex than a function pointer.)


Also note that using async as name for your function is not a good idea. There is a function template with the same name, template head and function parameter list in the std namespace. As a consequence, whenever your async is called with a type being from or depending on the namespace std, it will be ambiguous with std::async in overload resolution because it is found via ADL.

Therefore users would need to always make sure to call your async only by qualified name (which doesn't cause ADL). Otherwise the user's code may fail to compile randomly (because any standard library might or might not include std::async indirectly) and will certainly break any user that included <future>.

See https://godbolt.org/z/zan7YorYT for a demonstration.

user17732522
  • 53,019
  • 2
  • 56
  • 105
  • Yess that solved it! The key was "std::get will forward the value category only if you pass the tuple as rvalue". This is due to reference collapsing right? – glades Jan 20 '23 at 21:23
  • 1
    @glades Yes, if you look at the signature [here](https://en.cppreference.com/w/cpp/utility/tuple/get) it adds an lvalue reference to the return type in the lvalue overload. That way rvalue references in the tuple collapse to lvalue references when returned. That's intentional so that there can't be implicit moves from a tuple lvalue which might still be used later. – user17732522 Jan 20 '23 at 21:25
  • Now I get an internal compiler error: segmentation fault x) Looks like I found gcc's weak spot – glades Jan 20 '23 at 21:42
  • @glades ICE is always a bug, so it seems so. But it is usually an indication that one is trying to something in an unusual way. Consider using one of the simpler approaches with `std::apply` or direct `std::invoke` if you can. – user17732522 Jan 20 '23 at 21:43
  • I've put it here: https://stackoverflow.com/questions/75193528/stdforwarddecltypeargsargs-for-parameter-pack-in-lambda How would you do it? – glades Jan 21 '23 at 13:07