using std::forward on the same argument in a loop

236 Views Asked by At

I know there are cases where using std::forward on the same argument in a loop is wrong because it may cause moving from a moved object, like so:

template <typename T>
auto applyTenTimes(T&& arg, auto&& f){
    for(int i = 0; i < 10; ++i)
        f(std::forward<T>(arg));

    return arg; 
}

But, what about the case where the forwarded object gets assigned again? Like in this example:

template <typename T>
auto applyTenTimes(T&& arg, auto&& f){
    for(int i = 0; i < 10; ++i)
        arg = f(std::forward<T>(arg));

    return arg; 
}

Would this be valid? If yes, then why? Is it basically never creating a new object (when called with an rvalue) and just moving the arg into the function f and then gets moved back again into arg by RVO?

I tried looking at different StackOverflow questions, but none seemed to have what I was looking for!

3

There are 3 best solutions below

6
Ahmed AEK On

std::forward, this will expand according to the input being an rvalue, or lvalue as follows.

// for lvalue or moved lvalue
template <typename T>
auto applyTenTimes(T& arg, auto&& f){
    for(int i = 0; i < 10; ++i)
        arg = f(arg);

    return arg; 
}

// for rvalue
template <typename T>
auto applyTenTimes(T arg, auto&& f){
    for(int i = 0; i < 10; ++i)
        arg = f(std::move(arg));

    return arg; 
}

both functions are perfectly valid c++ code, and both involve calling the copy/move assignment operator, as function parameters can't be copy-elided.

0
HolyBlackCat On

It's invalid. If you passed an rvalue and f happens to return the same reference it was given, you could end up with self-move-assignment, which classes are generally not expected to handle correctly (unlike self-copy-assignment).

I'm not entirely sure how to handle this. Something along the lines of:

if constexpr (std::is_same_v<decltype(f(std::move(arg))), std::remove_reference_t<T> &&>)
    arg = auto(f(std::move(arg)));`.
else
    arg = f(std::move(arg));

Or, if you can't be bothered, simply arg = f(arg).

Forwarding rather than moving arg doesn't make sense, since we don't need to preserve the old value, as we're going to overwrite it immediately anyway.


Also it's weird to pass arg by reference, but then to return by value.

I would suggest either passing by value and returning by value, or doing both by reference.

1
Yakk - Adam Nevraumont On

Function templates are a form of overloading. Overloading is having the same function name refer to more than one function.

The key to make overloading sane is to make every overload have the same semantic meaning. Then the selection of which specific implementation, being not visible to the caller without extreme effort, is less likely to cause them to go insane or cause crazy bugs.

Your applyTenTimes, despite having the same implementation (up to the type parameter) doesn't really work that way. If passed an lvalue, the first parameter is an in-out argument; if passed an rvalue, it is an in only parameter; in both cases, it is used as temporary swap space and is left in a strange state at the end of the operation.

A sane version of your template is:

template <typename T>
requires !std::is_reference_v<T>
[[nodiscard]] T applyTenTimes(T arg, auto&& f){
  for(int i = 0; i < 10; ++i)
    arg = f(std::move(arg));

  return arg; 
}

or even just

[[nodiscard]] auto applyTenTimes(auto arg, auto&& f){
  for(int i = 0; i < 10; ++i)
    arg = f(std::move(arg));

  return arg; 
}

this always takes arg by value, then uses it as a temporary internally, then returns the result. It avoids a number of traps (like the risks of move self-assign, or const-assign-to).

The input argument is not used as swap space. f does not get a reference parameter it thinks is meaningful, which is semantically accurate.