CODEWITHSUNDEEP
c++move-semantics
Epic
Epic

Reputation: 622

passing a rvalue vs lvalue vector

I have a method my func that gets a vector from get_vec and passes it to some constructor of class A.

class A
{
public:
    A(std::vector<int>&& vec) : m_vec(std::move(vec)) {}
    std::vector<int> m_vec;
};

std::vector<int> get_vec()
{
   std::vector<int> res;
   // do something
   return res;
}

void my_func()
{
    std::vector<int> vec = get_vec();
    A(std::move(vec));
}

https://godbolt.org/z/toQ5KZ

Ideally I would like the vector to be constructed once, but in this example I create the vector in get_vec, then copied it to my_func, moved it to a constructor and then again moved it to A::m_vec.

What is the correct and efficient way to pass the vector?

Upvotes: 2

Views: 399

Answers (3)

catnip
catnip

Reputation: 25388

I decided to rewrite this answer as Jorge Perez kindly pointed out that the original was flawed, and because none of the other answers really address the original question in its entirety.

I started by writing a simple test program:

#include <iostream>

class Moveable
{
public:
    Moveable ()  { std::cout << "Constructor\n"; }
    ~Moveable ()  { std::cout << "Destructor\n"; }
    Moveable (const Moveable&) { std::cout << "Copy constructor\n"; }
    Moveable& operator= (const Moveable&) { std::cout << "Copy assignment\n"; return *this; }
    Moveable (const Moveable&&) { std::cout << "Move constructor\n"; }
    Moveable& operator= (const Moveable&&) { std::cout << "Move assignment\n"; return *this; }
};

class A
{
public:
    A (Moveable &&m) : m_m (std::move (m)) {}
    Moveable m_m;
};

Moveable get_m ()
{
   Moveable res;
   return res;
}

int main ()
{
    Moveable m = get_m ();
    A (std::move (m));
}

which gives the following output:

Constructor
Move constructor
Destructor
Destructor

So you can immediately see that the inefficiencies in your code are not as bad as you think - there's just one move and no copies.

Now, as others have said:

Moveable m = get_m ();

doesn't copy anything because of Named Return Value Optimisation (NRVO).

And there's only one move because:

A (std::move (m));

doesn't actually move anything (it's just a cast).

Live demo

With regard to the move, it's clearer what's going on, arguably, if you change this:

A (Moveable&& m) : m_m (std::move (m)) {}

to this:

A (Moveable& m) : m_m (std::move (m)) {}

because you can then change this:

A (std::move (m));

to this:

A {m};

and still get the same output (you need the braces to avoid 'the most vexing parse').

Live demo

Upvotes: 0

Max Langhof
Max Langhof

Reputation: 23681

Compiler optimizations are usually only allowed if they don't change observable behavior. However, copy elision is one of the few cases where even changing the program's output (compared to that of the abstract machine) is allowed in the name of performance.

We can demonstrate this with a mock vector whose special member functions have side effects (such as writing to a volatile variable):

class MyVec
{
public:
    MyVec() { x = 11; };

    MyVec(const MyVec&) { x = 22; }
    MyVec(MyVec&&) { x = 33; }

    MyVec& operator=(const MyVec&) { x = 44; return *this; }
    MyVec& operator=(MyVec&&) { x = 55; return *this; }

    // Make this the same size as a std::vector
    void* a = nullptr;
    void* b = nullptr;
    void* c = nullptr;
};

If we inspect the optimized assembly, we can see that only one default constructor and one move constructor's side effects are actually kept in my_func, everything else is optimized away. The first default constructor is the inlined get_vec, the other is the move in the constructor of A. That's as efficient as you could possibly be when constructing a member from a temporary.

This is allowed because copies can be elided when returning from a function, as well as when initializing from (more or less) a temporary. The latter used to be "you can elide the copy in X x = getX();", but since C++ 17 versions no temporary is ever created (https://en.cppreference.com/w/cpp/language/copy_initialization).

Upvotes: 1

Maxim Egorushkin
Maxim Egorushkin

Reputation: 136237

std::vector<int>&& m_vec member will cause a dangling reference if you do A a(get_vec());, for example.

The correct and safe way is to have that member by value:

std::vector<int> m_vec;

Upvotes: 3

Related Questions