15

Advice on std::forward is generally limited to the canonical use case of perfectly forwarding function template arguments; some commentators go so far as to say this is the only valid use of std::forward. But consider code like this:

// Temporarily holds a value of type T, which may be a reference or ordinary
// copyable/movable value.
template <typename T>
class ValueHolder {
 public:
  ValueHolder(T value)
    : value_(std::forward<T>(value)) {
  }


  T Release() {
    T result = std::forward<T>(value_);
    return result;
  }

 private:
  ~ValueHolder() {}

  T value_;
};

In this case, the issue of perfect forwarding does not arise: since this is a class template rather than a function template, client code must explicitly specify T, and can choose whether and how to ref-qualify it. By the same token, the argument to std::forward is not a "universal reference".

Nonetheless, std::forward appears to be a good fit here: we can't just leave it out because it wouldn't work when T is a move-only type, and we can't use std::move because it wouldn't work when T is an lvalue reference type. We could, of course, partially specialize ValueHolder to use direct initialization for references and std::move for values, but this seems like excessive complexity when std::forward does the job. This also seems like a reasonable conceptual match for the meaning of std::forward: we're trying to generically forward something that might or might not be a reference, only we're forwarding it to the caller of a function, rather than to a function we call ourselves.

Is this a sound use of std::forward? Is there any reason to avoid it? If so, what's the preferred alternative?

Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
Geoff Romer
  • 2,358
  • 1
  • 18
  • 19
  • 2
    `std::forward`, in this case, doesn't make sense. `T` is an argument to the class template, not the constructor/function. `std::forward` makes sense when you let the compiler to deduce the type, which is possible in case of function template. – Nawaz Dec 16 '13 at 17:32
  • 1
    This only makes any sense at all if the parameter is T&& –  Dec 16 '13 at 17:33
  • This is an extended case of perfect-forwarding, if you will. It becomes clearer if you have a `template ValueHolder make_holder(T&& v){ return {std::forward(v)}; }` factory function. Whether the `T` is then user-supplied or through the factory function is irrelevant. (I do advise against the `delete this` part, though.) – Xeo Dec 16 '13 at 17:45
  • @Nawaz, can you elaborate on what you mean by "makes sense" in this context? – Geoff Romer Dec 16 '13 at 23:41
  • As the linked commentator, I should clarify the original statement "`std::forward` has a single use case: to cast a templated function parameter (inside the function) to the value category (lvalue or rvalue) the caller used to pass it." The templated function parameter may be templated by the class, as a member non-template function. And the parameter may be stored in a structure and cast later; I would still consider it to be a parameter because the caller is none the wiser. `std::forward` can be used for optionally-owning proxies, but you should be very very careful with any such design. – Potatoswatter Dec 17 '13 at 01:22
  • The "inside the function" in the statement was not intended as an implementation restriction, but a disambiguation between parameters which are inside the implementation, and arguments which are on the other side of the interface. – Potatoswatter Dec 17 '13 at 02:25
  • As an aside to this year-old problem, one thing I do in similar cases is that my `ValueHolder` ctor takes a universal reference and forwards it into a `t`. That can reduce your moves by 1. Also include an explicit `T&&` ctor to allow `{brace}` initialization. Also, your code fails if `T` is an `int&&`. Also, `Release()` should be `Release() &&` at the least. And as `Release()` implies that the class **must** be allocated on the free store, I'd regularize the type it and have this class do both the `new` and `delete` (only exposing move ctors) mayhap (storing a `store*`) – Yakk - Adam Nevraumont Jan 09 '15 at 21:40

2 Answers2

10

std::forward is a conditional move-cast (or more technically rvalue cast), nothing more, nothing less. While using it outside of a perfect forwarding context is confusing, it can do the right thing if the same condition on the move applies.

I would be tempted to use a different name, even if only a thin wrapper on forward, but I cannot think of a good one for the above case.

Alternatively make the conditional move more explicit. Ie,

