Consequences of and alternatives to use std::forward on non-forwarding-reference type template parameter

Question

I am writing a factory. Both "interface" and the "implementation" are defined by template classes.

#include <memory>

template<class I, class ...Args>
struct IFactory
{
  virtual std::unique_ptr<I> Create(Args... args) = 0;
};

template<class I, class C, class ...Args>
struct Factory : IFactory<I, Args...>
{
  std::unique_ptr<I> Create(Args... args) override
  {
    return std::make_unique<C>(std::forward<Args>(args)...); // args are no forwarding references
  }
};

The code violates the sonar source rule RSPEC-5417, which states:

std::forward has a single use-case: to cast a templated function parameter of type forwarding reference (T&&) to the value category (lvalue or rvalue) the caller used to pass it. [...] An error [...] has less dire consequences [than using std::move on a forwarding reference], and might even work as intended if the right template argument is used, but the code would be clumsy and not clearly express the intent.
[Emphasis by me]

I wonder,

what are the less dire consequences if wrong template arguments used?
what are the wrong template arguments?
how to ensure the right template parameters are used?
how to write the code less clumsy and express intent more clearly?

I considered to use static_cast<Args&&>() directly, but that would make the code less readable in my opinion and i think it would only re-implement std::forward.

Example usage of Factory<...> shows that Factory::Create() generates one additional move construction (for the ctor argument T1 a, in the example below):

#include <string>
#include <iostream>

void P(std::string msg){std::cout << msg << std::endl;} // Debug print

// Some types used as ctor arguments types of the class for which we want to create objects.
struct T1{T1()=default; T1(const T1&){P("T1&");} T1(T1&&){P("T1&&");}}; // Move- and copyable
struct T2{T2()=default; T2(const T2&){P("T2&");} T2(T2&&)=delete;    }; // Copyable
struct T3{T3()=default; T3(const T3&)=delete;    T3(T3&&){P("T3&&");}}; // Moveable
struct T4{T4()=default; T4(const T4&)=delete;    T4(T4&&)=delete;    }; // None of move/copy

// Interface of the class
struct IType
{ /*Some pure virtual functions.*/
};

struct Type : IType
{
  T1  t1;
  T2  t2;
  T3  t3;
  T4& t4;
  Type(T1 a, const T2& b, T3&& c, T4& d)
  :t1(a), t2(b), t3(std::move(c)), t4(d) {}
};

void F(const IFactory<IType, T1, const T2&, T3&&, T4&>& factory)
{
  T1 t1;
  T2 t2;
  T3 t3a, t3b;
  T4 t4;
  std::cout << "Ctor:\n";
  Type obj1(t1, t2, std::move(t3a), t4);
  std::cout << "Factory:\n";
  auto ptri1 = factory.Create(t1, t2, std::move(t3b), t4);
}

int main()
{
  Factory<IType, Type, T1, const T2&, T3&&, T4&> factory1;
  F(factory1);
  return 0;
}

Output:

Ctor:
T1&
T1&
T2&
T3&&
Factory:
T1&
T1&&         <- additional move for the not optimal ctor argument
T1&
T2&
T3&&

Example on gobolt.org

You shouldn’t really ever delete a move constructor—it causes dumb things to happen during overload resolution (necessitating a special rule for *implicitly-declared* deleted move constructors). Just declaring a copy constructor—even as defaulted—is enough to make a class copyable but “not movable”. — Davis Herring, Nov 07 '21 at 05:13
@DavisHerring, can you explain what dumb things happen if you take T(T&&) out of overload resolution? To me it looks like a good thing, because it can turn a performance bug into a compile-time error. — e3918, Nov 07 '21 at 08:35
The whole problem is that it isn’t removed from consideration for overload resolution. `std::move` *grants permission* for the recipient to consume the object, and it’s [wrong](https://stackoverflow.com/q/37092864/8586227) to say “you mustn’t have such permission”. The converse, “permission to copy” as a performance gatekeeper, just doesn’t exist (partly because the `int`s would always need it in the current system). — Davis Herring, Nov 07 '21 at 18:55
Thanks for the explanation, @DavisHerring. I learned, that my wording in my post above was wrong. `T(T&&) = delete` *adds* the move-ctor to the overloads (as deleted), not removes it. Anyhow, i leave my test-code for my original question as it is, because i want it to create reasonable results even for unusual user-classes. — e3918, Nov 08 '21 at 08:32

score 0 · Accepted Answer · answered Nov 08 '21 at 15:19

If a template argument is deduced other than from a forwarding reference, it is never deduced as a reference: then std::forward<T> is just the overload set

T&& forward(T&);
T&& forward(T&&);

which behaves exactly like std::move. If the function parameter was declared as T&, this is misleading: the argument will be moved from, so the function should probably accept an rvalue (or forwarding) reference instead.

If the function parameter is just T, std::forward is harmless since the parameters are your own objects, but you might as well just use std::move.

Your case, however, is a bit different: this interface requires explicit template arguments, which can be references at the discretion of the client. If it’s not a reference, it again behaves like std::move. If it is, then std::forward reproduces the same kind of reference, which seems to be the correct behavior.

For completeness, consider the case of explicitly specifying the template argument for a function that accepts T&: regardless of the reference status of the template argument, the function will accept an lvalue reference, and will move from it(!) if the template argument is itself not a reference or an rvalue reference. The same two cases will result in a move when forwarding a non-forwarding T&& parameter, but then the (function) argument must be an rvalue.

In conclusion, using std::forward<T> when the parameter is T& is wrong whether or not T is deduced, but is correct for T or T&& regardless. The quoted guideline is too strict in that case: the deduced-T case should just be std::move, but your usage is useful and correct.

Consequences of and alternatives to use std::forward on non-forwarding-reference type template parameter

1 Answers1