To forward or move or not; how to determine which is preferred within the context of a class's usage?

Question

class Test {
public:
    typedef std::set<std::pair<double, double>> DataSet;

    explicit Test(const DataSet&& d) {
        for (auto &itr : d) {
            std::cout << "value1 = " << itr.first << '\n';
            std::cout << "value2 = " << itr.second << '\n';
        }
    }

};

int main() {
    //using namespace util;
    try {
        // Forwarding
        Test obj( std::forward<Test::DataSet>(
            { 
              { 10.0, 20.0 },
              { 30.0, 40.0 },
              { 50.0, 60.0 }
            }
        ));

        std::cout << '\n';

        // Move
        Test obj2(std::move(Test::DataSet(
            {
              { 10.0, 20.0 },
              { 30.0, 40.0 },
              { 50.0, 60.0 }
            }
           )
        ));

    } catch (const std::exception& e) {
        std::cerr << e.what() << std::endl;
        return EXIT_FAILURE;
    }

    return EXIT_SUCCESS;
}

Answer 1

Neither.

Test::DataSet(/*...*/) is already a rvalue that can bind to a rvalue reference. The only thing std::move does is to turn a lvalue into a xvalue (which is a rvalue), so that it can bind to a rvalue reference, which lvalues cannot. So std::move is redundant.

std::forward does the same only conditional on its template argument. So it is redundant (at best) as well.

const DataSet&& as parameter makes no sense. You want either DataSet&& which binds only to rvalues or const DataSet&, which binds to rvalues and lvalues.

If you want to make use of the fact that the passed reference might be a rvalue (i.e. by moving from its members), then you need two overloads of your function. One taking DataSet&& which is called with rvalue arguments and one taking const DataSet& which is called with lvalue arguments.

In the specific case of the Test function in your question, I don't see how there would be any benefit to moving from the members or do any other modification of the DataSet state, so I would go with simply const DataSet& as parameter and never bother with std::moveing or std::forwarding into that function specifically.

Answer 2

[Code samples edited in response to comments]
A useful bit of advice from the ISO Core Guidelines. This would suggest that you should prefer (const T&), but if you really need speed add a (T&&) specifically. This way you can pass small sets as (L-Value)

const auto data = Test::DataSet{ 
          { 10.0, 20.0 },
          { 30.0, 40.0 },
          { 50.0, 60.0 }};
Test obj( data );

or (R-Value)

Test obj( Test::DataSet{ 
          { 10.0, 20.0 },
          { 30.0, 40.0 },
          { 50.0, 60.0 },
        });

Adding the extra move makes the second kind of call more efficient, but the advice is to only use this type of optimization if it is required. You should avoid having different semantics for one constructor than for the other, and the easiest way to avoid that is to only have one constructor.

Personally, even after 20+ years of C++ programming, I found the table in the link so useful I printed it out and stuck it to my monitor.