template<bool do_move, typename T>
struct helper {
  auto operator()(T&& t) const
  -> decltype(std::move(t))
  {
      return (std::move(t));
  }
};
template<typename T>
struct helper<false, T> {
  T& operator()(T&& t) const { return t; }
};
template<bool do_move, typename T>
auto conditional_move(T&& t)
 ->decltype( helper<do_move,T>()(std::forward<T>(t)) )
{
    return ( helper<do_move,T>()(std::forward<T>(t)) );
}

that is a noop pass-through if bool is false, and move if true. Then you can make your equivalent-to-std::forward more explicit and less reliant on the user understanding the arcane secrets of C++11 to understand your code.

Use:

std::unique_ptr<int> foo;
std::unique_ptr<int> bar = conditional_move<true>(foo); // compiles
std::unique_ptr<int> baz = conditional_move<false>(foo); // does not compile

On the third hand, the kind of thing you are doing above sort of requires reasonably deep understanding of rvalue and lvalue semantics, so maybe forward is harmless.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • 2
    `std::forward` is actually the right tool here. You may turn it into `conditional_move<!std::is_lvalue_reference::value>(value);` but that just obscures and overcomplicates things. It's a form of forwarding, a modified form of perfect-forwarding (since it moves instead of just keeping things as rvalue refs). – Xeo Dec 16 '13 at 21:35
  • How would I use `conditional_move` to implement `ValueHolder` in the original example? In particular, how would I determine the argument to pass for `do_move`? I have no doubt it can be done, but it seems like it would add yet more complexity, and be fairly failure-prone. The DRY principle would also seem to argue against explicitly specifying `do_move` when it's inferrable from `T`. – Geoff Romer Dec 16 '13 at 23:46
  • @GeoffRorner see Xeo's comment above: it reduces to `std::forward`, as you have stated that your intended logic is identical to what `std::forward` does, except it is not called `std::forward` and the condition (under which it moves) is explicit. You could strip the logic out of the expression, like `static constexpr bool should_move = !std::is_lvalue_reference::value;`, then use `conditional_move(blah)`. – Yakk - Adam Nevraumont Dec 17 '13 at 15:28
  • @Yakk-AdamNevraumont Good answer. Small q: instead of `->decltype( helper()(std::forward(t)) )` one could use `decltype(auto)` as return type correct? – KeyC0de Oct 01 '18 at 16:49
  • @nik-lz That requires require C++14, and this is a C++11 question asked before C++14 existed. But yes. In a more general situation, it also costs SFINAE-friendliness, but `conditional_move` cannot fail, so it doesn't need to be SFINAE friendly. – Yakk - Adam Nevraumont Oct 01 '18 at 17:04
1

This sort of wrapper works as long as it's used properly. std::bind does something similar. But this class also reflects that it is a one-shot functionality when the getter performs a move.

ValueHolder is a misnomer because it explicitly supports references as well, which are the opposite of values. The approach taken by std::bind is to ignore lvalue-ness and apply value semantics. To get a reference, the user applies std::ref. Thus references are modeled by a uniform value-semantic interface. I've used a custom one-shot rref class with bind as well.

There are a couple inconsistencies in the given implementation. The return result; will fail in an rvalue reference specialization because the name result is an lvalue. You need another forward there. Also a const-qualified version of Release, which deletes itself and returns a copy of the stored value, would support the occasional corner case of a const-qualified yet dynamically-allocated object. (Yes, you can delete a const *.)

Also, beware that a bare reference member renders a class non-assignable. std::reference_wrapper works around this as well.

As for use of forward per se, it follows its advertised purpose as long as it's passing some argument reference along according to the type deduced for the original source object. This takes additional footwork presumably in classes not shown. The user shouldn't be writing an explicit template argument… but in the end, it's subjective what is good, merely acceptable, or hackish, as long as there are no bugs.

Potatoswatter
  • 134,909
  • 25
  • 265
  • 421
  • `ValueHolder` would not hold an rvalue reference, I think. It'd either hold a `T` or `T&` (i.e., a value or an lvalue reference). As such, just `return value;` is the right way. – Xeo Dec 17 '13 at 08:28