CODEWITHSUNDEEP
c++c++11forwarding-reference
Mário Feroldi
Mário Feroldi

Reputation: 3591

Forwarding the same value to two or more functions

When using forwarding references, is it a bad idea to forward the same value to more than one function? Consider the following piece of code:

template<typename Container>
constexpr auto
front(Container&& c)
-> typename Container::value_type
{ return std::forward<Container>(c).front(); }

template<typename Container>
constexpr auto
back(Container&& c)
-> typename Container::value_type
{ return std::forward<Container>(c).back(); }

template<typename Container>
constexpr auto
get_corner(Container&& c)
{
    return do_something(front(std::forward<Container(c)),
                        back(std::forward<Container>(c));
}

If Container is an lvalue-reference, the function works just fine. However, I'm worrying about situations where rvalues are passed on to it, because the value would get invalidated once a move occurs. My doubt is: Is there a correct way to forward the container in that case, without losing the value category?

Upvotes: 13

Views: 2829

Answers (4)

kfsone
kfsone

Reputation: 24259

You presumably realize that you wouldn't want to std::move an object being passed to multiple functions:

std::string s = "hello";
std::string hello1 = std::move(s);
std::string hello2 = std::move(s);  // hello2 != "hello"

The role of forward is simply to restore any rvalue status that a parameter had when it was passed to the function.

We can quickly demonstrate that it is bad practice by forwarding one parameter two times to a function that has a move effect:

#include <iostream>
#include <string>

struct S {
    std::string name_ = "defaulted";
    S() = default;
    S(const char* name) : name_(name) {}
    S(S&& rhs) { std::swap(name_, rhs.name_); name_ += " moved"; }
};

void fn(S s)
{
    std::cout << "fn(" << s.name_ << ")\n";
}

template<typename T>
void fwd_test(T&& t)
{
    fn(std::forward<T>(t));
    fn(std::forward<T>(t));
}

int main() {
    fwd_test(S("source"));
}

http://ideone.com/NRM8Ph

If forwarding was safe, we should see fn(source moved) twice, but instead we see:

fn(source moved)
fn(defaulted moved)

Upvotes: 3

Kyle Strand
Kyle Strand

Reputation: 16499

In general, yes, this is potentially dangerous.

Forwarding a parameter ensures that if the value received by the universal reference parameter is an rvalue of some sort, it will continue to be an rvalue when it is forwarded. If the value is ultimately forwarded to a function (such as a move-constructor) that consumes the value by moving from it, its internal state is not likely to be valid for use in subsequent calls.

If you do not forward the parameter, it will not (in general) be eligible for move operations, so you would be safe from such behavior.

In your case, front and back (both the free functions and the member functions) do not perform a move on the container, so the specific example you gave should be safe. However, this also demonstrates that there's no reason to forward the container, since an rvalue won't be given different treatment from an lvalue--which is the only reason to preserve the distinction by forwarding the value in the first place.

Upvotes: 2

Nicol Bolas
Nicol Bolas

Reputation: 473577

In general, it is not reasonable for the same function to forward the same parameter twice. Not unless it has specific knowledge of what the receiver of that forwarded parameter will do.

Remember: the behavior of std::forward can be equivalent to the behavior of std::move, depending on what parameter the user passed in. And the behavior of an xvalue will be contingent on how the receiving function processes it. If the receiver takes a non-const rvalue reference, it will likely move from that value if possible. That would leave you holding a moved-from object. If it takes a value, it will certainly move from it if the type supports it.

So unless you have specific knowledge of the expected behavior of the operations you are using, it is not safe to forward a parameter more than once.

Upvotes: 15

Barry
Barry

Reputation: 303186

There's actually no rvalue-reference version of std::begin - we just have (setting aside constexpr and return values):

template <class C>
??? begin(C& );

template <class C>
??? begin(C const& );

For lvalue containers, you get iterator, and for rvalue containers, you get const_iterator (or whatever the container-specific equivalent ends up being).

The one real problem in your code is returning decltype(auto). For lvalue containers, that's fine - you'll return a reference to an object whose lifetime exceeds the function. But for rvalue containers, that's returning a dangling reference. You'll want to return a reference for lvalue containers and a value for rvalue containers.

On top of that, forward-ing the containers into begin()/end() is probably not what you want to do. It'd be more efficient to conditionally wrap the result of the select() as a move iterator. Something like this answer of mine:

template <typename Container,
          typename V = decltype(*std::begin(std::declval<Container&>())),
          typename R = std::conditional_t<
              std::is_lvalue_reference<Container>::value,
              V,
              std::remove_reference_t<V>
              >
          >
constexpr R operator()(Container&& c)
{
    auto it = select(std::begin(c), std::end(c));
    return *make_forward_iterator<Container>(it);
}

There's probably a less verbose way to express all of that.

Upvotes: 4

Related Questions